Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make GPG errors less puzzling #1480

Closed
wants to merge 2 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 14, 2023

For one times too many, I was asked to help with a commit signing problem where the only error message was an unhelpful:

error: gpg failed to sign the data
fatal: failed to write commit object

That was it. No further indication what went wrong. And certainly not that wonderful error message that the helper that was configured as gpg.program wrote to its stderr.

Let's show whatever GPG (or any alternative configured via gpg.program) had to say when signing failed.

This test case not only increases test coverage in setups without
working gpg, but also prepares for verifying that the error message of
`gpg.program` is shown upon failure.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
There are few things more frustrating when signing a commit fails than
reading a terse "error: gpg failed to sign the data" message followed by
the unsurprising "fatal: failed to write commit object" message.

In many cases where signing a commit or tag fails, `gpg` actually said
something helpful, on its stderr, and Git even consumed that, but then
keeps mum about it.

Teach Git to stop withholding that rather important information.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho self-assigned this Feb 14, 2023
@dscho
Copy link
Member Author

dscho commented Feb 15, 2023

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 15, 2023

Submitted as pull.1480.git.1676440714.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1480/dscho/make-gpg-errors-less-puzzling-v1

To fetch this version to local tag pr-1480/dscho/make-gpg-errors-less-puzzling-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1480/dscho/make-gpg-errors-less-puzzling-v1

@@ -977,9 +977,13 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
break; /* found */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  	ret |= !cp;
> +	if (ret) {
> +		error(_("gpg failed to sign the data:\n%s"),
> +		      gpg_status.len ? gpg_status.buf : "(no gpg output)");
> +		strbuf_release(&gpg_status);
> +		return -1;
> +	}
>  	strbuf_release(&gpg_status);
> -	if (ret)
> -		return error(_("gpg failed to sign the data"));

Good.  As we are worried about error messages that are too terse,
dumping everything to the output would be a vast improvement.
Hopefully gpg_status.len would to be thousands of bytes long, and
this is not a codepath that is triggered remotely anyway.

Will queue.  Thanks.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 15, 2023

This branch is now known as js/gpg-errors.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 15, 2023

This patch series was integrated into seen via git@26c4ad1.

@gitgitgadget gitgitgadget bot added the seen label Feb 15, 2023
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 17, 2023

This patch series was integrated into seen via git@006a6ed.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 17, 2023

This patch series was integrated into next via git@510898f.

@gitgitgadget gitgitgadget bot added the next label Feb 17, 2023
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 17, 2023

This patch series was integrated into seen via git@e10923d.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 22, 2023

This patch series was integrated into seen via git@ac463d6.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2023

There was a status update in the "New Topics" section about the branch js/gpg-errors on the Git mailing list:

Error messages given upon a signature verification failure used to
discard the errors from underlying gpg program, which has been
corrected.

Will merge to 'master'.
source: <pull.1480.git.1676440714.gitgitgadget@gmail.com>

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 24, 2023

This patch series was integrated into seen via git@4180fb8.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 24, 2023

This patch series was integrated into seen via git@38a227b.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 24, 2023

This patch series was integrated into master via git@38a227b.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 24, 2023

This patch series was integrated into next via git@38a227b.

@gitgitgadget gitgitgadget bot added the master label Feb 24, 2023
@gitgitgadget gitgitgadget bot closed this Feb 24, 2023
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 24, 2023

Closed via 38a227b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant