I ran into 3 different issues while trying to run the verify-commits script for the first time and I think documenting them would help save time for future developers.
32 | @@ -33,6 +33,17 @@ Configuration files 33 | * `trusted-keys`: This file should contain a \n-delimited list of all PGP fingerprints of authorized commit signers (primary, not subkeys). 34 | * `allow-revsig-commits`: This file should contain a \n-delimited list of git commit hashes. See next section for more info. 35 | 36 | +Import trusted keys 37 | +------------------- 38 | +In order to check the commit signatures you must add the trusted PGP keys to your machine. This can be done in Linux by running 39 | + while read -r LINE; do gpg --recv-keys $LINE; done < contrib/verify-commits/trusted-keys
Should be a code block? :-)
This is a markdown file. I suggest you surround this line with backticks to show that it's a command to type, ie:
In order to check the commit signatures you must add the trusted PGP keys to your machine. This can be done in Linux by running:
while read -r LINE; do gpg --recv-keys $LINE; done < contrib/verify-commits/trusted-keys
This also assumes that the current working directory is the repo's root directoy. I'd assume it to be the verify-commits directory (ie the command would be while read -r LINE; do gpg --recv-keys $LINE; done < trusted-keys)
Bonus points: add sh as hint for syntax highlighting
```sh
while read...
assume it to be the verify-commits directory
Agreed, but then the main command should probably also assume that. I wonder if that causes problems if it reaches old commits where this directory doesn't exist?
Here's a simpler alternative that accomplishes the same thing:
gpg --recv-keys $(<contrib/verify-commits/trusted-keys)
Changed to use backticks with the shell suggestion.
I'm ambivalent about the root directory assumption; the reason I went with this assumption was due to the command that already exists in the readme that makes this same assumption.
I'm ambivalent about the root directory assumption
Yep, I'm not really too fussed either way.
Nice, I switched to this command.
39 | + while read -r LINE; do gpg --recv-keys $LINE; done < contrib/verify-commits/trusted-keys 40 | + 41 | +Note for smartcard users 42 | +---------------------- 43 | +If you use a smartcard such as a YubiKey for GPG signing that requires a 44 | +physical button press, you will need to set gpgSign = false in your git config,
"gpgSign = false" better formatted as gpgSign = false? :-)
ACK a75edb990516eba298e1652050dbfd28a3dd0a30. One minor style nit inline.
2 | @@ -3,7 +3,7 @@ Tooling for verification of PGP signed commits 3 | 4 | This is an incomplete work in progress, but currently includes a pre-push hook 5 | script (`pre-push-hook.sh`) for maintainers to ensure that their own commits 6 | -are PGP signed (nearly always merge commits), as well as a script to verify 7 | +are PGP signed (nearly always merge commits), as well as a python 3 script to verify
Nit: upper case Python. I'm a bit surprised #!/usr/bin/env python3 at the top of that script wouldn't complain when it's run with Python 2.
I updated the capitalization. Played around with the python version support but found out that it's crashing immediately upon interpreting the script, so it's not even getting to attempting execution. Thus I don't think there's any way to improve the current state of the script without trying to rewrite all of the logging calls that cause the interpreter to blow up.
Concept ACK. See above style improvements, as it's difficult to read now.
I never tried this script before. Was able to run it based on your updated instructions (I wish it had a progress indicator though).
A more thorough fix for (3) is to add --no-gpg-sign in verify-commits.py (see docs, untested).
Further suggestions:
contrib/verify-commits
utACK 45842c3d26098ad6d12f5cb96bbcf9bcde650950
utACK 45842c3
tACK 45842c3
utACK 45842c3d26098ad6d12f5cb96bbcf9bcde650950
Milestone
0.18.0