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

Add --gpg-sign to git commit calls #29

Merged
merged 9 commits into from
Apr 30, 2022
Merged

Add --gpg-sign to git commit calls #29

merged 9 commits into from
Apr 30, 2022

Conversation

mattwynne
Copy link
Member

@mattwynne mattwynne commented Apr 29, 2022

This is awkward to test as it integrates with gpg tooling and key database.

🤔 What's changed?

  • Add --gpg-sign to all git commit commands (except in tests)
  • Add an indicator to the git log output used in approval tests.
  • Added a GPG key for the tests to use (as user@example.com), which is installed when the tests run.

⚡️ What's your motivation?

We want all commits to be signed.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

  • If you run the tests locally, this will leave a user@example.com key in your gpg key list. I can't work how to delete this non-interactively (my pintentry fires up even if I use gpg --delete-secret-keys in --batch mode. Is this OK?
  • Maybe there's a way to pass a key to git on the fly instead of cluttering up the global gpg key database? If there is I can't find one.

📋 Checklist:


This text was originally generated from a template, then edited by hand. You can modify the template here.

This is awkward to test reliably as git looks at the global config
(~/.gitconfig) of the user running the tests, and will pick up
the commit.gpgSign setting from there.

Hopefully this will work in CI, but we'll see.
@mattwynne
Copy link
Member Author

mattwynne commented Apr 29, 2022

of course, the WoMM problem here is that we don't have any GPG keys in the CI environment

need to either not test this feature somehow (make it an option?) or figure out how to set up GPG keys in CI.

@mattwynne mattwynne marked this pull request as ready for review April 29, 2022 17:02
@mpkorstanje
Copy link
Contributor

Would aliasing the gpg command to use a different home directory while testing work? E.g:

alias gpg="gpg --homedir=/c/Users/s156757/.gnupg"

From:

https://stackoverflow.com/questions/46863981/how-to-sign-git-commits-from-within-an-ide-like-intellij/46884134#46884134

@mattwynne
Copy link
Member Author

Great idea! I'll take a look.

@mattwynne
Copy link
Member Author

Sweet, I also discovered https://stackoverflow.com/questions/55576302/how-do-i-pass-homedir-to-git-when-signing-using-gpg which helps to convince git to use the right gpg home directory. Easier that writing shell script wrappers.

polyglot-release Outdated Show resolved Hide resolved
I used a step in the actual `release.sh` test script to verify the tag was signed.
Seems simpler than any alternative I can think of.
@mattwynne
Copy link
Member Author

Do you think we should have a pre-flight check for GPG keys? I'm not sure how clear the feedback will be if you don't have them set up.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Looks good!

@mpkorstanje
Copy link
Contributor

Do you think we should have a pre-flight check for GPG keys?

Would be nice. We don't have it documented that you should set it up so it's either writing a check or documentation.

@mattwynne mattwynne merged commit a6932c6 into main Apr 30, 2022
@mattwynne mattwynne deleted the sign-commits branch April 30, 2022 21:54
@mattwynne mattwynne mentioned this pull request Apr 30, 2022
7 tasks
@mpkorstanje mpkorstanje added this to the v1 milestone May 14, 2022
This pull request was closed.
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