You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "kaxil (via GitHub)" <gi...@apache.org> on 2026/10/17 21:12:22 UTC
Re: [PR] Add mechanism to skip installing dependencies of excluded providers during image builds [airflow]
kaxil commented on code in PR #74556:
URL: https://github.com/apache/airflow/pull/74556#discussion_r4304527065
##########
scripts/docker/install_airflow_when_building_images.sh:
##########
@@ -35,6 +35,55 @@
# shellcheck source=scripts/docker/common.sh
. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"
+function has_providers_excluded_for_python() {
+ local python_minor="${AIRFLOW_PYTHON_VERSION%.*}"
+ PYTHONPATH="$( dirname "${BASH_SOURCE[0]}" )" python3 -c \
+ 'from pathlib import Path; import sys; from filter_ci_install_set import split_members; sys.exit(not split_members(Path.cwd(), sys.argv[1])[1])' \
+ "${python_minor}"
+}
+
+# uv sync --all-packages installs every workspace member, including providers excluded for this
+# Python minor and the third-party packages only they need. Resolve the members that are not excluded
+# for this interpreter, with the lock's pins as constraints, and install exactly that result.
+function install_workspace_without_excluded_providers() {
+ local python_minor="${AIRFLOW_PYTHON_VERSION%.*}"
+ local work_dir="/tmp/ci-install-set"
+ local python_bin
+ local platform_arch
+ local -a member_dirs=()
+ local -a editable_args=()
+ local member_dir
+ mkdir -p "${work_dir}"
+ python_bin="$(command -v python3)"
+ case "$(uname -m)" in
+ x86_64 | aarch64) platform_arch="$(uname -m)" ;;
+ *)
+ echo "Unsupported architecture for the filtered install: $(uname -m)"
+ exit 1
+ ;;
+ esac
+ echo
+ echo "${COLOR_BLUE}Installing the workspace without providers excluded for Python ${python_minor}.${COLOR_RESET}"
+ echo
+ set -x
+ uv export --frozen --all-packages --group ci-image --no-hashes --no-emit-workspace \
Review Comment:
`uv sync --frozen` checks every download against the sha256 in `uv.lock`. `--no-hashes` plus `uv pip install` drops that check for the whole 3.14 install, including the direct-URL `sphinx-airflow-theme` wheel the `ci-image` group pulls in through `devel-common[docs]`. Was that intended? If not, carrying the lock hashes for the resolved names into the requirements file and installing with `--require-hashes` would keep the check.
##########
scripts/docker/install_airflow_when_building_images.sh:
##########
@@ -35,6 +35,55 @@
# shellcheck source=scripts/docker/common.sh
. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"
+function has_providers_excluded_for_python() {
+ local python_minor="${AIRFLOW_PYTHON_VERSION%.*}"
+ PYTHONPATH="$( dirname "${BASH_SOURCE[0]}" )" python3 -c \
+ 'from pathlib import Path; import sys; from filter_ci_install_set import split_members; sys.exit(not split_members(Path.cwd(), sys.argv[1])[1])' \
+ "${python_minor}"
+}
+
+# uv sync --all-packages installs every workspace member, including providers excluded for this
+# Python minor and the third-party packages only they need. Resolve the members that are not excluded
+# for this interpreter, with the lock's pins as constraints, and install exactly that result.
+function install_workspace_without_excluded_providers() {
+ local python_minor="${AIRFLOW_PYTHON_VERSION%.*}"
+ local work_dir="/tmp/ci-install-set"
+ local python_bin
+ local platform_arch
+ local -a member_dirs=()
+ local -a editable_args=()
+ local member_dir
+ mkdir -p "${work_dir}"
+ python_bin="$(command -v python3)"
Review Comment:
In the PROD build stage this resolves to `/usr/python/bin/python3`, not the venv. `VIRTUAL_ENV` is `~/.local` there, but `ENV PATH="/usr/python/bin:$PATH"` (Dockerfile line 2407) puts the base interpreter ahead of `~/.local/bin`. So a from-sources `breeze prod-image build --python 3.14` (installation method `.` is the default, and cassandra, tinkerpop and vespa are excluded on 3.14) points both `uv pip install` calls and `uv pip check` at `/usr/python` instead of the venv the old `uv sync --active` path installed into. Running as `airflow`, I'd expect that to fail on permissions.
Could this prefer `${VIRTUAL_ENV:+${VIRTUAL_ENV}/bin/python}` and fall back to `command -v python3` only when no venv is set? The CI image has no `VIRTUAL_ENV`, so it would behave as it does now.
##########
scripts/tests/docker/test_install_airflow_when_building_images.py:
##########
@@ -0,0 +1,187 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+import subprocess
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parents[3]
+INSTALLER = ROOT / "scripts" / "docker" / "install_airflow_when_building_images.sh"
+HELPER = INSTALLER.with_name("filter_ci_install_set.py")
+
+
+@pytest.fixture
+def image_scripts(tmp_path: Path) -> Path:
+ scripts = tmp_path / "scripts"
+ scripts.mkdir()
+ source = INSTALLER.read_text(encoding="utf-8").split("\ncommon::get_colors\n", 1)[0]
+ source = source.replace('local work_dir="/tmp/ci-install-set"', f'local work_dir="{tmp_path}/work"')
+ (scripts / INSTALLER.name).write_text(source, encoding="utf-8")
+ (scripts / "common.sh").write_text("set -euo pipefail\n", encoding="utf-8")
+ (scripts / HELPER.name).write_text(HELPER.read_text(encoding="utf-8"), encoding="utf-8")
+ return scripts
+
+
+def _build_environment(bin_dir: Path) -> dict[str, str]:
+ environment = {
+ **os.environ,
+ "PATH": f"{bin_dir}:{os.environ['PATH']}",
+ "AIRFLOW_PYTHON_VERSION": "3.15.0",
+ "PACKAGING_TOOL_CMD": "uv",
+ "AIRFLOW_FALLBACK_NO_CONSTRAINTS_INSTALLATION": "false",
+ "UPGRADE_RANDOM_INDICATOR_STRING": "",
+ "COLOR_BLUE": "",
+ "COLOR_RESET": "",
+ }
+ environment.pop("AIRFLOW_SOURCES", None)
+ return environment
+
+
+@pytest.mark.parametrize(
+ ("metadata", "requires_python", "filtered"),
+ [
+ ('excluded-python-versions:\n - "3.15"\n', ">=3.11,!=3.15.*", True),
+ ("excluded-python-versions:\n - '3.15'\n", ">=3.11,!=3.15.*", True),
+ ("excluded-python-versions: ['3.15']\n", ">=3.11,!=3.15.*", True),
+ ("", ">=3.11", False),
+ ],
+)
+@pytest.mark.parametrize("upgrade", [False, True])
+def test_source_install_selects_path_without_ci_sources(
+ image_scripts: Path, tmp_path: Path, metadata: str, requires_python: str, filtered: bool, upgrade: bool
+):
+ workspace = tmp_path / "workspace"
+ provider = workspace / "providers" / "example"
+ provider.mkdir(parents=True)
+ (workspace / "pyproject.toml").write_text(
+ '[project]\nname = "test-root"\n[tool.uv.workspace]\nmembers = [".", "providers/example"]\n',
+ encoding="utf-8",
+ )
+ (provider / "pyproject.toml").write_text(
+ f'[project]\nname = "test-provider"\nrequires-python = "{requires_python}"\n', encoding="utf-8"
+ )
+ (provider / "provider.yaml").write_text(metadata, encoding="utf-8")
+ bin_dir = tmp_path / "bin"
+ bin_dir.mkdir()
+ (bin_dir / "uv").write_text('#!/usr/bin/env bash\nprintf "uv %s\\n" "$*"\n', encoding="utf-8")
+ (bin_dir / "uv").chmod(0o755)
+
+ result = subprocess.run(
+ [
+ "bash",
+ "-c",
+ 'source "$1"; function install_workspace_without_excluded_providers() { printf "filtered\\n"; }; install_from_sources',
+ "bash",
+ str(image_scripts / INSTALLER.name),
+ ],
+ cwd=workspace,
+ env={**_build_environment(bin_dir), "UPGRADE_RANDOM_INDICATOR_STRING": "random" if upgrade else ""},
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+
+ assert result.returncode == 0, result.stderr
+ if filtered and upgrade:
+ assert "uv lock --resolution highest" in result.stdout
+ assert result.stdout.endswith("filtered\n")
+ assert "uv sync" not in result.stdout
+ elif filtered:
+ assert result.stdout == "filtered\n"
+ elif upgrade:
+ assert "uv sync --all-packages --resolution highest --group ci-image" in result.stdout
+ assert "uv lock" not in result.stdout
+ else:
+ assert "uv sync --all-packages --frozen --group ci-image" in result.stdout
+
+
+@pytest.mark.parametrize("helper_fails", [False, True])
+def test_filtered_install_consumes_set_and_propagates_helper_failure(
+ image_scripts: Path, tmp_path: Path, helper_fails: bool
+):
+ bin_dir = tmp_path / "bin"
+ bin_dir.mkdir()
+ log = tmp_path / "commands"
+ (bin_dir / "uv").write_text(
+ '#!/usr/bin/env bash\nprintf "%s\\0" uv "$@" >> "$COMMAND_LOG"\nprintf "\\0" >> "$COMMAND_LOG"\n',
+ encoding="utf-8",
+ )
+ (bin_dir / "python3").write_text(
+ "#!/usr/bin/env bash\n"
+ 'printf "%s\\0" python3 "$@" >> "$COMMAND_LOG"\nprintf "\\0" >> "$COMMAND_LOG"\n'
+ 'if [[ "$HELPER_FAILS" == "true" ]]; then exit 23; fi\n'
+ "while [[ $# -gt 0 ]]; do\n"
+ ' case "$1" in\n'
+ ' --output) printf "shared==1.0\\n" > "$2"; shift ;;\n'
+ ' --editables) printf "%s\\n" "$KEPT_MEMBER" "$KEPT_WITH_SPACE" > "$2"; shift ;;\n'
+ " esac\n"
+ " shift\n"
+ "done\n",
+ encoding="utf-8",
+ )
+ for command in ("uv", "python3"):
+ (bin_dir / command).chmod(0o755)
+ kept = str(tmp_path / "kept")
+ kept_with_space = str(tmp_path / "kept with space")
+ result = subprocess.run(
+ [
+ "bash",
+ "-c",
+ 'source "$1"; install_workspace_without_excluded_providers',
+ "bash",
+ str(image_scripts / INSTALLER.name),
+ ],
+ cwd=tmp_path,
+ env={
+ **_build_environment(bin_dir),
+ "COMMAND_LOG": str(log),
+ "HELPER_FAILS": str(helper_fails).lower(),
+ "KEPT_MEMBER": kept,
+ "KEPT_WITH_SPACE": kept_with_space,
+ },
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+ commands = [entry.decode().split("\0") for entry in log.read_bytes().split(b"\0\0") if entry]
Review Comment:
On macOS both cases fail here with `FileNotFoundError`: `uname -m` returns `arm64`, the installer exits before `uv export`, and the log is never written. Stubbing `uname` in `bin_dir` like `uv` and `python3`, or asserting `returncode` before reading the log, would fix it and give a clearer failure.
##########
scripts/docker/install_airflow_when_building_images.sh:
##########
@@ -62,6 +120,10 @@ function install_from_sources() {
--no-python-downloads --no-managed-python
else
set +x
+ if has_providers_excluded_for_python; then
Review Comment:
This returns before the `AIRFLOW_FALLBACK_NO_CONSTRAINTS_INSTALLATION` handling below, and `Dockerfile.ci` defaults that flag to `true`. When a PR adds a dependency without updating `uv.lock`, 3.11 to 3.13 fall back to an unfrozen `uv sync`, but 3.14 stops in the helper with `... is not in the lock`. Should the filtered path honour the flag (re-lock, then filter, like the upgrade branch above), or is the stricter behaviour intended here?
##########
scripts/tests/docker/test_install_airflow_when_building_images.py:
##########
@@ -0,0 +1,187 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+import subprocess
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parents[3]
+INSTALLER = ROOT / "scripts" / "docker" / "install_airflow_when_building_images.sh"
+HELPER = INSTALLER.with_name("filter_ci_install_set.py")
+
+
+@pytest.fixture
+def image_scripts(tmp_path: Path) -> Path:
+ scripts = tmp_path / "scripts"
+ scripts.mkdir()
+ source = INSTALLER.read_text(encoding="utf-8").split("\ncommon::get_colors\n", 1)[0]
+ source = source.replace('local work_dir="/tmp/ci-install-set"', f'local work_dir="{tmp_path}/work"')
+ (scripts / INSTALLER.name).write_text(source, encoding="utf-8")
+ (scripts / "common.sh").write_text("set -euo pipefail\n", encoding="utf-8")
+ (scripts / HELPER.name).write_text(HELPER.read_text(encoding="utf-8"), encoding="utf-8")
+ return scripts
+
+
+def _build_environment(bin_dir: Path) -> dict[str, str]:
+ environment = {
+ **os.environ,
+ "PATH": f"{bin_dir}:{os.environ['PATH']}",
+ "AIRFLOW_PYTHON_VERSION": "3.15.0",
+ "PACKAGING_TOOL_CMD": "uv",
+ "AIRFLOW_FALLBACK_NO_CONSTRAINTS_INSTALLATION": "false",
+ "UPGRADE_RANDOM_INDICATOR_STRING": "",
+ "COLOR_BLUE": "",
+ "COLOR_RESET": "",
+ }
+ environment.pop("AIRFLOW_SOURCES", None)
+ return environment
+
+
+@pytest.mark.parametrize(
+ ("metadata", "requires_python", "filtered"),
+ [
+ ('excluded-python-versions:\n - "3.15"\n', ">=3.11,!=3.15.*", True),
+ ("excluded-python-versions:\n - '3.15'\n", ">=3.11,!=3.15.*", True),
+ ("excluded-python-versions: ['3.15']\n", ">=3.11,!=3.15.*", True),
+ ("", ">=3.11", False),
+ ],
+)
+@pytest.mark.parametrize("upgrade", [False, True])
+def test_source_install_selects_path_without_ci_sources(
+ image_scripts: Path, tmp_path: Path, metadata: str, requires_python: str, filtered: bool, upgrade: bool
+):
+ workspace = tmp_path / "workspace"
+ provider = workspace / "providers" / "example"
+ provider.mkdir(parents=True)
+ (workspace / "pyproject.toml").write_text(
+ '[project]\nname = "test-root"\n[tool.uv.workspace]\nmembers = [".", "providers/example"]\n',
+ encoding="utf-8",
+ )
+ (provider / "pyproject.toml").write_text(
+ f'[project]\nname = "test-provider"\nrequires-python = "{requires_python}"\n', encoding="utf-8"
+ )
+ (provider / "provider.yaml").write_text(metadata, encoding="utf-8")
+ bin_dir = tmp_path / "bin"
+ bin_dir.mkdir()
+ (bin_dir / "uv").write_text('#!/usr/bin/env bash\nprintf "uv %s\\n" "$*"\n', encoding="utf-8")
+ (bin_dir / "uv").chmod(0o755)
+
+ result = subprocess.run(
+ [
+ "bash",
+ "-c",
+ 'source "$1"; function install_workspace_without_excluded_providers() { printf "filtered\\n"; }; install_from_sources',
+ "bash",
+ str(image_scripts / INSTALLER.name),
+ ],
+ cwd=workspace,
+ env={**_build_environment(bin_dir), "UPGRADE_RANDOM_INDICATOR_STRING": "random" if upgrade else ""},
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+
+ assert result.returncode == 0, result.stderr
+ if filtered and upgrade:
+ assert "uv lock --resolution highest" in result.stdout
+ assert result.stdout.endswith("filtered\n")
+ assert "uv sync" not in result.stdout
+ elif filtered:
+ assert result.stdout == "filtered\n"
+ elif upgrade:
+ assert "uv sync --all-packages --resolution highest --group ci-image" in result.stdout
+ assert "uv lock" not in result.stdout
+ else:
+ assert "uv sync --all-packages --frozen --group ci-image" in result.stdout
+
+
+@pytest.mark.parametrize("helper_fails", [False, True])
+def test_filtered_install_consumes_set_and_propagates_helper_failure(
+ image_scripts: Path, tmp_path: Path, helper_fails: bool
+):
+ bin_dir = tmp_path / "bin"
+ bin_dir.mkdir()
+ log = tmp_path / "commands"
+ (bin_dir / "uv").write_text(
+ '#!/usr/bin/env bash\nprintf "%s\\0" uv "$@" >> "$COMMAND_LOG"\nprintf "\\0" >> "$COMMAND_LOG"\n',
+ encoding="utf-8",
+ )
+ (bin_dir / "python3").write_text(
+ "#!/usr/bin/env bash\n"
+ 'printf "%s\\0" python3 "$@" >> "$COMMAND_LOG"\nprintf "\\0" >> "$COMMAND_LOG"\n'
+ 'if [[ "$HELPER_FAILS" == "true" ]]; then exit 23; fi\n'
+ "while [[ $# -gt 0 ]]; do\n"
+ ' case "$1" in\n'
+ ' --output) printf "shared==1.0\\n" > "$2"; shift ;;\n'
+ ' --editables) printf "%s\\n" "$KEPT_MEMBER" "$KEPT_WITH_SPACE" > "$2"; shift ;;\n'
+ " esac\n"
+ " shift\n"
+ "done\n",
+ encoding="utf-8",
+ )
+ for command in ("uv", "python3"):
+ (bin_dir / command).chmod(0o755)
+ kept = str(tmp_path / "kept")
+ kept_with_space = str(tmp_path / "kept with space")
+ result = subprocess.run(
+ [
+ "bash",
+ "-c",
+ 'source "$1"; install_workspace_without_excluded_providers',
+ "bash",
+ str(image_scripts / INSTALLER.name),
+ ],
+ cwd=tmp_path,
+ env={
+ **_build_environment(bin_dir),
+ "COMMAND_LOG": str(log),
+ "HELPER_FAILS": str(helper_fails).lower(),
+ "KEPT_MEMBER": kept,
+ "KEPT_WITH_SPACE": kept_with_space,
+ },
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+ commands = [entry.decode().split("\0") for entry in log.read_bytes().split(b"\0\0") if entry]
+
+ assert commands[0][:3] == ["uv", "export", "--frozen"]
Review Comment:
Only the first three tokens of the export are checked. Dropping `--group ci-image` or `--no-emit-workspace` from the export, or `--no-binary lxml --no-binary xmlsec` from the installs, would still pass. A membership assert for each would cover the flags this path depends on.
##########
scripts/docker/filter_ci_install_set.py:
##########
@@ -0,0 +1,310 @@
+#!/usr/bin/env python3
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Compute the CI image install set for one Python minor without excluded workspace members.
+
+``uv sync --all-packages`` installs every workspace member, including the dev groups of providers
+excluded for a Python minor (``Requires-Python`` carries ``!=<minor>``). Their third-party
+dependencies would still be installed, and some do not build for the new interpreter.
+
+The helper resolves the requirements of the members that are not excluded, not the lock's export
+graph. Each kept member becomes a local ``file://`` requirement, and the requirements of each kept
+member's ``dev`` group (the root's ``ci-image`` group for the root) are added verbatim, with
+excluded members removed. ``uv pip compile`` resolves that input for the target interpreter and
+platform, so markers apply per edge. The lock's pins are passed as constraints, so every version
+matches the lock. Output is the third-party pins and the editable member directories.
+"""
+
+from __future__ import annotations
+
+import argparse
+import re
+import subprocess
+import sys
+import tomllib
+from glob import glob
+from pathlib import Path
+from typing import Any
+from urllib.parse import unquote, urlparse
+
+MEMBER_DEFAULT_GROUP = "dev"
+ROOT_CI_GROUP = "ci-image"
+MINOR_VERSION = re.compile(r"\d+\.\d+")
+REQUIREMENT_NAME = re.compile(r"\s*([A-Za-z0-9][A-Za-z0-9._-]*)")
+RESOLVED_MEMBER = re.compile(r"^([^\s@]+)\s*@\s*(file://\S+)$")
+PINNED_NAME = re.compile(r"^([A-Za-z0-9][A-Za-z0-9._-]*)(?:\[[^\]]*\])?\s*(?:==|@)")
+PINNED_VERSION = re.compile(r"^([A-Za-z0-9][A-Za-z0-9._-]*)==([^\s;]+)")
+GLOB_CHARACTERS = frozenset("*?[")
+PYTHON_PLATFORMS = {"x86_64": "x86_64-manylinux_2_28", "aarch64": "aarch64-manylinux_2_28"}
+
+
+def normalize_name(name: str) -> str:
+ return re.sub(r"[-_.]+", "-", name).lower()
+
+
+def requirement_name(entry: str) -> str:
+ match = REQUIREMENT_NAME.match(entry)
+ if not match:
+ raise ValueError(f"Cannot read a requirement name from {entry!r}")
+ return normalize_name(match.group(1))
+
+
+def is_python_minor_excluded(requires_python: str, minor: str) -> bool:
+ # The lookahead keeps longer versions such as !=3.150 or !=3.15.1 from matching a whole-minor exclusion.
+ return re.search(rf"!=\s*{re.escape(minor)}(\.\*)?(?![\d.])", requires_python) is not None
+
+
+def load_manifest(directory: Path) -> dict[str, Any]:
+ with (directory / "pyproject.toml").open("rb") as manifest:
+ return tomllib.load(manifest)
+
+
+def has_glob_characters(pattern: str) -> bool:
+ return any(character in GLOB_CHARACTERS for character in pattern)
+
+
+def find_workspace_members(root: Path, patterns: list[str]) -> list[Path]:
+ members: list[Path] = []
+ for pattern in patterns:
+ matches = sorted(glob(str(root / pattern)))
+ if not matches and not has_glob_characters(pattern):
+ raise ValueError(f"Workspace member {pattern!r} does not exist under {root}")
+ for match in matches:
+ candidate = Path(match)
+ if (candidate / "pyproject.toml").is_file():
+ members.append(candidate)
+ elif not has_glob_characters(pattern):
+ raise ValueError(f"Workspace member {pattern!r} has no pyproject.toml")
+ return members
+
+
+def expand_group(groups: dict[str, list[Any]], group: str, seen: set[str] | None = None) -> list[str]:
+ """Return the raw requirement strings of a dependency group, following ``include-group`` entries."""
+ if seen is None:
+ seen = set()
+ if group in seen:
+ return []
+ seen.add(group)
+ entries: list[str] = []
+ for entry in groups.get(group, []):
+ if isinstance(entry, dict):
+ entries.extend(expand_group(groups, entry["include-group"], seen))
+ else:
+ entries.append(entry)
+ return entries
+
+
+def split_members(root: Path, minor: str) -> tuple[dict[str, Path], set[str]]:
+ """Return the members kept for installation (name -> directory) and the excluded member names."""
+ patterns = load_manifest(root)["tool"]["uv"]["workspace"]["members"]
+ kept: dict[str, Path] = {}
+ excluded: set[str] = set()
+ for member in find_workspace_members(root, patterns):
+ project = load_manifest(member).get("project", {})
+ name = normalize_name(project.get("name", member.name))
+ if is_python_minor_excluded(project.get("requires-python", ""), minor):
+ excluded.add(name)
+ else:
+ kept[name] = member
+ return kept, excluded
+
+
+def build_requirement_input(root: Path, kept: dict[str, Path], excluded: set[str]) -> list[str]:
+ """Return the requirements to resolve: kept members as local references and their dev requirements."""
+ lines = [f"{name} @ {path.resolve().as_uri()}" for name, path in sorted(kept.items())]
+ for path in kept.values():
+ groups = load_manifest(path).get("dependency-groups", {})
+ group = ROOT_CI_GROUP if path == root else MEMBER_DEFAULT_GROUP
+ for entry in expand_group(groups, group):
+ if requirement_name(entry) not in excluded:
+ lines.append(entry)
+ return lines
+
+
+def resolve_requirements(
+ requirements: list[str],
+ constraints: Path,
+ minor: str,
+ platform: str,
+ workdir: Path,
+ root: Path,
+ find_links: list[Path],
+) -> list[str]:
+ """Resolve the requirements for the target interpreter with the lock's pins as constraints.
+
+ The command runs from the workspace root so that the root's ``exclude-newer`` applies to the resolver.
+ """
+ # uv runs from the workspace root, so relative paths would resolve there rather than here.
+ workdir = workdir.resolve()
+ constraints = constraints.resolve()
+ requirements_in = workdir / "requirements.in"
+ resolved = workdir / "resolved.txt"
+ requirements_in.write_text("\n".join(requirements) + "\n", encoding="utf-8")
+ find_links_args = [argument for directory in find_links for argument in ("--find-links", str(directory))]
+ subprocess.run(
+ [
+ "uv",
+ "pip",
+ "compile",
+ str(requirements_in),
+ "--constraints",
+ str(constraints),
+ "--python-version",
+ minor,
+ "--python-platform",
+ platform,
+ *find_links_args,
+ "--no-header",
+ "--no-annotate",
+ "--output-file",
+ str(resolved),
+ ],
+ check=True,
+ cwd=root,
+ )
+ return resolved.read_text(encoding="utf-8").splitlines()
+
+
+def split_resolved(lines: list[str]) -> tuple[list[str], list[Path]]:
+ """Separate third-party pins from the resolved local member directories (``-e file://`` or ``name @ file://``)."""
+ pins: list[str] = []
+ members: list[Path] = []
+ for line in lines:
+ stripped = line.strip()
+ if not stripped or stripped.startswith("#"):
+ continue
+ if stripped.startswith("-e "):
+ members.append(member_path(stripped[len("-e ") :]))
+ continue
+ match = RESOLVED_MEMBER.match(stripped)
+ if match:
+ members.append(member_path(match.group(2)))
+ elif "file://" in stripped:
+ raise ValueError(f"Cannot read a local member reference from the resolved output: {stripped!r}")
+ else:
+ pins.append(line)
+ return pins, members
+
+
+def member_path(file_uri: str) -> Path:
+ return Path(unquote(urlparse(file_uri.strip()).path))
+
+
+def verify_resolved_pins(pins: list[str], lock_pins: str, excluded: set[str]) -> None:
+ """Fail when the resolver returns an excluded member, a package the lock does not contain, or a locked version differing."""
+ locked_names: set[str] = set()
+ locked_versions: dict[str, set[str]] = {}
+ for line in lock_pins.splitlines():
+ stripped = line.strip()
+ name_match = PINNED_NAME.match(stripped)
+ if name_match:
+ locked_names.add(normalize_name(name_match.group(1)))
+ version_match = PINNED_VERSION.match(stripped)
+ if version_match:
+ locked_versions.setdefault(normalize_name(version_match.group(1)), set()).add(
+ version_match.group(2)
+ )
+ problems: list[str] = []
+ for line in pins:
+ stripped = line.strip()
+ name_match = PINNED_NAME.match(stripped)
+ if not name_match:
+ problems.append(f"unreadable resolved requirement {stripped!r}")
+ continue
+ name = normalize_name(name_match.group(1))
+ if name in excluded:
+ problems.append(f"excluded member {name} was resolved from an index")
+ elif name not in locked_names:
+ problems.append(f"{name} is not in the lock")
+ else:
+ version_match = PINNED_VERSION.match(stripped)
+ if version_match and version_match.group(2) not in locked_versions.get(name, set()):
+ problems.append(
+ f"{name}=={version_match.group(2)} differs from the locked versions "
+ f"{sorted(locked_versions.get(name, set()))}"
+ )
+ if problems:
+ raise ValueError("Resolved pins disagree with the lock: " + "; ".join(problems))
+
+
+def main(argv: list[str] | None = None) -> int:
+ parser = argparse.ArgumentParser(
+ description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter
+ )
+ parser.add_argument(
+ "--root", type=Path, required=True, help="Workspace root holding the root pyproject.toml"
+ )
+ parser.add_argument(
+ "--constraints", type=Path, required=True, help="Lock pins (uv export) used as constraints"
+ )
+ parser.add_argument("--python-minor", required=True, help="Target Python major.minor, e.g. 3.15")
+ parser.add_argument(
+ "--platform", choices=sorted(PYTHON_PLATFORMS), required=True, help="Target CPU architecture"
+ )
+ parser.add_argument("--workdir", type=Path, required=True, help="Scratch directory for resolver inputs")
+ parser.add_argument("--output", type=Path, required=True, help="Where to write the third-party pins")
+ parser.add_argument(
+ "--editables", type=Path, required=True, help="Where to write the kept member directories"
+ )
+ parser.add_argument(
+ "--find-links",
+ type=Path,
+ action="append",
+ default=[],
+ help="Directory of local wheels the resolver may use, e.g. temporary wheels for a missing release",
+ )
+ args = parser.parse_args(argv)
+ if not MINOR_VERSION.fullmatch(args.python_minor):
+ parser.error(f"--python-minor must be major.minor, got {args.python_minor!r}")
+ root = args.root.resolve()
+ args.workdir.mkdir(parents=True, exist_ok=True)
+
+ kept, excluded = split_members(root, args.python_minor)
+ requirements = build_requirement_input(root, kept, excluded)
+ resolved = resolve_requirements(
+ requirements,
+ args.constraints,
+ args.python_minor,
+ PYTHON_PLATFORMS[args.platform],
+ args.workdir,
+ root,
+ [directory.resolve() for directory in args.find_links],
+ )
+ pins, member_dirs = split_resolved(resolved)
+ verify_resolved_pins(pins, args.constraints.read_text(encoding="utf-8"), excluded)
Review Comment:
These guards aren't exercised through `main()`. Both `main()` tests hand back a resolved set that already agrees with the lock, so removing this call, or the "not kept" / "missing" checks below, keeps the suite green. The "not kept" and "missing" checks have no test at all. Parametrizing `fake_resolve` in `test_main_writes_pins_and_editables_for_kept_members` to return a version outside the lock, an excluded member's `-e` line, and a set missing a kept member would cover all three.
##########
scripts/tests/docker/test_install_airflow_when_building_images.py:
##########
@@ -0,0 +1,187 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+import subprocess
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parents[3]
+INSTALLER = ROOT / "scripts" / "docker" / "install_airflow_when_building_images.sh"
+HELPER = INSTALLER.with_name("filter_ci_install_set.py")
+
+
+@pytest.fixture
+def image_scripts(tmp_path: Path) -> Path:
+ scripts = tmp_path / "scripts"
+ scripts.mkdir()
+ source = INSTALLER.read_text(encoding="utf-8").split("\ncommon::get_colors\n", 1)[0]
+ source = source.replace('local work_dir="/tmp/ci-install-set"', f'local work_dir="{tmp_path}/work"')
+ (scripts / INSTALLER.name).write_text(source, encoding="utf-8")
+ (scripts / "common.sh").write_text("set -euo pipefail\n", encoding="utf-8")
+ (scripts / HELPER.name).write_text(HELPER.read_text(encoding="utf-8"), encoding="utf-8")
+ return scripts
+
+
+def _build_environment(bin_dir: Path) -> dict[str, str]:
+ environment = {
+ **os.environ,
+ "PATH": f"{bin_dir}:{os.environ['PATH']}",
+ "AIRFLOW_PYTHON_VERSION": "3.15.0",
+ "PACKAGING_TOOL_CMD": "uv",
+ "AIRFLOW_FALLBACK_NO_CONSTRAINTS_INSTALLATION": "false",
+ "UPGRADE_RANDOM_INDICATOR_STRING": "",
+ "COLOR_BLUE": "",
+ "COLOR_RESET": "",
+ }
+ environment.pop("AIRFLOW_SOURCES", None)
+ return environment
+
+
+@pytest.mark.parametrize(
+ ("metadata", "requires_python", "filtered"),
Review Comment:
Nothing reads `provider.yaml` in this path (the check only looks at `requires-python`), so the three `excluded-python-versions` spellings run the same case. Can the `metadata` parameter go?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org