silent_exec_failure when calling gpg

silent_exec_failure when calling gpg

[John Passaro]

I've noticed that in v2.19.1, when using git to pretty print
information about the signature, if git cannot find gpg (e.g. "git
config gpg.program nogpg"), it prints an error to stderr:

$ git show -s --pretty=%G?
fatal: cannot run nogpg: No such file or directory
N

When I build from master, that no longer happens:

$ ../git/git show -s --pretty=%G?
N

Is this intentional behavior, i.e. something I can count on being the
case in future releases? Or should I treat this as a bug report? (I
have no opinion on whether this should be a feature or a bug, but I'm
working on a patch whose implementation may look very different based
on the answer to this question.)

I've dug around the code and come up with the notion that setting
child_process.silent_exec_failure = 1 is the usual way of suppressing
this message. But "git log -S silent_exec_failure v2.19.1..master"
shows no changes that could have caused this, nor can I find any
mention of it in release notes. It occurs for just about any
interaction with GPG that I can find, whether signing tags or commits,
or verifying signatures through pretty-print or git verify-commit or
git merge --log or git merge --verify-signatures. In all these cases,
v2.19.1 issues "fatal: cannot run nogpg: No such file or directory" to
stderr, while a binary built from master behaves the same in all
respects (including other errors when trying to sign) except that it's
missing the "fatal:" message.

This behavior makes sense in a lot of ways. If you're interested in
verifying commit signatures, it's hard to imagine needing a reminder
to install the program it depends on (though the error might help you
identify bad configuration for "gpg.program"). Conversely, if you're
not familiar with crypto software and try git log --format=%G? or git
verify-commit for fun, "Fatal: cannot run gpg" may look like a bug. So
I can definitely imagine justifications for this. Nonetheless, the
fact that I can't find documentation for the change smells funny.

I'd be very grateful if somebody on this list could tell me whether I
can count on this behavior in the future, or whether my code should
account for a possibility that this behavior could change in the future.
I'd also be very very interested to see in what commit(s) this change
occurred.

Thanks in advance!

John Passaro
(917) 678-8293

Re: silent_exec_failure when calling gpg

[Junio C Hamano]

John Passaro <john.a.passaro@gmail.com> writes:

> I've noticed that in v2.19.1, when using git to pretty print
> information about the signature, if git cannot find gpg (e.g. "git
> config gpg.program nogpg"), it prints an error to stderr:
>
> $ git show -s --pretty=%G?
> fatal: cannot run nogpg: No such file or directory
> N
>
> When I build from master, that no longer happens:
>
> $ ../git/git show -s --pretty=%G?
> N
>
> Is this intentional behavior, i.e. something I can count on being the
> case in future releases? Or should I treat this as a bug report?

In general, behaviour of Porcelain commands like "git show" can and
will be changed to match interactive use by humans, and should never
be relied on in your scripts.

Having said that, I think we did not mean to kill the diagnositic
message.

> This behavior makes sense in a lot of ways. If you're interested in
> verifying commit signatures, it's hard to imagine needing a reminder
> to install the program it depends on (though the error might help you
> identify bad configuration for "gpg.program").

Quite the contrary, from a single "N", you cannot immediately tell
if the commit is not signed, if your GPG is misconfigured, or if you
have a stray gpg.program that points at nowhere.

I think the uninteded behaviour change was in 17809a98 ("Merge
branch 'jk/run-command-notdot'", 2018-10-30).

Thanks for a report.

Re: silent_exec_failure when calling gpg

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> John Passaro <john.a.passaro@gmail.com> writes:
>
>> I've noticed that in v2.19.1, when using git to pretty print
>> information about the signature, if git cannot find gpg (e.g. "git
>> config gpg.program nogpg"), it prints an error to stderr:
>>
>> $ git show -s --pretty=%G?
>> fatal: cannot run nogpg: No such file or directory
>> N
>
> I think the uninteded behaviour change was in 17809a98 ("Merge
> branch 'jk/run-command-notdot'", 2018-10-30).

Perhaps something like this.  There needs an additional test added
for this codepath, which I haven't done yet, though.

 run-command.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/run-command.c b/run-command.c
index d679cc267c..e2bc18a083 100644
--- a/run-command.c
+++ b/run-command.c
@@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
 	if (prepare_cmd(&argv, cmd) < 0) {
 		failed_errno = errno;
 		cmd->pid = -1;
+		if (!cmd->silent_exec_failure)
+			error_errno("cannot run %s", cmd->argv[0]);
 		goto end_of_spawn;
 	}
 

Re: silent_exec_failure when calling gpg

[Jeff King]

On Tue, Dec 11, 2018 at 01:09:37PM +0900, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > John Passaro <john.a.passaro@gmail.com> writes:
> >
> >> I've noticed that in v2.19.1, when using git to pretty print
> >> information about the signature, if git cannot find gpg (e.g. "git
> >> config gpg.program nogpg"), it prints an error to stderr:
> >>
> >> $ git show -s --pretty=%G?
> >> fatal: cannot run nogpg: No such file or directory
> >> N
> >
> > I think the uninteded behaviour change was in 17809a98 ("Merge
> > branch 'jk/run-command-notdot'", 2018-10-30).
> 
> Perhaps something like this.  There needs an additional test added
> for this codepath, which I haven't done yet, though.

Thanks, both, for the report and the patch.

> diff --git a/run-command.c b/run-command.c
> index d679cc267c..e2bc18a083 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
>  	if (prepare_cmd(&argv, cmd) < 0) {
>  		failed_errno = errno;
>  		cmd->pid = -1;
> +		if (!cmd->silent_exec_failure)
> +			error_errno("cannot run %s", cmd->argv[0]);
>  		goto end_of_spawn;
>  	}

Yes, I think this is the right fix. For a test, I think we could just
do:

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c8514..866268dfd1 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -33,7 +33,8 @@ test_expect_success 'run_command is restricted to PATH' '
 	write_script should-not-run <<-\EOF &&
 	echo yikes
 	EOF
-	test_must_fail test-tool run-command run-command should-not-run
+	test_must_fail test-tool run-command run-command should-not-run 2>err &&
+	grep should-not-run err
 '
 
 test_expect_success !MINGW 'run_command can run a script without a #! line' '


I assume you'll wrap that up into a real commit?

-Peff