Commit signing backend implementation #3007
Conversation
julienvincent
changed the title
julienvincent
force-pushed
the
jv/signing-backend
branch
from
58b4ed3 to
4427cdf
Compare
julienvincent
force-pushed
the
jv/signing-backend
branch
2 times, most recently
from
77921f6 to
c05e6a1
Compare
julienvincent
force-pushed
the
jv/signing-backend
branch
15 times, most recently
from
95f1361 to
4627b2a
Compare
|
Wow it was an absolute nightmare to get the tests passing on Windows. I even have access to a windows machine and this was like pulling teeth. So glad that's actually working now. Surely nix can't be as bad. Right? |
julienvincent
force-pushed
the
jv/signing-backend
branch
5 times, most recently
from
bbe63c3 to
c9b720f
Compare
|
@martinvonz I think this PR should be ready for an initial review. I've managed to get all the tests passing and it seems to function well in my personal use of it. Would be great if I can get some eyes on it to see if I am missing anything or any changes are still required. Let me know! |
julienvincent
force-pushed
the
jv/signing-backend
branch
2 times, most recently
from
fda7fb1 to
b09d5ae
Compare
|
Ok, everything addressed! Let me know if you need anything more from me. |
julienvincent
force-pushed
the
jv/signing-backend
branch
from
b09d5ae to
b5f4576
Compare
|
Anything needed to merge this? I'm not sure what the process is |
Once it's approved and you haven't made any non-trivial changes since the last round of reviews, feel free to merge. Thank for all your work on this! And thanks again to @necauqua too! |
|
Ok but I don't have permission to merge. Does someone else need to do it? |
Oops, sorry! Now you do (once you accept the invite). |
|
Oh! Ok awesome. I will merge this now :)
Thanks for all the help getting this ready. |
Yes, thanks, @yuja! :) |
|
FYI, this adds a dev dependency on |
| Self::new( | ||
| config | ||
| .get_string("signing.backends.gpg.program") | ||
| .unwrap_or_else(|_| "gpg2".into()) |
There was a problem hiding this comment.
@necauqua maybe the default should be "gpg"?
(just spotted because the test uses "gpg" not "gpg2")
There was a problem hiding this comment.
I think the gpg2 command is the one that requires the agent to work?.
(and also gpg 2.x)
So that agent use is enforced since we sign every snapshot and whatnot
Anyway I don't remember there being any other reasons, so could be gpg, yes, maybe I forgot to get back to think about this
There was a problem hiding this comment.
Perhaps, it's historical thing? I don't have gpg2 in my Debian unstable environment, and gpg --version says it's 2.x.
https://packages.debian.org/bookworm/gnupg2
There was a problem hiding this comment.
You don't?.
I thought it was a symlink that every gpg 2.x also installs or something
I have it on NixOS
Yes it is a historical thing afaiu
Anyway I've no objections to changing the default to just gpg
Any unusual or historical setups could change the config value anyway
| data, | ||
| &[ | ||
| "--status-fd=1".as_ref(), | ||
| "--verify".as_ref(), |
There was a problem hiding this comment.
@yuja but then here we'd also need to not forget to add --keyid-format=long which is the default in gpg2
(I just noticed that reading a commit message where they added that very argument to gpg call in git; I think I have it in my personal gpg config set to be the default too)
…ingly This is the default of Git, and Debian sid doesn't install the gpg2 symlink by default. https://github.com/git/git/blob/v2.43.2/gpg-interface.c#L92 jj-vcs#3007 (comment) https://packages.debian.org/bookworm/gnupg2
…ingly This is the default of Git, and Debian sid doesn't install the gpg2 symlink by default. https://github.com/git/git/blob/v2.43.2/gpg-interface.c#L92 #3007 (comment) https://packages.debian.org/bookworm/gnupg2
|
Implements #58, if I'm not mistaken. |
This is an expansion on the work that was started by @necauqua in #2728 with the intent of getting the core implementation ready to land.
This involved some small behavioural tweaks such as adding config to control whether or not commit signatures are verified and displayed by default. This also involved a lot of massaging to get the test suite green again.
One significant addition to the original work is the addition of an SSH signing backend which works as closely as possible to the git SSH signing workflow.
Checklist
If applicable:
CHANGELOG.md