Re: [PATCH] git: run alias subprocess according to the API
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 15, 2025, 08:33 UTC
- Message-ID
- <xmqqms6wi08y.fsf@gitster.g>
- In-Reply-To
- <ad7066022efe5344eb1972d1c489372d97b2d18a.1757842206.git.code@khaugsbakk.name>
kristofferhaugsbakk@fastmail.com writes:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name> > > `run-command.h` tells us that this is how we should run git(1) commands.
Perhaps that comment needs to be updated at the same time?
Show 7 quoted lines
> The only other thing I found was in `upload-pack.c`. > > strvec_push(&pack_objects.args, "git"); > > But that one is intentional; this is inside an else-block and the > if-block has `pack_objects.git_cmd = 1;`. And attempts to refactor it > broke `t5544-pack-objects-hook.sh`.
Hmph, doesn't it indicate that it is wrong to use ".git_cmd = 1" unconditionally, no? I.e., the instruction you found might make it sound as if the only difference is to spawn "git" directly or doing it via the "shell", but because it is not, the result of your update broke that test, right?
So, we want to tell programmers when to use it, with the comment that says "most of the time, if you are spawning a "git" command, use the .git_cmd = 1 mechanism, but if you are doing X or Y then, spawn that "git" command just like any other programs spawned via the shell, because of such and such reasons", no?
Show 29 quoted lines
> git.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index d020eef021c..f16a8fbb55d 100644
> --- a/git.c
> +++ b/git.c
> @@ -825,18 +825,18 @@ static int run_argv(struct strvec *args)
> * process will log the actual verb when it runs.
> */
> trace2_cmd_name("_run_git_alias_");
>
> commit_pager_choice();
>
> - strvec_push(&cmd.args, "git");
> for (size_t i = 0; i < args->nr; i++)
> strvec_push(&cmd.args, args->v[i]);
>
> trace_argv_printf(cmd.args.v, "trace: exec:");
>
> + cmd.git_cmd = 1;
> /*
> * if we fail because the command is not found, it is
> * OK to return. Otherwise, we just pass along the status code.
> */
> cmd.silent_exec_failure = 1;
> cmd.clean_on_exit = 1;
>
> base-commit: ab427cd991100e94792fce124b0934135abdea4b