{"thread":{"id":"14475","subject":"[PATCH] Use SHELL_PATH","startedAt":"2008-07-16T01:31:28Z","lastAt":"2008-07-17T00:38:56Z","messageCount":7,"participants":["SungHyun Nam","Sverre Rabbelier","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83461","messageId":"g5jj21$bsp$1@ger.gmane.org","threadId":"14475","inReplyTo":null,"subject":"[PATCH] Use SHELL_PATH","fromName":"SungHyun Nam","fromEmail":"goweol@gmail.com","sentAt":"2008-07-16T01:31:28Z","receivedAt":"2008-07-16T01:31:28Z","isPatch":true,"sender":{"key":"goweol@gmail.com","avatar":null},"body":"\nSigned-off-by: SungHyun Nam \n---\n t/Makefile | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex a778865..0d65ced 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -26,7 +26,7 @@ clean:\n \t$(RM) -r 'trash directory' test-results\n\n aggregate-results:\n-\t./aggregate-results.sh test-results/t*-*\n+\t'$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*\n\n # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL\n full-svn-test:\n-- \n1.5.6.3.350.g6c11a\n"},{"id":"83502","messageId":"bd6139dc0807160347n2ebcd64fy7a15fe94bfabe580@mail.gmail.com","threadId":"14475","inReplyTo":"g5jj21$bsp$1@ger.gmane.org","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-07-16T10:47:43Z","receivedAt":"2008-07-16T10:47:43Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Wed, Jul 16, 2008 at 3:31 AM, SungHyun Nam wrote:\n>\n> Signed-off-by: SungHyun Nam \n> ---\n> t/Makefile | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/Makefile b/t/Makefile\n> index a778865..0d65ced 100644\n> --- a/t/Makefile\n> +++ b/t/Makefile\n> @@ -26,7 +26,7 @@ clean:\n> $(RM) -r 'trash directory' test-results\n>\n> aggregate-results:\n> - ./aggregate-results.sh test-results/t*-*\n> + '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*\n>\n> # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL\n> full-svn-test:\n> --\n> 1.5.6.3.350.g6c11a\n\nIt is not clear to me what this patch does, there is no justification\nin the commit msg either. Instead you say what is being done, which we\ncan see from the commit diff. Please clarify?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"83503","messageId":"alpine.DEB.1.00.0807161259150.8503@eeepc-johanness","threadId":"14475","inReplyTo":"bd6139dc0807160347n2ebcd64fy7a15fe94bfabe580@mail.gmail.com","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-16T11:00:59Z","receivedAt":"2008-07-16T11:00:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 16 Jul 2008, Sverre Rabbelier wrote:\n\n> On Wed, Jul 16, 2008 at 3:31 AM, SungHyun Nam wrote:\n> >\n> > Signed-off-by: SungHyun Nam \n> > ---\n> > t/Makefile | 2 +-\n> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> >\n> > diff --git a/t/Makefile b/t/Makefile\n> > index a778865..0d65ced 100644\n> > --- a/t/Makefile\n> > +++ b/t/Makefile\n> > @@ -26,7 +26,7 @@ clean:\n> > $(RM) -r 'trash directory' test-results\n> >\n> > aggregate-results:\n> > - ./aggregate-results.sh test-results/t*-*\n> > + '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*\n> >\n> > # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL\n> > full-svn-test:\n> > --\n> > 1.5.6.3.350.g6c11a\n> \n> It is not clear to me what this patch does, there is no justification\n> in the commit msg either. Instead you say what is being done, which we\n> can see from the commit diff. Please clarify?\n\nMy _guess_ is that this comes from a platform like Solaris, where /bin/sh \nis not even POSIX. And I'd expect aggregate-results to use some \nnon-trivial shell constructs which break with such a broken shell.\n\nBut I completely agree, the commit message desperately wants to include \nsome justification.\n\nCiao,\nDscho\n"},{"id":"83528","messageId":"7vr69tu91e.fsf@gitster.siamese.dyndns.org","threadId":"14475","inReplyTo":"bd6139dc0807160347n2ebcd64fy7a15fe94bfabe580@mail.gmail.com","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-16T15:46:05Z","receivedAt":"2008-07-16T15:46:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sverre Rabbelier\" writes:\n\n> On Wed, Jul 16, 2008 at 3:31 AM, SungHyun Nam wrote:\n>>\n>> Signed-off-by: SungHyun Nam \n>> ---\n>> t/Makefile | 2 +-\n>> 1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/t/Makefile b/t/Makefile\n>> index a778865..0d65ced 100644\n>> --- a/t/Makefile\n>> +++ b/t/Makefile\n>> @@ -26,7 +26,7 @@ clean:\n>> $(RM) -r 'trash directory' test-results\n>>\n>> aggregate-results:\n>> - ./aggregate-results.sh test-results/t*-*\n>> + '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*\n>>\n>> # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL\n>> full-svn-test:\n>> --\n>> 1.5.6.3.350.g6c11a\n>\n> It is not clear to me what this patch does, there is no justification\n> in the commit msg either. Instead you say what is being done, which we\n> can see from the commit diff. Please clarify?\n\nIt wants to make sure that the shell specified from the toplevel Makefile\n(or from make command line) is used to run the aggregation script. It is\noften necessary when platform /bin/sh is not quite POSIX (e.g. the script\nin question uses arithmetic expansion \"$(( $var1 + $var2 ))\").\n\nJust saying \"Use specified shell to run shell scripts\" on the title line\nwould be sufficient, but I wonder if this is the only remaining place...\n"},{"id":"83530","messageId":"7vej5tu84y.fsf@gitster.siamese.dyndns.org","threadId":"14475","inReplyTo":"7vr69tu91e.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-16T16:05:33Z","receivedAt":"2008-07-16T16:05:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> It wants to make sure that the shell specified from the toplevel Makefile\n> (or from make command line) is used to run the aggregation script. It is\n> often necessary when platform /bin/sh is not quite POSIX (e.g. the script\n> in question uses arithmetic expansion \"$(( $var1 + $var2 ))\").\n>\n> Just saying \"Use specified shell to run shell scripts\" on the title line\n> would be sufficient, but I wonder if this is the only remaining place...\n\nPeople on funny platforms might find this one useful, which is consistent\nwith the way the patch under discussion called the shell. This lets you\nto say\n\n\tSHELL_PATH = /Program Files/GNU/Bash\n\nor something silly like that ;-)\n\n Makefile | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 9b52071..a2b2627 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -153,7 +153,7 @@ all::\n # broken, or spawning external process is slower than built-in grep git has).\n \n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n-\t@$(SHELL_PATH) ./GIT-VERSION-GEN\n+\t@'$(SHELL_PATH_SQ)' ./GIT-VERSION-GEN\n -include GIT-VERSION-FILE\n \n uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')\n"},{"id":"83628","messageId":"487E932A.5080502@gmail.com","threadId":"14475","inReplyTo":"7vr69tu91e.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"SungHyun Nam","fromEmail":"goweol@gmail.com","sentAt":"2008-07-17T00:32:42Z","receivedAt":"2008-07-17T00:32:42Z","isPatch":true,"sender":{"key":"goweol@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> \"Sverre Rabbelier\" writes:\n> \n>> On Wed, Jul 16, 2008 at 3:31 AM, SungHyun Nam wrote:\n>>> Signed-off-by: SungHyun Nam \n>>> ---\n>>> t/Makefile | 2 +-\n>>> 1 files changed, 1 insertions(+), 1 deletions(-)\n>>>\n>>> diff --git a/t/Makefile b/t/Makefile\n>>> index a778865..0d65ced 100644\n>>> --- a/t/Makefile\n>>> +++ b/t/Makefile\n>>> @@ -26,7 +26,7 @@ clean:\n>>> $(RM) -r 'trash directory' test-results\n>>>\n>>> aggregate-results:\n>>> - ./aggregate-results.sh test-results/t*-*\n>>> + '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*\n>>>\n>>> # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL\n>>> full-svn-test:\n>>> --\n>>> 1.5.6.3.350.g6c11a\n>> It is not clear to me what this patch does, there is no justification\n>> in the commit msg either. Instead you say what is being done, which we\n>> can see from the commit diff. Please clarify?\n> \n> It wants to make sure that the shell specified from the toplevel Makefile\n> (or from make command line) is used to run the aggregation script. It is\n> often necessary when platform /bin/sh is not quite POSIX (e.g. the script\n> in question uses arithmetic expansion \"$(( $var1 + $var2 ))\").\n> \n> Just saying \"Use specified shell to run shell scripts\" on the title line\n> would be sufficient, but I wonder if this is the only remaining place...\n\nFor the 'make test', yes, it's the only remaining place to me.\nWell, I skip the cvs/svn/.. tests which I never use.\nAnd I need this patch for the Solaris.\n\n\nAnd I also need to run a script function below between 'make all'\nand 'make test/install'. I hope GIT does this. Of course, GIT's\nMakefile would use SHELL_PATH and PERL_PATH.\n\ngit_fix_interpreter()\n{\n p=\"/usr/local/bin/perl\"\n if test -x \"$p\"\n then\n for f in $(find . -name '*.perl')\n do\n e=$(dirname $f)/$(basename $f .perl)\n [ -f \"$e\" -a -x \"$e\" ] \\\n && \"$p\" -pi -e \n's@^#!/usr/(bin/perl.*)$@#!/usr/local/\\1@g' \"$e\"\n done\n fi\n if test -x /bin/bash\n then\n for f in $(find . -name '*.sh')\n do\n e=$(dirname $f)/$(basename $f .sh)\n [ -f \"$e\" -a -x \"$e\" ] \\\n && \"$p\" -pi -e 's@^#!/bin/sh(.*)$@#!/bin/bash\\1@g' \"$e\"\n done\n fi\n}\n"},{"id":"83629","messageId":"487E94A0.1040801@gmail.com","threadId":"14475","inReplyTo":"487E932A.5080502@gmail.com","subject":"Re: [PATCH] Use SHELL_PATH","fromName":"SungHyun Nam","fromEmail":"goweol@gmail.com","sentAt":"2008-07-17T00:38:56Z","receivedAt":"2008-07-17T00:38:56Z","isPatch":true,"sender":{"key":"goweol@gmail.com","avatar":null},"body":"SungHyun Nam wrote:\n> And I also need to run a script function below between 'make all'\n> and 'make test/install'. I hope GIT does this. Of course, GIT's\n> Makefile would use SHELL_PATH and PERL_PATH.\n\nOops! I'm sorry! Ignore this part. GIT alreay does it, just i didn't \nrealize that.\n\nRegards,\nnamsh\n"}]}