Re: [PATCH v4] [GSOC]trailer: pass arg as positional parameter

From: ZheNing Hu <adlternative@gmail.com>
Date: Apr 1, 2021, 03:39 UTC
Message-ID: <CAOLTT8Tn9E5qo-mv_o9-r+zQ0-FT6vOH3BZDLuXpACjPscMqeQ@mail.gmail.com>
In-Reply-To: <xmqqeefv89tc.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2021年4月1日周四 上午2:20写道：

Show 24 quoted lines

>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > The configuration is like this:
> > trailer.bug.key=BUG:
> > trailer.bug.ifexists=add
> > trailer.bug.cmd=echo "123"
> >
> > And use:
> >
> > $ git interpret-trailers --trailer="bug:456" --trailer="bug:789"<<-EOF
> > EOF
> >
> > BUG: 123
> > BUG: 123 456
> > BUG: 123 789
>
> I think that is quite expected.  You said the command to run is
> 'echo 123', and that is not "pick a directory $D on $PATH where
> there is an executable '$D/echo 123' exists, and run that".  It
> runs the given command with the shell, and in general that is
> what we want for end-user supplied commands specified in the
> configuration file [*1*].
>

I agree that if you want to use execv directly to execute a terminal command, if arg[0] is something like "emacs -nw", error will be reported:"No such file or directory". But by wrapping a layer of "sh" "-c", The program name 'emacs' in 'emacs -nw' can be found and executed normally.

Show 10 quoted lines

> So we form a shell command whose beginning is 'echo 123' and tuck
> the argument after that command line, so it is understandable that
> "echo 123 456" gets executed for "--trailer=bug:456".
>
> I wasn't following the discussion between you and Christian closely
> but I recall seeing him saying that the command is executed one
> extra time without any arg before it is run for actual --trailer
> requests with the value?  I am guessing that is where the first
> output "BUG: 123" (without anything else) is coming from.
>

Exactly. Each .command/.cmd willl executes this 'beforeCLI' operation once, I use ifexists='add' here just for seeing the effect, In general, we will use ifexists='replace'.

Show 16 quoted lines

>
> *1* Imagine .editor set to 'emacs -nw' or 'vim -f'; we do not want
>     Git to find a directory on $PATH that has an executable whose
>     name is 'emacs -nw' and run that file (i.e. give 'emacs -nw' as
>     the first argument to execlp()).  Instead, you'd want to behave
>     as if the user typed "emacs -nw", followed by any arguments we
>     want to give to it (in .editor's case, the name of the file to
>     be edited) properly quoted for the shell.
>
>     And the way we do so is to form a moral equivalent of
>
>         execlp("sh", "-c", "emacs -nw $@", ...);
>
>     and put the arguments at the end where I wrote ... (we actually
>     do so with execvp(), but illustrating with execlp() is easier to
>     read and write---hence "a moral equivalent of").

I can see the benefits of this.