Am 20.12.2016 um 21:49 schrieb Stefan Beller:
On Tue, Dec 20, 2016 at 12:12 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:
I have to ask, though: Is it so much necessary to report the exact command
line in an error message?
Have a look at https://github.com/git/git/blob/master/submodule.c#L1122
die("Could not run 'git status --porcelain -uall \
--ignore-submodules=none' in submodule %s", path);
There are 2 things:
1) The error message is not very informative, as your question hints at.
I consider changing it to add more meaning. I think the end user
would also be interested in "Why did we run this command?".
You don't have to. start_command() and finish_command() report any
low-level errors (exec failed, signal received...). If the exit code of
the program is non-zero, finish_command() reports nothing because the
command *itself* will have written some error message ("working
directory dirty", "object database corrupt", "xyz: no such file or
directory"...). At this level, it only makes sense to leave a trace in
which submodule an error occured. So I think it would be sufficient to just
die("could not run 'git status' in submodule %s", path);
or
die("'git status' in submodule %s failed", path);
2) We really want to report the correct command line here.
Why? We do not do this anywhere else.
Currently that is the case, but if you look at the prior patch that extends
the arguments conditionally (and then also uses the same condition
to format the error message... that hints at hard to maintain error
messages.)
This does not explain why the *complete and detailed* invocation must be
reported. I haven't followed this topic at all, so I may be missing some
cruical detail. (If you say "it must happen" one more time, then I will
believe you, because for me that's simpler than to plough through a
flock of submodule topics. ;-)
-- Hannes