ci: Re-enable verify-commits.py check #22744

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2108-ciVerifyCommits changing 2 files +12 −5

MarcoFalke commented at 11:06 AM on August 19, 2021: member

Might be useful to detect bugs in the script itself or an accidentally missed signature.
fanquake added the label Tests on Aug 19, 2021
MarcoFalke commented at 11:12 AM on August 19, 2021: member

To clarify: There is currently no Cirrus CI cron job running. Previously this was run on Travis every day with the last two days worth of commits. Now it runs on every push on the last 10 commits.
DrahtBot commented at 5:32 AM on August 20, 2021: member



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.
DrahtBot added the label Needs rebase on Aug 20, 2021
MarcoFalke force-pushed on Aug 20, 2021
DrahtBot removed the label Needs rebase on Aug 20, 2021
in ci/lint/06_script.sh:32 in 555528c263 outdated
```
  29 | +    # Sanity check only the last few commits to get notified of missing sigs,
  30 | +    # missing keys, or expired keys. Usually there is only one new merge commit
  31 | +    # per push on the master branch and a few commits on release branches, so
  32 | +    # sanity checking only a few commits seems sufficient and cheap.
  33 | +    git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit
  34 | +    git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-git-root
```
josibake commented at 2:38 PM on August 23, 2021:

just curious, why is 10 the magic number here? or does it not matter?

MarcoFalke commented at 2:59 PM on August 23, 2021:

It is explained in the comment above. (10 is "few")

josibake commented at 3:41 PM on August 23, 2021:

not important, but perhaps change the comment to "sanity checking the last 10 commits seems sufficient and cheap." i read the comment and was then surprised to see 10 (which is why i initially commented "why 10").

feel free to ignore, tho. could just be my brain. a few to me means "greater than one but not many"

MarcoFalke commented at 10:04 AM on August 26, 2021:

Added few (10) to the comment
josibake commented at 2:38 PM on August 23, 2021: member

Concept ACK
Zero-1729 commented at 6:57 PM on August 23, 2021: contributor

Concept ACK
ci: Unconditionally set the global git author name in cirrys.yml
The author is also needed for the verify-commits.py script.
fa880b10d6
ci: Re-enable verify-commits.py check fa001602cd
MarcoFalke force-pushed on Aug 26, 2021

josibake commented at 12:01 PM on August 27, 2021: member

ACK https://github.com/bitcoin/bitcoin/pull/22744/commits/fa001602cd5ac61b9258e998ee2b236688c19ef7

not sure if there is a better way to test this, but I rebased the branch on master, git reset upstream/master, set the CIRRUS env variables and ran ./ci/lint/06_script.sh

verified everything worked as expected:

Success: no issues found in 213 source files
Skipping shell linting since shellcheck is not installed.
gpg: key 410108112E7EA81F: "Hennadii Stepanov (hebasto) <hebasto@gmail.com>" not changed
gpg: key 944D35F9AC3DB76A: "Michael Ford (bitcoin-otc) <fanquake@gmail.com>" not changed
gpg: key D300116E1C875A3D: "MeshCollider <dobsonsa68@gmail.com>" not changed
gpg: key 3648A882F4316B9B: "Marco Falke <marco.falke@tum.de>" not changed
gpg: key 29D4BCB6416F53EC: "Jonas Schnelli <dev@jonasschnelli.ch>" not changed
gpg: key 860FEB804E669320: "Pieter Wuille <pieter@wuille.net>" not changed
gpg: key 74810B012346C9A6: "Wladimir J. van der Laan <laanwj@visucore.com>" not changed
gpg: Total number processed: 7
gpg:              unchanged: 7
Using verify-commits data from /home/josibake/bitcoin/contrib/verify-commits
There is a valid path from "HEAD" to 84be9a89c1a49a7be5bc43d832b4cb9cbd09fce3 where all commits are signed!

MarcoFalke commented at 12:09 PM on August 27, 2021: member

not sure if there is a better way to test this

Easiest way would be to just merge it :grimacing:
josibake commented at 12:25 PM on August 27, 2021: member

just merge it

:cowboy_hat_face:
Zero-1729 commented at 12:39 PM on August 27, 2021: contributor

tACK fa001602cd5ac61b9258e998ee2b236688c19ef7

Followed @josibake's steps and checked that everything still works as expected 🥤
josibake approved
fanquake approved
fanquake commented at 12:48 PM on August 31, 2021: member

untested ACK fa001602cd5ac61b9258e998ee2b236688c19ef7
fanquake merged this on Aug 31, 2021
fanquake closed this on Aug 31, 2021
MarcoFalke deleted the branch on Aug 31, 2021
sidhujag referenced this in commit 58c3ce0c0a on Aug 31, 2021
DrahtBot locked this on Aug 31, 2022

Contributors

Labels

ci: Re-enable verify-commits.py check #22744

Conflicts