gitui icon indicating copy to clipboard operation
gitui copied to clipboard

Amending a signed commit overwrites author

Open ItsHarper opened this issue 1 month ago • 17 comments

Describe the bug

Amending a signed commit overwrites the author (bad) and the committer (good).

Amending an unsigned commit does not overwrite the author (good), but it also does not overwrite the committer (bad).

To Reproduce

  1. Check out a branch where the latest commit was authored and committed by someone else
  2. Run git log --pretty=full -n 1 and verify that the Author field shows someone other than you
  3. Stage changes
  4. Open the commit dialog
  5. Press Ctrl-A to make the pending commit be an amend operation
  6. Press Ctrl-D to finish the commit
  7. Run git log --pretty=full -n 1 again
  8. Observe that either the author or committer is wrong (depending on whether the commit you amended was signed)

Expected behavior

Amending a commit should overwrite the committer, but not the author.

Context

  • OS: Chimera Linux (updated as of 2025-11-20)
  • GitUI Version: 0.27.0
  • Rust version: 1.91.0

ItsHarper avatar Nov 20 '25 21:11 ItsHarper

I’ll have a look!

cruessler avatar Nov 22 '25 18:11 cruessler

Currently, this seems to be somewhat intentional as the code for amending a commit passes None for committer which results in the committer being unchanged. I suggest we change that and override the committer with the current user, that is whatever is returned by signature_allow_undefined_name (which usually is git config --get user.name/email). I’ll prepare a PR soon!

https://github.com/gitui-org/gitui/blob/37d3dd291a6eb91f55d8e8091c376b10ee11d33f/asyncgit/src/sync/commit.rs#L43-L50

cruessler avatar Nov 27 '25 07:11 cruessler

seems to be somewhat intentional [..] results in the committer being unchanged

isnt that exactly the opposite of what the bug report states?

extrawurst avatar Nov 27 '25 12:11 extrawurst

seems to be somewhat intentional [..] results in the committer being unchanged

isnt that exactly the opposite of what the bug report states?

You’re absolutely right! Then I can’t seem to reproduce as the author is not overwritten if I do an amend using gitui. (This observation made me think we should probably change the committer if we amend, but now I’m not so sure anymore.) I can also try and check on a different machine to see whether I’m missing anything.

cruessler avatar Nov 27 '25 13:11 cruessler

This observation made me think we should probably change the committer if we amend, but now I’m not so sure anymore.

My understanding is that the committer should change, yeah

Let me double-check I can reproduce myself.

ItsHarper avatar Nov 27 '25 16:11 ItsHarper

@ItsHarper

My understanding is that the committer should change, yeah

But your bug report says:

Observe that you are now listed as the commit author

So what is it ? 🙈

extrawurst avatar Nov 27 '25 16:11 extrawurst

@ItsHarper

My understanding is that the committer should change, yeah

But your bug report says:

Observe that you are now listed as the commit author

So what is it ? 🙈

"Committer" and "Author" are two different things. The "Author" is supposed to be the original author of the commit. The "Committer" is the last person who modified the commit in some way (amend, rebase, etc).

ItsHarper avatar Nov 27 '25 16:11 ItsHarper

Screenshot demonstrating the correct behavior

The committer changes, but the author does not

Image

(I meant to have that first line be git config set user.name me for clarity; setting log.showSignature is irrelevant)

Screenshot demonstrating the incorrect behavior

These commands were performed immediately after the previous screenshot was taken. When I ran gitui, I amended the commit and did nothing else. The author has erroneously been changed.

Image

ItsHarper avatar Nov 27 '25 17:11 ItsHarper

In case you're looking at email notifications, I've edited my last comment to have screenshots that demonstrate both the correct git behavior and the incorrect gitui behavior (it does reproduce for me).

ItsHarper avatar Nov 27 '25 17:11 ItsHarper

the code for amending a commit passes None for committer which results in the committer being unchanged.

~~When in doubt, read the docs:~~

This creates a new commit that is exactly the same as the old commit, except that any non-None values will be updated. (emphasis mine)

~~You have to explicitly provide the original author to amend().~~

ItsHarper avatar Nov 27 '25 17:11 ItsHarper

Wait, I think the docs agree with you. Quite frankly, I'm not sure how I was reading it previously...

Is this a bug in my distribution's version of libgit2? Chimera Linux packages 1.9.1, the latest stable release.

ItsHarper avatar Nov 27 '25 18:11 ItsHarper

OK, I figured out why you couldn't reproduce it. The behavior is different depending on whether you're amending a signed commit.

When amending a signed commit, both author and committer are overwritten.

When amending an unsigned commit, neither author nor committer are overwritten.

Both behaviors are wrong, just in opposite directions. I've edited the issue description.

ItsHarper avatar Nov 27 '25 19:11 ItsHarper

Oh interesting. Makes sense. the signed amend is a hack because there is no such thing in libgit2. so we undo the last commit and commit again but then it overwrite both author and committer. So we need to preserve the original author explicitly here. The other issue is the committer not being set in the actual amend call.

@cruessler @Byron gix support for signed amend ? :)

extrawurst avatar Nov 27 '25 19:11 extrawurst

Actually it does, kind-a :D. The code is currently (and originated) in GitButler. It's a very faithful and by now proven implementation, but I didn't get around to porting it over yet.

If that was the case, signing commits in general is easy, and the same is true for amends.

Byron avatar Nov 28 '25 06:11 Byron

There’s also this addition to gitoxide https://github.com/GitoxideLabs/gitoxide/commit/aacc326a198e4c78dd546018eebaff192ff5223d that does a subset of what GitButler does. I’m interested in adding more comprehensive signing support to gitoxide, but I don’t know whether it’s on anyone else’s TODO list already. :-)

cruessler avatar Nov 28 '25 07:11 cruessler

It's on my TODO list, but that's the case for everything gitoxide, your help would be greatly appreciated 🙏.

Byron avatar Nov 28 '25 07:11 Byron

I just had a brief look at the code and think it should not be too difficult to address the second part of this issue. I’ll try to create a PR over the course of the next couple of days.

cruessler avatar Dec 02 '25 07:12 cruessler