[ci] Make pre-push checks safe to run in parallel - #3574
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3574 +/- ##
=======================================
Coverage 91.85% 91.85%
=======================================
Files 20 20
Lines 6093 6093
=======================================
Hits 5597 5597
Misses 496 496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Authored by an agent, posting via joshlf's account @codex review Please review the current head. Treat any earlier review as stale if it targets |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df82221d35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| TOOLCHAINS_READY=0 | ||
| } | ||
|
|
||
| bootstrap_toolchain stable |
There was a problem hiding this comment.
On a developer machine without the exact pinned stable toolchain, this call auto-installs it—and if installation is unavailable, skips every actual pre-push check—even though the backgrounded scripts do not invoke cargo-zerocopy +stable (the only descriptor used by them is +nightly in zerocopy/ci/check_fmt.sh). This makes otherwise runnable offline pushes fail and adds a substantial unnecessary toolchain/component download; bootstrap only the descriptors actually used by the checks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Authored by an agent, posting via joshlf's account
Fixed in c08a2eb. This commit now bootstraps only nightly, which is the only toolchain descriptor used before the repository-tools check is introduced. The stable bootstrap moves to the following commit, 765e70e, alongside ci/check_tools.sh, which is the first check that needs it. The hermetic tests assert the exact bootstrap sequence at each stack position. The standalone #3574 pre-push suite and the complete five-commit pre-push suite both pass.
df82221 to
c08a2eb
Compare
|
Authored by an agent, posting via joshlf's account @codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| # The pre-push hook coordinates concurrent repository checks and is itself | ||
| # production code. Exercise it with fake check scripts so this validation does | ||
| # not recursively run the real hook or depend on installed Rust toolchains. | ||
| PYTHONDONTWRITEBYTECODE=1 python3 githooks/test_pre_push.py |
There was a problem hiding this comment.
Why PYTHONDONTWRITEBYTECODE=1?
Fresh runners can start several cargo-zerocopy processes before rustup has installed the pinned nightly toolchain. Rustup shares download state between those processes, so concurrent installation can fail when one process moves another process's partial file. Install nightly before starting the checks in parallel. Preserve the hook protocol on stdin, scope automatic installation to the bootstrap command, and wait for every child. An early failure no longer discards diagnostics from checks that are still running. Also snapshot every first-party Cargo.lock before running the nominally read-only checks and reject any change. A hermetic fake-repository test covers bootstrap ordering, stdin preservation, lockfile accounting, subdirectory invocation, and child failures. *Authored by an agent, posting via joshlf's account* gherrit-pr-id: Gdqabzhpevkhxg3fzk6bvbtpws7nk2vwv
c08a2eb to
73337ca
Compare
2 participants
Fresh runners can start several cargo-zerocopy processes before rustup
has installed the pinned nightly toolchain. Rustup shares download state
between those processes, so concurrent installation can fail when one
process moves another process's partial file.
Install nightly before starting the checks in parallel. Preserve the
hook protocol on stdin, scope automatic installation to the bootstrap
command, and wait for every child. An early failure no longer discards
diagnostics from checks that are still running.
Also snapshot every first-party Cargo.lock before running the nominally
read-only checks and reject any change. A hermetic fake-repository test
covers bootstrap ordering, stdin preservation, lockfile accounting,
subdirectory invocation, and child failures.
Authored by an agent, posting via joshlf's account
Latest Update: v5 — Compare vs v4
📚 Full Patch History
Links show the diff between the row version and the column version.
⬇️ Download this PR
Branch
git fetch origin refs/heads/Gdqabzhpevkhxg3fzk6bvbtpws7nk2vwv && git checkout -b pr-Gdqabzhpevkhxg3fzk6bvbtpws7nk2vwv FETCH_HEADCheckout
git fetch origin refs/heads/Gdqabzhpevkhxg3fzk6bvbtpws7nk2vwv && git checkout FETCH_HEADCherry Pick
git fetch origin refs/heads/Gdqabzhpevkhxg3fzk6bvbtpws7nk2vwv && git cherry-pick FETCH_HEADPull
Stacked PRs enabled by GHerrit.