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

Support GPG-signing commits #465

Open
arxanas opened this issue Jul 18, 2022 · 18 comments · May be fixed by #966
Open

Support GPG-signing commits #465

arxanas opened this issue Jul 18, 2022 · 18 comments · May be fixed by #966
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@arxanas
Copy link
Owner

arxanas commented Jul 18, 2022

Some resources:

There's a few APIs in git-branchless which are capable of creating commits:

These commands are capable of creating commits:

  • git move.
    • Note that it can create commits in-memory or on-disk via git rebase, so both cases will have to be handled.
    • The calling code is in core::rewrite::execute. We'll probably want to add some configuration to to indicate that any commits created in this way should be signed. Also CherryPickFastOptions.
  • git amend.
  • git reword.
  • git sync and git restack: these should be handled fairly straightforwardly by whatever changes are made to support git move.
  • git record: experimental command, so not a priority at the moment.
  • git branchless snapshot create: these commits don't need to be signed since they're not supposed to be really committed/pushed.

These data structures will probably have to be updated to carry GPG information. You should be able to add a field and then follow the compiler errors to see where should be updated to use the new field:

In the long term, it would be best to contribute GPG-signing code to https://github.com/gitext-rs/git2-ext in some capacity. Either we can contribute it directly there, or contribute it to git-branchless first and extract it afterwards.

@arxanas arxanas added the enhancement New feature or request label Jul 18, 2022
@arxanas arxanas mentioned this issue Jul 18, 2022
27 tasks
@lthms
Copy link

lthms commented Jul 23, 2022

Awesome, @arxanas. Thanks a lot for this carefully written issue.

This is probably more challenging than what I had anticipated, though I am not surprise. I will take the time to review the various links you have gathered here, and give it a try at some point.

epage added a commit to epage/git2-ext that referenced this issue Jan 5, 2023
This was done based on arxanas/git-branchless#465.

Unsure what to do about `cherry_pick` as that uses the rebase mechanism
within libgit2.
epage added a commit to epage/git2-ext that referenced this issue Jan 5, 2023
This was done based on arxanas/git-branchless#465.

Unsure what to do about `cherry_pick` as that uses the rebase mechanism
within libgit2.
epage added a commit to epage/git2-ext that referenced this issue Jan 5, 2023
This was done based on arxanas/git-branchless#465.

Unsure what to do about `cherry_pick` as that uses the rebase mechanism
within libgit2.
@epage
Copy link
Collaborator

epage commented Jan 5, 2023

FYI I just released git2-ext v0.2 which has some of the basics of signing support implemented

  • A Sign trait
  • A commit that takes Sign
  • Updated reword and amend to take Sign

The only thing I didn't touch is the cherry_pick implementation as that uses git2's rebase mechanism and I don't see a way do to sign those.

@epage
Copy link
Collaborator

epage commented Jan 5, 2023

Also, for full git compatibility, we should try to support what is documented at https://github.com/git/git/blob/4f6db706e6ad145a9bf6b26a1ca0970bed27bb72/Documentation/config/gpg.txt

@epage
Copy link
Collaborator

epage commented Jan 5, 2023

I've done another release of git2-ext that re-implements git's gpg, x509, and ssh signing behavior.

The one problem with it is when the key is in a file, we do not yet correctly handle ~/. Knew that is complex enough that I didn't want to implement it myself and didn't find a crate to do it for me that was cross-platform.

@hauleth
Copy link

hauleth commented May 9, 2023

Yeah, I have just noticed that my signed commits are missing and just found that issue. It is not super urgent for me, but I wanted to know if there is timeline for this feature?

@arxanas
Copy link
Owner Author

arxanas commented May 12, 2023

@hauleth there are no plans to work on this. You might want to try jj instead, which has a draft PR; see the issue at jj-vcs/jj#58

@hauleth
Copy link

hauleth commented May 15, 2023

@arxanas I got used to git-branchless and I do not know if I want to test another implementation. If anything I would probably look how to implement it there. I was just curious if anyone is working on that yet. It is minor issue, but I just wondered if there are any plans for adding support for signing.

@tommyip
Copy link

tommyip commented Jun 1, 2023

there are no plans to work on this

@arxanas is that a "won't fix" or just no one have time to work on it? I can try to help out if it is the second case.

@necauqua
Copy link

necauqua commented Jun 6, 2023

@tommyip I guess you can call it a "not enough intererst to put explicit effort in" i.e. other things outweigh this for main dev(s) and they don't use gpg-signing themselves so there's little motivation.

I made the draft PR for jj - and haven't touched it since as I'm kind of busy lately 🤷
But people are happy to accept such PRs if they are well-formed :)

@arxanas arxanas added the help wanted Extra attention is needed label Jun 7, 2023
@arxanas
Copy link
Owner Author

arxanas commented Jun 7, 2023

@tommyip I simply have no plans to work on it for now, but I would accept a PR implementing it.

@tommyip
Copy link

tommyip commented Jun 7, 2023

Cool my work uses GPG-signing so I have a bit more incentive to see this implemented. No promises though 🫣.

@tommyip tommyip linked a pull request Jun 14, 2023 that will close this issue
9 tasks
@lbjarre
Copy link

lbjarre commented Nov 13, 2023

Hello, this happens to be a feature I'm also really interested in since we also require signed commits.

I see that @tommyip has some work already done here. First of all, thank you for looking into it! I just wanted to check the status of your WIP, and offer my help if there is anything that is still outstanding.

@tommyip
Copy link

tommyip commented Nov 13, 2023

Hi @lbjarre. The WIP PR supports all the git-branchless subcommands (at least at the time of my last push), I have been using it daily without any issue. The PR just need some integration tests, which is a bit difficult since the existing test suite assumes the output from git is deterministic but that is not the case for cryptographic signatures.

You can pull and compile my branch before this get upstreamed.

@hauleth
Copy link

hauleth commented Nov 13, 2023

Signatures should be deterministic as well. You just need to provide some signing key as a part of test suite. @tommyip have you tested it with SSH signing as well?

@tommyip
Copy link

tommyip commented Nov 13, 2023

I don't quite remember if this is the actual problem - openpgp uses the current time as part of the signature generation, but there is no easy way to mock the timestamp.

And yes, SSH signing works.

@necauqua
Copy link

necauqua commented Nov 13, 2023

but there is no easy way to mock the timestamp

There is a hidden gpg option --faked-system-time :)
If you append an ! to the argument the clock also would stay frozen (and not tick up from the set time)

edit: and signatures are stable with it set, so that's great
in fact, would help me with implementing similar tests once I finish doing something similar for jj

@claytonrcarter
Copy link
Collaborator

there is no easy way to mock the timestamp.

I don't know the first thing about signing commits, but the test harness for this project includes a way to mock the timestamp used by git, so that commits are created w/ deterministic ids. Does the gpg signing use the same timestamp at git?

Here's the options struct: https://github.com/arxanas/git-branchless/blob/master/git-branchless-lib/src/testing.rs#L71-L74

And here's a usage example: https://github.com/arxanas/git-branchless/blob/master/git-branchless-smartlog/tests/test_smartlog.rs#L119C11-L125

@augustebaum
Copy link

I'd really love this feature as my work now requires signing commits!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants