Re: [PATCH 0/8] run-command: remove run_command_v_*()

["René Scharfe" <l.s.r@web.de>]

Am 29.10.2022 um 04:17 schrieb Ævar Arnfjörð Bjarmason:
>
> On Fri, Oct 28 2022, René Scharfe wrote:
>
>> Wait a second: Does it even make sense to mention the command in a die()
>> message after start_command() failed?  Unless .silent_exec_failure is
>> set, start_command() already reports it.  E.g. archive-tar.c has:
>>
>> 	if (start_command(&filter) < 0)
>> 		die_errno(_("unable to start '%s' filter"), cmd.buf);
>>
>> ... and the result looks like this upon failure:
>>
>>    $ git -c tar.tgz.command=nonsense archive --format=tgz HEAD
>>    error: cannot run nonsense: No such file or directory
>>    fatal: unable to start 'nonsense' filter: No such file or directory
>>
>> The second message is mostly redundant, but it mentions that the failed
>> command was a filter.  Probably .silent_exec_failure should be set here,
>> then the die() message is no longer redundant.  This requires args[0] to
>> be stored outside of struct child_process, though, which is already done
>> here, but may be a bit tedious in other cases.
>
> Yes, maybe these messages aren't all that useful. But note that that
> "error" is emittted by thy "#ifndef ...WINDOWS..." part of the code. See
> also the recent 255a6f91ae4 (t1800: correct test to handle Cygwin,
> 2022-09-15).

On Windows I get:

  $ git -c tar.tgz.command=nonsense archive --format=tgz HEAD
  error: cannot spawn nonsense: No such file or directory
  fatal: unable to start 'nonsense' filter: No such file or directory

>> So for start_command(), would it be a generally useful to support a
>> scenario where upon failure
>>
>> - the program terminates,
>> - but before that prints a single message,
>> - which includes the command that could not be started
>
> Isn't this just the "cannot spawn"/"cannot fork" etc. messaging from
> start_command() now?

Sure, a subset is easy, but do we need all four points?

>> - and some kind of hint why we tried to start it?

I suspect .silent_exec_failure suffices for most cases.  Testing calls
of commands that usually exist is a bit hard without source code
changes, though.  Then again: Beatifying messages that will almost
never be seen isn't probably worth it anyway.

But I'll send a patch for the archive case above.

René