[PATCH] Makefile: use backticks rather than $() notation to support ancient shells

[PATCH] Makefile: use backticks rather than $() notation to support ancient shells

[Brandon Casey]

Since make is using /bin/sh to execute shell code, avoid the newish
shell construct $() so older (ancient) shells can execute the shell
code in the Makefile.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


I know that $() is preferred in the main scripts, but the Makefile
is using /bin/sh to execute shell code and there are already a few
places in the Makefile using back-ticks, so it doesn't seem like
going against the flow too much.

Otherwise, should we set the SHELL variable to the configured SHELL_PATH
at some point in the Makefile?

-brandon


 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 608a185..421af24 100644
--- a/Makefile
+++ b/Makefile
@@ -1366,8 +1366,8 @@ endif
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
 endif
-	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
-	execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
+	bindir=`cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd` && \
+	execdir=`cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd` && \
 	if test "z$$bindir" != "z$$execdir"; \
 	then \
 		ln -f "$$bindir/git$X" "$$execdir/git$X" || \
-- 
1.6.0.rc1.87.g56c9f.dirty

Re: [PATCH] Makefile: use backticks rather than $() notation to support ancient shells

[Johannes Schindelin]

Hi,

On Tue, 5 Aug 2008, Brandon Casey wrote:

> Otherwise, should we set the SHELL variable to the configured SHELL_PATH 
> at some point in the Makefile?

I think that would make more sense, especially since it would catch 
wrong SHELL_PATH early.

Maybe we can even have some sanity check that tests if SHELL_PATH groks 
$()?

Ciao,
Dscho

[PATCH] Makefile: set SHELL to value of SHELL_PATH

[Brandon Casey]

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 5 Aug 2008, Brandon Casey wrote:
> 
>> Otherwise, should we set the SHELL variable to the configured SHELL_PATH 
>> at some point in the Makefile?
> 
> I think that would make more sense, especially since it would catch 
> wrong SHELL_PATH early.

You're right, this makes more sense.

-brandon


 Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 52c67c1..61fc86d 100644
--- a/Makefile
+++ b/Makefile
@@ -1060,6 +1060,8 @@ export TAR INSTALL DESTDIR SHELL_PATH
 
 ### Build rules
 
+SHELL = $(SHELL_PATH)
+
 all:: $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$p';)
-- 
1.5.6.2

[PATCH] Makefile: add a target which will abort compilation with ancient shells

[Brandon Casey]

This adds a make target which can be used to try to execute certain shell
constructs which are required for compiling and running git.

This patch provides a test for the $() notation for command substition
which is used in the Makefile and extensively in the git scripts.

The make target is named in such a way as to be a hint to the user that
SHELL_PATH should be set to an appropriate shell. If the shell command
fails, the user should receive a message similar to the following:

make: *** [please_set_SHELL_PATH_to_a_more_modern_shell] Error 2

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Johannes Schindelin wrote:
> Maybe we can even have some sanity check that tests if SHELL_PATH groks 
> $()?

how about this?

-brandon


 Makefile |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 61fc86d..90c5a13 100644
--- a/Makefile
+++ b/Makefile
@@ -1062,7 +1062,7 @@ export TAR INSTALL DESTDIR SHELL_PATH
 
 SHELL = $(SHELL_PATH)
 
-all:: $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$p';)
 endif
@@ -1075,6 +1075,11 @@ endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
+please_set_SHELL_PATH_to_a_more_modern_shell:
+	@$$(:)
+
+shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
+
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
@@ -1457,6 +1462,7 @@ endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS
 
 .PHONY: all install clean strip
+.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
 .PHONY: .FORCE-GIT-BUILD-OPTIONS
 
-- 
1.5.6.2

Re: [PATCH] Makefile: add a target which will abort compilation with ancient shells

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> This adds a make target which can be used to try to execute certain shell
> constructs which are required for compiling and running git.
>
> This patch provides a test for the $() notation for command substition
> which is used in the Makefile and extensively in the git scripts.
>
> The make target is named in such a way as to be a hint to the user that
> SHELL_PATH should be set to an appropriate shell. If the shell command
> fails, the user should receive a message similar to the following:
>
> make: *** [please_set_SHELL_PATH_to_a_more_modern_shell] Error 2
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> Johannes Schindelin wrote:
>> Maybe we can even have some sanity check that tests if SHELL_PATH groks 
>> $()?
>
> how about this?

Thanks, both.  I think this is cute and at the same time the right thing
to do ;-).