{"thread":{"id":"14849","subject":"[PATCH] Makefile: use backticks rather than $() notation to support ancient shells","startedAt":"2008-08-05T23:22:09Z","lastAt":"2008-08-08T20:19:00Z","messageCount":5,"participants":["Brandon Casey","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"86301","messageId":"K5bb057jTokXyOIU_aDE4vMr3jT4DOgSPRcIktfus6QMVq6dszrgGw@cipher.nrlssc.navy.mil","threadId":"14849","inReplyTo":null,"subject":"[PATCH] Makefile: use backticks rather than $() notation to support ancient shells","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-08-05T23:22:09Z","receivedAt":"2008-08-05T23:22:09Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Since make is using /bin/sh to execute shell code, avoid the newish\nshell construct $() so older (ancient) shells can execute the shell\ncode in the Makefile.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nI know that $() is preferred in the main scripts, but the Makefile\nis using /bin/sh to execute shell code and there are already a few\nplaces in the Makefile using back-ticks, so it doesn't seem like\ngoing against the flow too much.\n\nOtherwise, should we set the SHELL variable to the configured SHELL_PATH\nat some point in the Makefile?\n\n-brandon\n\n\n Makefile |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 608a185..421af24 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1366,8 +1366,8 @@ endif\n ifneq (,$X)\n \t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)\n endif\n-\tbindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \\\n-\texecdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \\\n+\tbindir=`cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd` && \\\n+\texecdir=`cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd` && \\\n \tif test \"z$$bindir\" != \"z$$execdir\"; \\\n \tthen \\\n \t\tln -f \"$$bindir/git$X\" \"$$execdir/git$X\" || \\\n-- \n1.6.0.rc1.87.g56c9f.dirty\n"},{"id":"86305","messageId":"alpine.DEB.1.00.0808060208450.9611@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"14849","inReplyTo":"K5bb057jTokXyOIU_aDE4vMr3jT4DOgSPRcIktfus6QMVq6dszrgGw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] Makefile: use backticks rather than $() notation to support ancient shells","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-06T00:10:15Z","receivedAt":"2008-08-06T00:10:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 5 Aug 2008, Brandon Casey wrote:\n\n> Otherwise, should we set the SHELL variable to the configured SHELL_PATH \n> at some point in the Makefile?\n\nI think that would make more sense, especially since it would catch \nwrong SHELL_PATH early.\n\nMaybe we can even have some sanity check that tests if SHELL_PATH groks \n$()?\n\nCiao,\nDscho\n"},{"id":"86430","messageId":"BUkcoxp99ASidQD8sGQ9PR7w1MT6DXx-6v_djYMHRBKTs_wNXEO3hw@cipher.nrlssc.navy.mil","threadId":"14849","inReplyTo":"alpine.DEB.1.00.0808060208450.9611@pacific.mpi-cbg.de.mpi-cbg.de","subject":"[PATCH] Makefile: set SHELL to value of SHELL_PATH","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-08-07T19:03:42Z","receivedAt":"2008-08-07T19:03:42Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nJohannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 5 Aug 2008, Brandon Casey wrote:\n> \n>> Otherwise, should we set the SHELL variable to the configured SHELL_PATH \n>> at some point in the Makefile?\n> \n> I think that would make more sense, especially since it would catch \n> wrong SHELL_PATH early.\n\nYou're right, this makes more sense.\n\n-brandon\n\n\n Makefile |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 52c67c1..61fc86d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1060,6 +1060,8 @@ export TAR INSTALL DESTDIR SHELL_PATH\n \n ### Build rules\n \n+SHELL = $(SHELL_PATH)\n+\n all:: $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$p';)\n-- \n1.5.6.2\n"},{"id":"86431","messageId":"S1hQNVU9hvkXWOErhlMRjqFKaCUFasuIfMwjO4N7QOvu1ddz9cU6_w@cipher.nrlssc.navy.mil","threadId":"14849","inReplyTo":"BUkcoxp99ASidQD8sGQ9PR7w1MT6DXx-6v_djYMHRBKTs_wNXEO3hw@cipher.nrlssc.navy.mil","subject":"[PATCH] Makefile: add a target which will abort compilation with ancient shells","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-08-07T19:06:26Z","receivedAt":"2008-08-07T19:06:26Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This adds a make target which can be used to try to execute certain shell\nconstructs which are required for compiling and running git.\n\nThis patch provides a test for the $() notation for command substition\nwhich is used in the Makefile and extensively in the git scripts.\n\nThe make target is named in such a way as to be a hint to the user that\nSHELL_PATH should be set to an appropriate shell. If the shell command\nfails, the user should receive a message similar to the following:\n\nmake: *** [please_set_SHELL_PATH_to_a_more_modern_shell] Error 2\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nJohannes Schindelin wrote:\n> Maybe we can even have some sanity check that tests if SHELL_PATH groks \n> $()?\n\nhow about this?\n\n-brandon\n\n\n Makefile |    8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 61fc86d..90c5a13 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1062,7 +1062,7 @@ export TAR INSTALL DESTDIR SHELL_PATH\n \n SHELL = $(SHELL_PATH)\n \n-all:: $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$p';)\n endif\n@@ -1075,6 +1075,11 @@ endif\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all\n \t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)\n \n+please_set_SHELL_PATH_to_a_more_modern_shell:\n+\t@$$(:)\n+\n+shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n+\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n@@ -1457,6 +1462,7 @@ endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n \n .PHONY: all install clean strip\n+.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n .PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS\n .PHONY: .FORCE-GIT-BUILD-OPTIONS\n \n-- \n1.5.6.2\n"},{"id":"86541","messageId":"7vwsirb6q3.fsf@gitster.siamese.dyndns.org","threadId":"14849","inReplyTo":"S1hQNVU9hvkXWOErhlMRjqFKaCUFasuIfMwjO4N7QOvu1ddz9cU6_w@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] Makefile: add a target which will abort compilation with ancient shells","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-08T20:19:00Z","receivedAt":"2008-08-08T20:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> This adds a make target which can be used to try to execute certain shell\n> constructs which are required for compiling and running git.\n>\n> This patch provides a test for the $() notation for command substition\n> which is used in the Makefile and extensively in the git scripts.\n>\n> The make target is named in such a way as to be a hint to the user that\n> SHELL_PATH should be set to an appropriate shell. If the shell command\n> fails, the user should receive a message similar to the following:\n>\n> make: *** [please_set_SHELL_PATH_to_a_more_modern_shell] Error 2\n>\n> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n> ---\n>\n>\n> Johannes Schindelin wrote:\n>> Maybe we can even have some sanity check that tests if SHELL_PATH groks \n>> $()?\n>\n> how about this?\n\nThanks, both.  I think this is cute and at the same time the right thing\nto do ;-).\n"}]}