Currently, the iwyu CI fails to run in a git-worktree, or git-archive. This is due to the use of git diff.
Fix this by force-initializing a dummy git repo with a single dummy commit.
It may be possible to detect when git diff is not available in the directory, and only apply the fallback when needed, but the git history is not needed and it is easier to unconditionally apply the git init.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | willcl-ark, hebasto |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
No conflicts as of last run.
🚧 At least one of the CI tasks failed.
Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/21472570144/job/61848516389
LLM reason (✨ experimental): IWYU reported a failure in the CI run.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
43@@ -44,6 +44,19 @@ echo "=== BEGIN env ==="
44 env
45 echo "=== END env ==="
46
47+if [[ "${RUN_IWYU}" == true ]]; then
48+ # The CI framework should be flexible where it is run from. For example, from
49+ # a git-archive, a git-worktree, or a normal git repo.
50+ # The iwyu task requires a working git repo, which may not always be
51+ # available, so initialize one with force.
52+ rm -rf .git
In commit “ci: Allow running iwyu ci in worktree” (88887114b89de6642197fce8afd005172f8bc260)
This seems dangerous. It should be safe to run ci commands locally without expecting your git history to be wiped away. Would suggest a tweak like:
0# Initialize git if this is not a normal git directory (e.g. is a worktree)
1if [[ "$RUN_IWYU" == true && ! -d .git ]]; then
2 [[ -e .git || -L .git ]] && mv .git .git-previous
3 git init
4fi
I’d doubt that this script will pass at all when run locally and it is not designed to be used that way. Maybe there should be a check in the beginning, with an early abort if DANGER_RUN_CI_ON_HOST is not set?
In any case, I’ve replaced this line with mv .git .git_ci_backup || true
In the case that someone runs this script locally, might it be prudent to add a
0mv .git_ci_backup .git || true
to the end of the script to restore their .git directory to it’s original state and minimise broken worktree-related disappointments where possible?
edit: sorry, just seen yours and ryanofsky’s comments above!
edit: sorry, just seen yours and ryanofsky’s comments above!
No worries. I’ve implemented my idea from the comment now.
The bash pipes are broken. Sigh, I guess it is time to re-write this in Python.
https://github.com/bitcoin/bitcoin/actions/runs/22091425710/job/63837401348?pr=34441#step:11:2611 https://github.com/bitcoin/bitcoin/actions/runs/22091425710/job/63837401339?pr=34441#step:11:5205
edit: dropped pipefail for now. This is unrelated and can be done in a follow-up.
ACK 99997ca18520783cbcf9eba8bac1dc5785362fb9
Guarding with DANGER_RUN_CI_ON_HOST make good sense for these scripts as they modify the filesystem.
Tested locally in a worktree (but I did not test outside of a container!).
nit: tiny typo in commit message “exection”
The shell scripts are inherently unsafe, because they will install new
software packages, modify global configuration settings, write to the
root / or $HOME, and possibly modify the git repo.
The only safe way to run them is through the CI system itself, that is
the ci_exec python function.
The ci_exec funtion ensures that the user has set up a sandbox
externally and set DANGER_RUN_CI_ON_HOST=1 at their own risk, or that a
sandbox was set up with the given container_id, in which case it is safe
to set DANGER_RUN_CI_ON_HOST=1 for that sandbox.
Also, it is safe to set DANGER_RUN_CI_ON_HOST=1 when building the
sandbox image in ci/test_imagefile.
Then, the two shell scripts can reject early if unsafe execution is
detected.
nit: tiny typo in commit message “exection”
thx, fixed
reACK fafdb8f635bc157f55e23890264d12170ecd41ae
Based on range-diff (excluding rebase) of:
0src/core/bitcoin on pcp-loglevel [$?⇕] via △ v4.1.2 via 🐍 v3.13.11 via ❄️ impure (nix-shell-env)
1❯ prw upstream 34441
2From github.com:bitcoin/bitcoin
3 * branch refs/pull/34441/head -> FETCH_HEAD
4git range-diff 35e6444fdc4068adc79082648f9889ad593e623b..99997ca18520783cbcf9eba8bac1dc5785362fb9 c8c9c1e61759f689615a304254fed33cda7f895e..fafdb8f635bc157f55e23890264d12170ecd41ae
51: fa591c49a98 ! 1: fab73e213de ci: Reject unsafe exection of shell scripts
6 @@ Metadata
7 Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
8
9 ## Commit message ##
10 - ci: Reject unsafe exection of shell scripts
11 + ci: Reject unsafe execution of shell scripts
12
13 The shell scripts are inherently unsafe, because they will install new
14 software packages, modify global configuration settings, write to the
152: 99997ca1852 = 2: fafdb8f635b ci: Allow running iwyu ci in worktree
16HEAD is now at fafdb8f635b ci: Allow running iwyu ci in worktree
