bitcoin
bitcoin copied to clipboard
ci: run in worktrees
Fixes: #30028
- Run CI in worktrees by mounting the .git root target into the container
- Run the linter in worktrees by mounting the .git root target into the container
AFAIK mounting the git root should not present any additional security issues, as this would already be mounted (in RO mode) when not using a worktree.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31787.
Reviews
See the guideline for information on the review process. A summary of reviews will appear here.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #31948 (ci: [lint] Use Cirrus dockerfile cache by maflcko)
- #31349 (ci: detect outbound internet traffic generated while running tests by vasild)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Is this still meant to be a draft?
For reference, I can't review this, because I don't know if all the bash refactors/behavior changes here are correct and best practise, or if they could fail (and when) on error or on ancient versions of bash used in macOS.
🐙 This pull request conflicts with the target branch and needs rebase.
@maflcko thinking more on this after the merge of #31948 my opinion is that we should close this PR, the associated issue (#30028), and, if any action should be take it would be to document that linting in a worktree is possible by running the linter outside of a container? e.g.:
# install required python dependencies
bash -c '( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && RUST_BACKTRACE=1 cargo run )'
This change feels like it adds a lot of complexity, for little benefit (and I can't think of a simpler way to achieve the functionality desired).
What do you think?
For the lint task, the docker wrapper is nice, because it takes care of installing all the exact versions: https://github.com/bitcoin/bitcoin/blob/e568c1dd134e0318c46113cb7dfc23b40e2829e8/ci/lint/04_install.sh#L49-L65
However, given the limited feedback, I wonder if anyone cares about running the linters in a worktree.
For the "real" CI tasks, at least on person complained on IRC IIRC, so at least there seems to be some demand.
⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
Closing due to lack of interest.
Looks like this was closed, so I opened a trivial line-neutral +4-4 fix in https://github.com/bitcoin/bitcoin/pull/32767
(Missing lint and tidy support, but this can be done in a follow-up, if needed)