kaxil commented on code in PR #74803:
URL: https://github.com/apache/airflow/pull/74803#discussion_r4284751652
##########
scripts/tools/setup_breeze:
##########
@@ -84,7 +84,8 @@ else
echo "breeze: not inside an Airflow worktree, AIRFLOW_REPO_ROOT is unset
or not an Airflow worktree, and the install-time fallback
'\${fallback_root}/dev/breeze' is missing — re-run scripts/tools/setup_breeze"
>&2
exit 1
fi
-exec env AIRFLOW_ROOT_PATH="\${breeze_root}" SKIP_BREEZE_SELF_UPGRADE_CHECK=1
\\
+# '--locked' already guarantees no rewrite; an inherited UV_FROZEN conflicts
with it on older uv.
+exec env -u UV_FROZEN AIRFLOW_ROOT_PATH="\${breeze_root}"
SKIP_BREEZE_SELF_UPGRADE_CHECK=1 \\
Review Comment:
This fixes new installs, but a v2 shim already on someone's PATH still runs
`uv run --locked` with the `UV_FROZEN=1` it now inherits from prek. On uv
0.11.8 and 0.12.8 (both inside our `required-version = ">=0.11.8"` floor)
that's a hard `error: the argument --locked cannot be used with UV_FROZEN`;
0.12.9 is fine. So every breeze-backed root hook
(`check-template-fields-valid`, for one) fails on commit, and the SHIM_VERSION
bump never gets to tell them why, since `warn_if_shim_outdated` runs inside
breeze, which doesn't start. Could `initialize_breeze_prek` call
`describe_breeze_not_running_from_lock()` and exit with the re-run hint?
Dropping `UV_FROZEN` from `new_env` in `run_command_via_breeze_*`, or raising
the uv floor to 0.12.9, would also cover it.
##########
.pre-commit-config.yaml:
##########
@@ -17,11 +17,14 @@
---
default_stages: [pre-commit, pre-push]
default_install_hook_types: [pre-commit, commit-msg]
-minimum_prek_version: '0.3.4'
+minimum_prek_version: '0.4.8'
default_language_version:
python: python3
node: 22.19.0
golang: 1.25.0
+default_env:
+ # Unless overridden, no hook should change the resolutions in uv.lock; hooks
use the committed ones.
+ UV_FROZEN: "1"
Review Comment:
`default_env` only reaches hooks defined in this file. With prek 0.5.3, a
hook in a nested config like `providers/.pre-commit-config.yaml` sees
`UV_FROZEN` unset. The nested hook that still writes the lock is
`check-provider-yaml-valid`: `run_provider_yaml_files_check.py:128` runs `uv
sync --no-dev --all-packages` without `--frozen` inside breeze, with the host
`pyproject.toml`, `providers/` and `uv.lock` all bind-mounted. A provider
version bump without a re-lock (the release case from #74799) gets re-resolved
by the container's uv and written back to the host lock. That predates this PR,
but adding `--frozen` to that `uv sync` would close it. Breeze doesn't forward
`UV_FROZEN` into the container, so the flag is the only way there.
##########
contributing-docs/07_local_virtualenv.rst:
##########
@@ -287,8 +287,12 @@ or when running tests), you can pass the ``--frozen`` flag:
uv sync --frozen
-This will fail if the lock file is out of date with respect to
``pyproject.toml``, rather than
-silently updating it. This is useful when you want to guarantee fully
reproducible environments.
+This never resolves or writes to the lock file, even when it is out of date
with respect to
+``pyproject.toml``. Use ``--locked`` (or ``uv lock --check``) to fail on a
stale lock file instead.
+
+A change to a ``pyproject.toml`` (including a workspace member's version) must
be committed together
+with the refreshed ``uv.lock``: run ``uv lock``. The ``update-uv-lock`` prek
hook, which also runs in CI,
+fails otherwise. All other prek hooks run ``uv`` frozen and never change
``uv.lock``.
Review Comment:
This overstates it a bit. The two manual upgrade hooks run unfrozen on
purpose (`upgrade-important-versions` rewrites `dev/breeze/uv.lock`), and hooks
in nested configs don't get `default_env`. Maybe "Hooks in the root config that
run on commit use `uv` frozen and never change `uv.lock`"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]