fix(ci): make the update-constraints pre-commit hook work in CI - #6505
Open
DABH wants to merge 3 commits into
Open
fix(ci): make the update-constraints pre-commit hook work in CI#6505DABH wants to merge 3 commits into
DABH wants to merge 3 commits into
Conversation
The Pre-commit Linter job has failed on every run since the update-constraints hook landed, keeping main red: - The job never installs uv, so the hook died with 'uv: command not found' (masked as 'Resolution failed' by the script's error handling). - The hook ran the script in update mode, which recomputes the --exclude-newer date daily and exits 1 whenever it rewrites a file, so CI could not stay green even with uv installed. - The constraints-<ver>.txt files the hook manages were never committed, so check mode had nothing to validate. Fix accordingly: - Install uv in the Pre-commit Linter job, using the same pinned setup-uv action the type-check and test jobs already use. - Run the hook with --check, the script's documented CI mode: it pins the exclude-newer date from the committed file header, so validation is deterministic day over day. Developers still run the script directly to update pins. - Fail fast with a clear error when uv is not on PATH. - Commit the five generated constraints files. They are the fixed point of the script's own regeneration (including uv's 'via' annotations), so --check passes idempotently; reviewers can regenerate with ./scripts/update_constraints.sh to verify. - Skip generated constraints files in codespell (it flags the astroid package name as a typo). - Set persist-credentials: false on this workflow's checkout steps so the zizmor gate stays clean when the file is rescanned (its four pre-existing artipacked findings otherwise fail fork-PR runs, which scan with --no-ignores).
DABH
force-pushed
the
fix-precommit-constraints
branch
from
July 28, 2026 18:43
574533f to
f0a41ee
Compare
Contributor
Author
|
Hi @DeanChensj , this is related to my #6468 PR where I was noticing unexpected CI failures. However, this is now impacting everyone's PRs and |
Contributor
Author
7 tasks
copybara-service Bot
pushed a commit
that referenced
this pull request
Aug 9, 2026
The `update-constraints` hook ran `./scripts/update_constraints.sh` in update mode, which recomputes `--exclude-newer` as `today - 4 days` and rewrites the header of every `constraints-3.*.txt` file. GitHub Actions runs `pre-commit run --all-files`, so the hook fired on every change and the recomputed date almost never matched the date baked into the checked-in files. Every run ended in "files were modified by this hook". The rewrite carried no content: reproducing it locally changed 5 files by 5 lines, all of them the `--exclude-newer 2026-07-24` -> `--exclude-newer 2026-08-05` comment in the header. No package pin moved. Point the hook at `--check` instead. Check mode extracts the `--exclude-newer` date already recorded in each constraints file and regenerates with that same date, so it is stable across runs and never writes to the working tree. It still validates content: adding a dependency to `pyproject.toml` without regenerating the constraints files makes the hook fail with the missing pin in the diff, and a resolution that cannot be satisfied under the recorded date fails too. Only pure date drift stops being reported. Refreshing the pins is now an explicit action: run `./scripts/update_constraints.sh` without `--check` and commit the result. The hook's own failure message already prints that command. The constraints files are deliberately not regenerated here. That would be a two-week dependency bump unrelated to the CI break, and it would not fix the non-idempotency, only postpone it by four days. Reported in #6505 by @DABH, which proposed the same `--check` entry. That PR is stale and conflicting, and it bundles unrelated workflow hardening and a constraints regeneration, so only the one-line fix is taken here. Co-authored-by: Haran Rajkumar <haranrk@google.com> PiperOrigin-RevId: 961848034
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
The Pre-commit Linter job fails on every
mainrun since theupdate-constraintshook landed (e.g. this run), keepingmainred. This PR makes it green again. No functional code is touched.Why it fails
uv→ the hook dies withuv: command not found(masked as "Resolution failed").update_constraints.shin update mode, whose--exclude-newerdate moves daily and which exits 1 whenever it rewrites a file — so CI can't stay green even with uv installed.constraints-<ver>.txtfiles the hook manages were never committed, so check mode has nothing to validate.The fix
setup-uvaction the other jobs use).--check— the script's documented CI mode; it pins the date from the committed file header, so validation is deterministic. Devs still run the script directly to update pins.constraints-3.10…3.14.txtfiles; fail fast with a clear error if uv is missing; skip generated constraints files in codespell (it flags theastroidpackage name).persist-credentials: falseon this workflow's checkout steps so the zizmor gate is clean (it rescans the whole file on any edit and its four pre-existingartipackedfindings otherwise fail fork-PR runs).Testing plan
uvx pre-commit run --all-files: 13/13 hooks pass (previouslyupdate-constraintsfailed)../scripts/update_constraints.sh --checktwice back-to-back: exit 0, idempotent. Reviewers can verify the files by regenerating with./scripts/update_constraints.sh.zizmoron the workflow file: 0 findings (was 4, all pre-existing).