Re: [PATCH 2/2] help.c: use SHELL_PATH instead of hard-coded "/bin/sh"

[Jeff King <peff@peff.net>]

On Sun, Mar 08, 2015 at 11:32:22PM -0700, Kyle J. McKay wrote:

> >It is a common convention to make the first argument the command
> >name without its path, and this change breaks that convention.
> 
> Hmpf.  I present these for your consideration:
> 
> $ sh -c 'echo $0'
> sh
> $ /bin/sh -c 'echo $0'
> /bin/sh
> $ cd /etc
> $ ../bin/sh -c 'echo $0'
> ../bin/sh
> 
> I always thought it was the actual argument used to invoke the item.  If the
> item is in the PATH and was invoked with a bare word then arg0 would be just
> the bare word or possibly the actual full pathname as found in PATH.
> Whereas if it's invoked with a path (relative or absolute) that would passed
> instead.

Yes, you are correct. When there is a full path, that typically gets
passed instead (unless you are trying to convey something specific to
the program, like telling bash "pretend to be POSIX sh"; that's usually
done with a symlink, but the caller might want to override it).

If we were starting from scratch, I would say that SHELL_PATH is
supposed to be a replacement POSIX shell, and so we should call:

  execl(SHELL_PATH, "sh", "-c", ...);

to tell shells like bash to operate in POSIX mode.

However, that is _not_ what we currently do with run-command's
use_shell directive. There we put SHELL_PATH as argv[0], and run:

  execv(argv[0], argv);

I doubt it matters much in practice (after all, these are just "-c"
snippets, not whole scripts). But it's possible that by passing "-c" we
would introduce bugs (e.g., if somebody has a really complicated inline
alias, and sets SHELL_PATH to /path/to/bash, they'll get full-on bash
with the current code).

> I also have no objection to changing it to:
> 
> >-	execl("/bin/sh", "sh", "-c", shell_cmd.buf, (char *)NULL);
> >+	execl(SHELL_PATH, basename(SHELL_PATH), "-c", shell_cmd.buf, (char
> >*)NULL);
> 
> just to maintain the current behavior.

If we want to maintain consistency with the rest of our uses of
run-command, it would be just your original:

  execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, NULL);

That makes the most sense to me, unless we are changing run-command's
behavior, too. 

There's no point in calling basename(). Shells like bash which
behave differently when called as "sh" are smart enough to check the
basename themselves (this would matter, e.g., if you set SHELL_PATH to
"/path/to/my/sh" and that was actually a symlink to bash).

-Peff