Skip to content

feat(reword): support gpg signing for non-HEAD commits#2959

Open
guerinoni wants to merge 2 commits into
gitui-org:masterfrom
guerinoni:reword-gpg
Open

feat(reword): support gpg signing for non-HEAD commits#2959
guerinoni wants to merge 2 commits into
gitui-org:masterfrom
guerinoni:reword-gpg

Conversation

@guerinoni

@guerinoni guerinoni commented May 26, 2026

Copy link
Copy Markdown

Rewording any commit older than HEAD used to fail with
SignRewordNonLastCommit as soon as commit.gpgsign was enabled, because
git2's rebase API has no way to sign the commits it produces. The
behaviour matched neither git CLI nor user expectation, so anyone
relying on signing had to drop down to the shell to fix a typo a few
commits back.

This Pull Request fixes/closes #{issue_num}.

It changes the following:

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

@guerinoni guerinoni force-pushed the reword-gpg branch 3 times, most recently from 439cc80 to 2314d93 Compare May 26, 2026 13:08
Rewording any commit older than HEAD used to fail with
SignRewordNonLastCommit as soon as commit.gpgsign was enabled, because
git2's rebase API has no way to sign the commits it produces. The
behaviour matched neither git CLI nor user expectation, so anyone
relying on signing had to drop down to the shell to fix a typo a few
commits back.
@extrawurst extrawurst added this to the v0.29 milestone Jun 29, 2026
return Err(Error::SignRewordLastCommitStaged);
}

return Err(Error::SignRewordNonLastCommit);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lets make sure the new call to reword_signed just as the below call to reword_internal (likely better now call it reword_unsigned) feeds both into the same Result match that will rollback a failed attempt

) -> Result<Oid> {
use crate::sync::sign::SignError;

let buffer = repo.commit_create_buffer(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this block is almost identical to what we do in commit.rs create_signed_commit - lets extract a shared method for this to simplify maintanance

return Err(Error::SignRewordLastCommitStaged);
}

return Err(Error::SignRewordNonLastCommit);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SignRewordNonLastCommit is not used anymore now, right? lets remove


// collect commits from HEAD down to (and including) target.
let mut chain = Vec::new();
let mut cur = repo.find_commit(head_oid)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

isn't that just repo.head() no need for the head_oid loop around, right?

@extrawurst extrawurst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for helping with this. nothing major, mostly nits

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants