Re: [BUG] [PATCH]: run-command.c

[Duncan Roe <duncan_roe@acslink.net.au>]

On Fri, Oct 21, 2016 at 05:00:29AM -0400, Jeff King wrote:
> On Fri, Oct 21, 2016 at 04:50:13PM +1100, Duncan Roe wrote:
>
> > For example, if .git/config has this alias (the sleep is to leave time to
> > examine output from ps, &c.):
> >
> > [alias]
> > 	tryme = "!echo $PWD;sleep 600"
> >
> > [...]
> > 16:42:06$ ps axf|grep -A2 trym[e]
> >  2599 pts/4    S+     0:00      \_ git tryme
> >  2601 pts/4    S+     0:00          \_ /bin/sh -c echo $PWD;sleep 600 echo $PWD;sleep 600
> >  2602 pts/4    S+     0:00              \_ sleep 600
> > 16:42:45$ cat /proc/2601/cmdline | xargs -0 -n1 echo
> > /bin/sh
> > -c
> > echo $PWD;sleep 600
> > echo $PWD;sleep 600
>
> This duplicated argument is expected and normal. The arguments after "-c
> whatever" become positional parameters $0, $1, etc. The actual script
> arguments start at "$1", and "$0" is typically the "script name".
> So you have to stick some placeholder value in the "$0" slot, so that
> the sub-script can find the actual arguments. E.g., try:
>
>   sh -c '
>     for i in "$@"; do
>       echo "got $i"
>     done
>   ' one two three
>
> it will print only:
>
>   got two
>   got three
>
> But if you stick a placeholder there, it works:
>
>   sh -c '
>     for i in "$@"; do
>       echo "got $i"
>     done
>   ' placeholder one two three
>
> The value of the placeholder does not matter to the shell. But it is
> accessible to the script inside via $0:
>
>   sh -c '
>     echo "\$0 = $0"
>     echo "\$1 = $1"
>     echo "\$2 = $2"
>     echo "\$3 = $3"
>   ' placeholder one two three
>
> Since our script does not have a filename, we just stick the script
> contents there (which is really just a convention, and one I doubt
> anybody is really relying on, but there's no point in breaking it now).
>
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -182,8 +182,8 @@ static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
> >  		else
> >  			argv_array_pushf(out, "%s \"$@\"", argv[0]);
> >  	}
> > -
> > -	argv_array_pushv(out, argv);
> > +	else
> > +		argv_array_pushv(out, argv);
> >  	return out->argv;
> >  }
>
> Try running "make test" with this. Lots of things break, because we are
> not sending the positional parameters to the shell script at all.
>
> If we just cared about the positional parmeters, we _could_ do something
> like:
>
>   if (argv[0]) {
> 	argv_array_push(out, "sh");
> 	argv_array_pushv(out, argv + 1);
>   }
>
> That would omit "$0" entirely when we have no positional parameters (and
> the shell generally fills in "sh" there itself), and provide a dummy
> "sh" value when we need to use it as a placeholder.
>
> But again, there's no real value in breaking the existing convention.
>
> -Peff
Agreed - tests 110 and 111 in t1300-repo-config.sh fail. After that, "make test"
gives up, losing about 14000 lines of output.

Sorry for the noise,

Cheers ... Duncan.