fix(build): pin pre-commit's Python to .python-version - #502
Conversation
pre-commit builds its hook environments with whatever interpreter happens
to run it. Nothing pinned that, so on a machine where uvx resolved an
older Python the hooks were built against it, and `make fmt` failed with
a SyntaxError that has nothing to do with the caller's changes:
File "<unknown>", line 195
assert updated_time > initial_time, f"Time should increase. Initial: {
SyntaxError: unterminated string literal
rtc_test.py:195 uses a multi-line f-string, which is valid under the
project's declared Python (3.13, PEP 701) but not under 3.11, so the
interrogate hook could not parse it. CI happens to resolve a new enough
interpreter, so this only bites locally and only on some machines.
Pin the interpreter for uvx-run tooling to .python-version, keeping one
source of truth. uv downloads the pinned interpreter when it is missing,
so this needs no setup from contributors.
Verified with a cleared pre-commit cache: `make fmt` rebuilds every hook
environment and all hooks pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011kehXMi9zAAoPJ3PgfRd81
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe build tools now read the Python version from ChangesPre-commit interpreter pinning
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@makelib/build-tools.mk`:
- Line 15: Update the PYTHON_VERSION assignment in the build-tools configuration
to use a non-overridable assignment so .python-version remains authoritative for
pre-commit tooling; apply the same policy to the related UVX_PINNED setting if
present in the diff, or explicitly document intentional overrides.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 15b1d83c-54c4-4bf0-ac9c-846fd584d70c
📒 Files selected for processing (2)
Makefilemakelib/build-tools.mk
|
|
||
| ### Python version used for tooling environments. Single source of truth is .python-version, so | ||
| ### that tools do not silently build against whichever interpreter happens to be on the machine. | ||
| PYTHON_VERSION ?= $(shell cat .python-version) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
make -n PYTHON_VERSION=3.11 fmt | grep -F -- '--python 3.11'
make -n UVX_PINNED=uvx fmt | grep -F -- 'uvx pre-commit'Repository: Open-Source-Space-Foundation/proves-core-reference
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -eu
printf 'Python version file:\n'
cat -n .python-version 2>/dev/null || true
printf '\nRelevant build-tools.mk section:\n'
sed -n '1,120p' makelib/build-tools.mk 2>/dev/null || true
printf '\nUsages of PYTHON_VERSION/UVX_PINNED:\n'
rg -n 'PYTHON_VERSION|UVX_PINNED|pre-commit|uvx' -S . --glob '!**/.git/**' || trueRepository: Open-Source-Space-Foundation/proves-core-reference
Length of output: 2592
🌐 Web query:
GNU Make conditional assignment ?= environment variable command line override variable documentation
💡 Result:
In GNU Make, the ?= operator is a conditional variable assignment operator that assigns a value to a variable only if that variable is not yet defined [1][2]. Behavior and Precedence The ?= operator is functionally equivalent to [1]: ifeq ($(origin VARIABLE), undefined) VARIABLE = value endif Because it only acts when a variable is undefined, its behavior regarding environment variables and command-line arguments is dictated by the variable's state upon reaching the assignment [1][3]: 1. Command-Line Arguments: If you provide a variable assignment on the command line (e.g., make VAR=foo), that variable is considered defined before the makefile is parsed [3][4]. Consequently, any VAR ?= bar assignment in the makefile will be ignored, and the command-line value will persist [2][3]. 2. Environment Variables: By default, GNU Make imports environment variables as defined variables [5][6]. If a variable exists in the environment, VAR ?= bar will see that the variable is already defined and will not perform the assignment [3]. 3. Override Directive: If you must force a value in the makefile even when a command-line argument or environment variable is present, you can use the override directive (e.g., override VAR?= bar) [7][4]. Key Considerations * Defined vs. Empty: A variable set to an empty value (e.g., VAR=) is still considered defined [1][2]. In this case, VAR ?= bar will not assign "bar" because the variable is already defined, even though it is empty [1][2]. * Environment Overrides (-e): If you run make with the -e (--environment-overrides) flag, environment variables take precedence over ordinary assignments in the makefile [5][6]. However, even with -e, command-line arguments typically retain higher priority [3]. For most use cases, ?= is the standard way to provide default values that users can easily override via the command line or environment [2].
Citations:
- 1: https://www.gnu.org.cach3.com/software/make/manual/html_node/Conditional-Assignment.html
- 2: https://runebook.dev/en/docs/gnu_make/conditional-assignment
- 3: https://stackoverflow.com/questions/68825173/how-do-environment-variables-and-macros-interact-in-gnu-make
- 4: https://tool.oschina.net/uploads/apidocs/make-manual/Overriding.html
- 5: https://www.gnu.org.cach3.com/software/make/manual/html_node/Environment.html
- 6: https://pages.cs.wisc.edu/~alexande/books/gnu_make/make_6.html
- 7: https://www.math.utah.edu/docs/info/make_6.html
Keep .python-version authoritative for tooling.
PYTHON_VERSION ?= and UVX_PINNED ?= let command-line or environment values bypass the intended pin. Set these with non-overridable assignments if .python-version must control pre-commit tooling; otherwise document that overrides intentionally bypass it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@makelib/build-tools.mk` at line 15, Update the PYTHON_VERSION assignment in
the build-tools configuration to use a non-overridable assignment so
.python-version remains authoritative for pre-commit tooling; apply the same
policy to the related UVX_PINNED setting if present in the diff, or explicitly
document intentional overrides.
1 participant
Problem
make fmtfails on some machines with aSyntaxErrorin a file the caller never touched:pre-commitbuilds its hook environments with whatever interpreter happens to run it, and nothing in this repo pinned that. On a machine whereuvxresolves an older Python, theinterrogatehook gets built against it — andPROVESFlightControllerReference/test/int/rtc_test.py:195uses a multi-line f-string, which is valid under the project's declared Python (3.13, PEP 701) but a syntax error under 3.11.CI happens to resolve a new enough interpreter, so
mainis green and this only bites locally, and only on some machines. That makes it easy to mistake for a problem with your own branch.Fix
Pin the interpreter for
uvx-run tooling to.python-version, keeping one source of truth rather than hardcoding a version in the Makefile.uvdownloads the pinned interpreter when it is missing, so this requires no setup from contributors.Applied to both
fmtandpre-commit-install, so the installed git hook and themaketarget agree.Alternative considered
Rewriting the f-string in
rtc_test.pyto parse under 3.11 would clear the immediate error, but it treats the symptom — the next 3.12+ construct anyone writes breaks unpinned environments again, and the repo already requires 3.13+. Pinning fixes the class of problem. Happy to do both if reviewers prefer belt and braces.Verification
With the
interrogatehook cache cleared, so environments are genuinely rebuilt rather than reused:Build-tooling only — no firmware, component, or test sources are touched.
🤖 Generated with Claude Code
https://claude.ai/code/session_011kehXMi9zAAoPJ3PgfRd81