From: Jeff King Date: Mon, 15 Sep 2025 19:31:42 GMT Subject: Re: [PATCH] git: run alias subprocess according to the API Message-ID: <20250915193142.GA651907@coredump.intra.peff.net> In-Reply-To: On Mon, Sep 15, 2025 at 12:09:14PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > I'm not sure there _is_ any reason to use a raw "git" instead of > > ".git_cmd". The case in upload-pack is _not_ running a git command > > directly. It is passing the name of a git command to the hook. So you > > end up running: > > > > /path/to/my/hook git pack-objects ...args... > > > > So I don't think there is any X or Y, and the rule can just be "you > > should use .git_cmd". > > ... > > That said, I do not think it is particularly important to use .git_cmd; > > it really is just prepending "git" to argv. It does also override > > .use_shell, but it is nonsense to set both flags in the first place > > (which is something we could perhaps flag with a BUG() if we cared to). > > But I have no problem with preferring .git_cmd to "git" as a style > > preference. > > Ah, OK. I just was confused by the explanation in the middle that > appeared to be saying that they are not truly equivalent. If not, > then I am perfectly fine. Yeah, I think Kristoffer's explanation was just confusing. > I would actually prefer to get rid of the .git_cmd member altogether > to make it simpler to code. The run_command() function should be > able to internally decide to short-circuit and invoke "git" directly > without the shell by looking at the first element of the argv[] > array, One less thing for human programmers to worry about. I don't have any problem with that direction. In theory we might do something special for a .git_cmd (maybe run a builtin directly?), but in 20 years of run-command we have not bothered to do anything besides prepend "git". I think we already should skip the shell in that case, as the caller would not generally be setting .use_shell. And if they are, they are perhaps trying to do something clever and we should not override it. So I think you could literally just drop .git_cmd, replace all the callers with pushing "git" at the start, and be done. -Peff