#35686 lint: have git-subtree-check check for backportability
https://github.com/bitcoin/bitcoin/pull/35686 · · +75/-20 in 4 files, 2 commits · labels: Tests
Goal
- Check that subtree updates stack cleanly on previous subtree merges
- Let maintainers backport subtree fixes verbatim without redundant reviews
Updates `test/lint/git-subtree-check.sh` and `test/lint/test_runner/src/lint_repo_hygiene.rs` to verify that the most recent subtree merge commit reachable from a commit is stacked on the previous subtree merge commit. Also adds an `--incompatible` flag to bypass the check when API breaks require a newer base commit, and documents the practice in `doc/developer-notes.md`.
Problem: When subtree updates are based on arbitrary recent commits on `master` rather than previous subtree merges, backports to release branches cannot reuse the merge commit hash verbatim and require redundant re-review.
Category: Test infrastructure (#33 of 45)
P4 · cleanup
- P4 because it is a narrow workflow enhancement for subtree updates
- Affects only maintainers updating external subtrees rather than general CI runs
This is a narrow workflow enhancement for maintainers updating external subtrees. The check is bypassed in CI via `--incompatible` and is only run manually when subtrees are updated, making its overall impact on testing infrastructure marginal.
Membership: Modifies test/lint/git-subtree-check.sh and test/lint/test_runner/src/lint_repo_hygiene.rs.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 0
Reviewability: Ready
- Ready to review
- CI is passing and earlier review comments are addressed
The PR has resolved previous feedback, has passing CI, and has received positive review without pending author actions.
Author status: active
Resolved concerns:
- sedited and maflcko questioned whether backports could just use separate merge commits or scripted diffs; Sjors and ryanofsky clarified the reviewer time savings when backporting complex multi-branch fixes (e.g. libmultiprocess).
- ryanofsky suggested replacing vague warnings with an error, handling `--incompatible`, and improving option parsing, which Sjors adopted.
Agreement: Strong
- Strong support with earlier questions about backport necessity resolved
- Approved because documenting and enforcing this saves reviewer effort (ryanofsky)
- Verified by testing passing and failing test cases (BrandonOdiwuor)
Strong: approved by ryanofsky and tested ACK from BrandonOdiwuor with earlier questions resolved
The rationale for identical merge hashes during backports was accepted, and both ryanofsky and BrandonOdiwuor approved the implementation after testing.
- ryanofsky approved commit 5b8973e: 'The main benefit I see to this PR is just documenting the practice of basing new subtrees updates on previous updates... saving reviewer effort.'
- BrandonOdiwuor ACKed commit 5b8973e after verifying passing and failing test cases.
Review verdicts (DrahtBot): 2
- ACK: ryanofsky, BrandonOdiwuor
Files
86 lines under test/bench/ci.
- test/lint/git-subtree-check.sh +51/-16
- test/lint/README.md +9/-3
- doc/developer-notes.md +9/-0
- test/lint/test_runner/src/lint_repo_hygiene.rs +6/-1
Card
Adds a backportability check to git-subtree-check.sh that verifies subtree merge commits are stacked on the previous subtree merge commit unless --incompatible is passed. This saves reviewer effort during backports by enabling identical commit hashes to be merged across release branches. Subtree updates are infrequent and CI bypasses the check, making this a low-priority tooling convenience. The PR is ready and has strong support from ryanofsky and BrandonOdiwuor.