Re: [PATCH v5 3/3] run-command: show prepared command

From: Junio C Hamano <gitster@pobox.com>
Date: May 25, 2024, 05:44 UTC
Message-ID: <xmqqr0dqigx7.fsf@gitster.g>
In-Reply-To: <20240525012207.1415196-3-iwienand@redhat.com>

Ian Wienand <iwienand@redhat.com> writes:

Show 12 quoted lines

> diff --git a/run-command.c b/run-command.c
> index 1b821042b4..9892c4421c 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)
>  		goto end_of_spawn;
>  	}
>  
> +	trace_argv_printf(&argv.v[1], "trace: start_command:");
> +
>  	if (pipe(notify_pipe))
>  		notify_pipe[0] = notify_pipe[1] = -1;

This side is OK ...

Show 8 quoted lines

> @@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)
>  	else if (cmd->use_shell)
>  		cmd->args.v = prepare_shell_cmd(&nargv, sargv);
>  
> +	trace_argv_printf(&cmd->args.v[1], "trace: start_command:");
>  	cmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,
>  				  (char**) cmd->env.v,
>  				  cmd->dir, fhin, fhout, fherr);

... but this side should pass "cmd->args.v" (i.e., the entire array, without omitting the zeroth element) to be consistent with the other side. I made the same mistake in my "how about doing it this way" draft, by the way.

It is because prepare_cmd() does this:

        static int prepare_cmd(struct strvec *out, const struct child_process *cmd)
        {
                ...
                /*
                 * Add SHELL_PATH so in the event exec fails with ENOEXEC we can
                 * attempt to interpret the command with 'sh'.
                 */
                strvec_push(out, SHELL_PATH);

                if (cmd->git_cmd) {
                        prepare_git_cmd(out, cmd->args.v);
                } else if (cmd->use_shell) {
                        prepare_shell_cmd(out, cmd->args.v);
                ...

So, the other side (i.e. non Windows, that used the result from prepare_cmd()) skips the argv.v[0] element (which is the SHELL_PATH pushed by prepare_cmd()), but because a bare use of prepare_shell_cmd() done on the Windows side does not have that excess element at the beginning.