Brandon Williams <bmw...@google.com> writes:

> @@ -238,6 +238,12 @@ static void prepare_cmd(struct argv_array *out, const 
> struct child_process *cmd)
>       if (!cmd->argv[0])
>               die("BUG: command is empty");
>  
> +     /*
> +      * Add SHELL_PATH so in the event exec fails with ENOEXEC we can
> +      * attempt to interpret the command with 'sh'.
> +      */
> +     argv_array_push(out, SHELL_PATH);
> +
>       if (cmd->git_cmd) {
>               argv_array_push(out, "git");
>               argv_array_pushv(out, cmd->argv);


So, given "cat-file", "-t", "HEAD" with cmd->git_cmd == TRUE, this
will now prepare ("/bin/sh", "git", "cat-file", "-t", "HEAD", NULL) in
the argv-array, and then ...

> @@ -246,6 +252,20 @@ static void prepare_cmd(struct argv_array *out, const 
> struct child_process *cmd)
>       } else {
>               argv_array_pushv(out, cmd->argv);
>       }
> +
> +     /*
> +      * If there are no '/' characters in the command then perform a path
> +      * lookup and use the resolved path as the command to exec.  If there
> +      * are no '/' characters or if the command wasn't found in the path,
> +      * have exec attempt to invoke the command directly.
> +      */
> +     if (!strchr(out->argv[1], '/')) {
> +             char *program = locate_in_PATH(out->argv[1]);
> +             if (program) {
> +                     free((char *)out->argv[1]);
> +                     out->argv[1] = program;
> +             }
> +     }

... turn the first element from "git" to "/usr/bin/git", i.e.

        ("/bin/sh", "/usr/bin/git", "cat-file", "-t", "HEAD", NULL)

which ...

>  #endif
>  
> @@ -445,7 +465,15 @@ int start_command(struct child_process *cmd)
>                       }
>               }
>  
> -             sane_execvp(argv.argv[0], (char *const *) argv.argv);
> +             /*
> +              * Attempt to exec using the command and arguments starting at
> +              * argv.argv[1].  argv.argv[0] contains SHELL_PATH which will
> +              * be used in the event exec failed with ENOEXEC at which point
> +              * we will try to interpret the command using 'sh'.
> +              */
> +             execv(argv.argv[1], (char *const *) argv.argv + 1);

... first is given without the leading "/bin/sh", as the end-user
intended (sort of), but if it fails

> +             if (errno == ENOEXEC)
> +                     execv(argv.argv[0], (char *const *) argv.argv);

"/bin/sh" tries to run "/usr/bin/git" that was not executable (well,
the one in "usr/bin/" would have +x bit, but let's pretend that we
are trying to run one from bin-wrappers/ and somehow forgot +x bit)?

I think all of that is sensible, but there is one "huh?" I can't
figure out.  Typically we do "sh -c git cat-file -t HEAD" but this
lacks the "-c" (cf. the original prepare_shell_cmd()); why do we not
need it in this case?

Thanks.

Reply via email to