From 63045fb3e1338e04787defd74e682e4fef8e7936 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 27 Sep 2026 12:23:04 -0700 Subject: [PATCH 1/4] workflow: post comment when comment-triggered workflow fails (#4189) When a release or backport workflow triggered by an issue or PR comment fails, no notification appears on the thread. Maintainers must manually check the Actions tab to discover that a command failed. Add a failure-reporting job to the comment workflow that runs whenever any upstream job fails. It adds a thumbs-down reaction to the triggering comment and posts a reply linking to the failed workflow run. --- .github/workflows/on_comment.py | 96 +++++++++++++- .github/workflows/on_comment.yaml | 27 ++++ tests/workflows/on_comment_test.py | 129 ++++++++++++++++++- tests/workflows/workflow_permissions_test.py | 13 ++ 4 files changed, 259 insertions(+), 6 deletions(-) diff --git a/.github/workflows/on_comment.py b/.github/workflows/on_comment.py index 7a0ac45f26..502db98507 100755 --- a/.github/workflows/on_comment.py +++ b/.github/workflows/on_comment.py @@ -1,12 +1,26 @@ #!/usr/bin/env python3 """Parses issue and PR comments to dispatch release and backport workflows.""" +import argparse +import json import os import re import subprocess import sys +def _load_event_data() -> dict: + """Loads event JSON payload from GITHUB_EVENT_PATH.""" + event_path = os.environ.get("GITHUB_EVENT_PATH") + if not event_path or not os.path.isfile(event_path): + return {} + try: + with open(event_path, "r", encoding="utf-8") as f: + return json.load(f) + except Exception: + return {} + + def _get_bool(key: str, default: bool = False) -> bool: """Returns boolean value for an environment variable.""" val = os.environ.get(key) @@ -65,6 +79,21 @@ def _add_comment_reaction(repo: str, comment_id: str, content: str) -> None: ) +def _post_issue_comment(repo: str, issue_number: str, body: str) -> None: + """Posts a comment to a GitHub issue or PR using the gh CLI.""" + subprocess.run( + [ + "gh", + "issue", + "comment", + issue_number, + f"--repo={repo}", + f"--body={body}", + ], + check=True, + ) + + def _react_negative(repo: str, comment_id: str) -> None: """Logs error and adds a negative reaction to the comment.""" print("::error::No PRs specified for backport.") @@ -151,8 +180,60 @@ def _process_pr_comment(comment_body: str, pr_number: str) -> None: _write_github_output("command", "none") -def process_comment() -> int: +def _report_failure() -> int: + """Posts a failure comment and negative reaction when a workflow fails.""" + event = _load_event_data() + issue_data = event.get("issue") or {} + comment_data = event.get("comment") or {} + repo_data = event.get("repository") or {} + + event_number = str(issue_data.get("number") or os.environ.get("EVENT_NUMBER", "")) + comment_id = str(comment_data.get("id") or os.environ.get("COMMENT_ID", "")) + comment_url = str(comment_data.get("html_url") or os.environ.get("COMMENT_URL", "")) + repo = str(repo_data.get("full_name") or os.environ.get("GITHUB_REPOSITORY", "")) + command = os.environ.get("COMMAND", "") + run_id = os.environ.get("GITHUB_RUN_ID", "") + server_url = os.environ.get("GITHUB_SERVER_URL", "https://github.com") + + if comment_id and repo: + _add_comment_reaction(repo=repo, comment_id=comment_id, content="-1") + + if not event_number or not repo: + print( + "::error::Issue number and repository are required to post a" + " failure comment." + ) + return 1 + + if command and command != "none": + if comment_url: + header = ( + f"Workflow failed for command `{command}` ([comment]({comment_url}))." + ) + else: + header = f"Workflow failed for command `{command}`." + else: + if comment_url: + header = f"Workflow failed while processing [comment]({comment_url})." + else: + header = "Workflow failed while processing comment." + + if run_id: + run_url = f"{server_url}/{repo}/actions/runs/{run_id}" + details = f"See [workflow run]({run_url}) for logs." + else: + details = "See workflow logs for details." + + body = f"{header}\n\n{details}" + _post_issue_comment(repo=repo, issue_number=event_number, body=body) + return 0 + + +def process_comment(*, report_failure: bool = False) -> int: """Processes a comment from environment variables and dispatches actions.""" + if report_failure: + return _report_failure() + comment_body = os.environ.get("COMMENT_BODY", "") is_pr = _get_bool("IS_PR") event_number = os.environ.get("EVENT_NUMBER", "") @@ -189,9 +270,16 @@ def process_comment() -> int: return 0 -def _main() -> None: - sys.exit(process_comment()) +def _main(argv: list[str] | None = None) -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--report-failure", + action="store_true", + help="Post a failure comment to the issue or PR.", + ) + args = parser.parse_args(argv if argv is not None else []) + sys.exit(process_comment(report_failure=args.report_failure)) if __name__ == "__main__": - _main() + _main(sys.argv[1:]) diff --git a/.github/workflows/on_comment.yaml b/.github/workflows/on_comment.yaml index 7de72bc190..4bc531797e 100644 --- a/.github/workflows/on_comment.yaml +++ b/.github/workflows/on_comment.yaml @@ -139,3 +139,30 @@ jobs: with: issue: ${{ needs.parse_comment.outputs.issue_number }} secrets: inherit + + report_failure: + needs: + - parse_comment + - call_create_rc + - call_prepare_complete + - call_create_release_branch + - call_prepare + - call_add_backports + - call_process_backports_after_add + - call_process_backports_only + - call_sync_changelog + - call_promote + - call_backport_prepare + - call_backport_create_releases + if: always() && contains(needs.*.result, 'failure') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: "3.14" + - name: Post failure comment + env: + COMMAND: "${{ needs.parse_comment.outputs.command }}" + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: .github/workflows/on_comment.py --report-failure diff --git a/tests/workflows/on_comment_test.py b/tests/workflows/on_comment_test.py index 8ec6780456..03c1f9583c 100644 --- a/tests/workflows/on_comment_test.py +++ b/tests/workflows/on_comment_test.py @@ -1,6 +1,7 @@ """Tests for .github/workflows/on_comment.py.""" import dataclasses +import json from pathlib import Path import pytest @@ -14,6 +15,7 @@ class GitHubActionEnv: output_file: Path env_file: Path + event_file: Path def read_outputs(self) -> dict[str, str]: if not self.output_file.exists(): @@ -35,15 +37,38 @@ def read_env(self) -> dict[str, str]: res[k] = v return res + def set_event( + self, + *, + issue_number: int | str = 4175, + comment_id: int | str = 5772601429, + comment_url: str = "", + repo: str = "bazel-contrib/rules_python", + ) -> None: + payload = { + "issue": {"number": issue_number} if issue_number else {}, + "comment": { + k: v for k, v in [("id", comment_id), ("html_url", comment_url)] if v + }, + "repository": {"full_name": repo} if repo else {}, + } + self.event_file.write_text(json.dumps(payload), encoding="utf-8") + @pytest.fixture(name="gha_env", autouse=True) def fixture_gha_env(tmp_path, monkeypatch) -> GitHubActionEnv: - """Fixture that always sets GITHUB_OUTPUT and GITHUB_ENV environment variables.""" + """Fixture that sets GITHUB_OUTPUT, GITHUB_ENV, and GITHUB_EVENT_PATH.""" out_file = tmp_path / "github_output.txt" env_file = tmp_path / "github_env.txt" + event_file = tmp_path / "github_event.json" monkeypatch.setenv("GITHUB_OUTPUT", str(out_file)) monkeypatch.setenv("GITHUB_ENV", str(env_file)) - return GitHubActionEnv(output_file=out_file, env_file=env_file) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + return GitHubActionEnv( + output_file=out_file, + env_file=env_file, + event_file=event_file, + ) @pytest.fixture(name="mock_add_reaction", autouse=True) @@ -52,6 +77,12 @@ def fixture_mock_add_reaction(mocker): return mocker.patch("on_comment._add_comment_reaction") +@pytest.fixture(name="mock_post_comment", autouse=True) +def fixture_mock_post_comment(mocker): + """Fixture that mocks out _post_issue_comment by default for all tests.""" + return mocker.patch("on_comment._post_issue_comment") + + def _run_comment( monkeypatch, comment_body: str, @@ -376,3 +407,97 @@ def test_main_cli_execution(monkeypatch, gha_env): "command": "create-rc", } assert gha_env.read_env() == {"issue_number": "42"} + + +def test_report_failure_with_command_and_urls( + monkeypatch, gha_env, mock_add_reaction, mock_post_comment +): + comment_url = ( + "https://github.com/bazel-contrib/rules_python/issues/4175" + "#issuecomment-5772601429" + ) + run_url = "https://github.com/bazel-contrib/rules_python/actions/runs/35698664041" + gha_env.set_event( + issue_number=4175, + comment_id=5772601429, + comment_url=comment_url, + repo="bazel-contrib/rules_python", + ) + monkeypatch.setenv("COMMAND", "create-rc") + monkeypatch.setenv("GITHUB_RUN_ID", "35698664041") + monkeypatch.setenv("GITHUB_SERVER_URL", "https://github.com") + + assert process_comment(report_failure=True) == 0 + + mock_add_reaction.assert_called_once_with( + repo="bazel-contrib/rules_python", + comment_id="5772601429", + content="-1", + ) + mock_post_comment.assert_called_once_with( + repo="bazel-contrib/rules_python", + issue_number="4175", + body=( + f"Workflow failed for command `create-rc` ([comment]({comment_url})).\n\n" + f"See [workflow run]({run_url}) for logs." + ), + ) + + +def test_report_failure_without_command_or_urls( + monkeypatch, gha_env, mock_add_reaction, mock_post_comment +): + gha_env.set_event( + issue_number=4175, + comment_id="", + comment_url="", + repo="bazel-contrib/rules_python", + ) + monkeypatch.setenv("COMMAND", "") + monkeypatch.delenv("GITHUB_RUN_ID", raising=False) + + assert process_comment(report_failure=True) == 0 + + mock_add_reaction.assert_not_called() + mock_post_comment.assert_called_once_with( + repo="bazel-contrib/rules_python", + issue_number="4175", + body=( + "Workflow failed while processing comment.\n\n" + "See workflow logs for details." + ), + ) + + +def test_report_failure_missing_env(monkeypatch, gha_env, mock_post_comment, capsys): + gha_env.set_event(issue_number="", comment_id="", repo="") + monkeypatch.setenv("EVENT_NUMBER", "") + monkeypatch.setenv("GITHUB_REPOSITORY", "") + + assert process_comment(report_failure=True) == 1 + mock_post_comment.assert_not_called() + captured = capsys.readouterr() + assert "::error::Issue number and repository are required" in captured.out + + +def test_main_cli_report_failure( + monkeypatch, gha_env, mock_add_reaction, mock_post_comment +): + gha_env.set_event( + issue_number=42, + comment_id=111, + repo="bazel-contrib/rules_python", + ) + monkeypatch.setenv("COMMAND", "promote") + monkeypatch.setenv("GITHUB_RUN_ID", "99999") + + with pytest.raises(SystemExit) as exc_info: + _main(["--report-failure"]) + + assert exc_info.value.code == 0 + mock_add_reaction.assert_called_once_with( + repo="bazel-contrib/rules_python", + comment_id="111", + content="-1", + ) + mock_post_comment.assert_called_once() diff --git a/tests/workflows/workflow_permissions_test.py b/tests/workflows/workflow_permissions_test.py index db81924e2f..67b1ee4105 100644 --- a/tests/workflows/workflow_permissions_test.py +++ b/tests/workflows/workflow_permissions_test.py @@ -130,3 +130,16 @@ def test_reusable_workflow_permissions_hierarchy(): f"'{scope}: {required_level}', but caller only allows " f"'{scope}: {available_level}'." ) + + +def test_on_comment_report_failure_needs_all_jobs(): + """Validates that report_failure in on_comment.yaml depends on all action jobs.""" + rf = runfiles.CreateOrRaise() + on_comment_path = rf.root() / "rules_python/.github/workflows/on_comment.yaml" + content = yaml.safe_load(on_comment_path.read_text(encoding="utf-8")) + jobs = content["jobs"] + assert "report_failure" in jobs + + report_failure_needs = set(jobs["report_failure"]["needs"]) + expected_needs = set(jobs.keys()) - {"noop", "report_failure"} + assert report_failure_needs == expected_needs From fd22cfff38697b2c10b3feff88ea55d6d57bed7f Mon Sep 17 00:00:00 2001 From: gfrankliu <17630355+gfrankliu@users.noreply.github.com> Date: Sun, 27 Sep 2026 19:40:28 +0000 Subject: [PATCH 2/4] fix(zipapp): reduce bundled interpreter size (#4165) Bazel reports source symlinks as regular files, causing Python executable aliases to be copied into zipapps as full binaries. Hermetic runtimes also include shared libpython artifacts even when the interpreter links Python statically. Preserve source symlinks through sandbox indirection and exclude libpython shared objects from runtime files while retaining them for explicit native dependencies. Add regression coverage for archive structure and native extension loading. Fixes #3534 Fixes #4163 --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Richard Levasseur --- docs/toolchains.md | 10 ++++ news/4165.changed.md | 9 +++ news/py-zipapp-size.fixed.md | 4 ++ python/private/BUILD.bazel | 1 + .../private/hermetic_runtime_repo_setup.bzl | 5 ++ python/private/pbs_manifest.bzl | 28 +++++++++ python/private/python.bzl | 44 ++++++++++++++ python/private/python_register_toolchains.bzl | 4 ++ python/private/python_repository.bzl | 26 ++++++-- tests/py_zipapp/venv_zipapp_test.py | 6 ++ tests/python/python_tests.bzl | 22 +++++++ .../parse_runtime_manifest_tests.bzl | 43 +++++++++++++- tests/support/mocks/python_ext.bzl | 3 + tests/tools/zipapp/zipper_test.py | 59 +++++++++++++++++++ tools/zipapp/zipper.py | 24 ++++++-- 15 files changed, 277 insertions(+), 11 deletions(-) create mode 100644 news/4165.changed.md create mode 100644 news/py-zipapp-size.fixed.md diff --git a/docs/toolchains.md b/docs/toolchains.md index de495d5122..3ddce7b97e 100644 --- a/docs/toolchains.md +++ b/docs/toolchains.md @@ -240,12 +240,22 @@ existing attributes: via {attr}`python.override.minor_mapping`. * Per-version control of the coverage tool used using {attr}`python.single_version_platform_override.coverage_tool`. +* Control of shared `libpython` files using the `libpython` attribute on + {bzl:obj}`python.override`, {bzl:obj}`python.single_version_override`, or + {bzl:obj}`python.single_version_platform_override`. * Adding additional Python versions via {bzl:obj}`python.single_version_override` or {bzl:obj}`python.single_version_platform_override`. * Adding additional Python versions dynamically from a manifest file or URL via {attr}`python.override.add_runtime_manifest_files` or {attr}`python.override.add_runtime_manifest_urls`. +The `libpython` attribute accepts `auto`, `include`, or `exclude`, and defaults +to `auto`. In automatic mode, shared `libpython` files are excluded only for +recognized Astral Python Standalone builds from `20250517` onward, which are +known to statically link `libpython` into the interpreter. Unknown, custom, and +older runtimes retain the shared libraries. Use `include` or `exclude` to +override this behavior when mirroring or customizing a runtime. + ### Registering custom runtimes Because the python-build-standalone project has _thousands_ of prebuilt runtimes diff --git a/news/4165.changed.md b/news/4165.changed.md new file mode 100644 index 0000000000..64e91b2347 --- /dev/null +++ b/news/4165.changed.md @@ -0,0 +1,9 @@ +(toolchains) Shared `libpython*.so*` files are now excluded by default from +hermetic Python runtime files for recognized Astral `python-build-standalone` +builds (`20250517` and newer), which statically link `libpython` into the +interpreter. Older, custom, or unrecognized runtimes retain them. Set +`libpython = "include"` or `"exclude"` on {obj}`python.override`, +{obj}`python.single_version_override`, +{obj}`python.single_version_platform_override`, or +{obj}`python_register_toolchains` to override this behavior +([#4165](https://github.com/bazel-contrib/rules_python/pull/4165)). diff --git a/news/py-zipapp-size.fixed.md b/news/py-zipapp-size.fixed.md new file mode 100644 index 0000000000..508882b9da --- /dev/null +++ b/news/py-zipapp-size.fixed.md @@ -0,0 +1,4 @@ +(zipapp) Reduced self-contained archive sizes by preserving Python executable +symlinks instead of storing each alias as another copy of the interpreter, and +by omitting shared `libpython` files from recognized statically linked Astral +runtime builds. diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index b4ff84f14a..b88fb724a4 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -780,6 +780,7 @@ bzl_library( srcs = ["python_repository.bzl"], deps = [ ":auth", + ":pbs_manifest", ":repo_utils", ":text_util", "//python:versions", diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index 20b0324894..d61a040fe6 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -29,6 +29,7 @@ def define_hermetic_runtime_toolchain_impl( name, extra_files_glob_include, extra_files_glob_exclude, + interpreter_has_static_libpython = False, python_version, python_bin, coverage_tool): @@ -45,6 +46,8 @@ def define_hermetic_runtime_toolchain_impl( binaries). extra_files_glob_exclude: {type}`list[str]` additional glob exclude patterns for the target runtime files. + interpreter_has_static_libpython: {type}`bool` whether the interpreter + includes libpython statically. python_version: {type}`str` The Python version, in `major.minor.micro` format. python_bin: {type}`str` The path to the Python binary within the @@ -78,6 +81,8 @@ def define_hermetic_runtime_toolchain_impl( # During pyc creation, temp files named *.pyc.NNN are created "**/__pycache__/*.pyc.*", ] + if interpreter_has_static_libpython: + files_exclude.append("lib/libpython*.so*") files_exclude += extra_files_glob_exclude native.filegroup( diff --git a/python/private/pbs_manifest.bzl b/python/private/pbs_manifest.bzl index 86434dc0ea..ad019df0a8 100644 --- a/python/private/pbs_manifest.bzl +++ b/python/private/pbs_manifest.bzl @@ -1,5 +1,11 @@ """Helper functions to parse python-build-standalone manifests.""" +_ASTRAL_STATIC_LIBPYTHON_RELEASE = 20250517 +_ASTRAL_RELEASE_URL_PREFIXES = [ + "https://github.com/astral-sh/python-build-standalone/releases/download/", + "https://releases.astral.sh/github/python-build-standalone/releases/download/", +] + def parse_filename(filename): """Parses a python-build-standalone filename (or URL) into its components. @@ -112,6 +118,28 @@ def parse_filename(filename): "vendor": vendor, } +# buildifier: disable=function-docstring-args +# buildifier: disable=function-docstring-return +# urls: list[str], release_filename: str -> bool +def is_astral_static_libpython_build(urls, release_filename): + """Whether an Astral build includes libpython statically in its interpreter.""" + parsed = parse_filename(release_filename) + if not parsed: + return False + + build_version = parsed["build_version"] + if ( + not build_version.isdigit() or + int(build_version) < _ASTRAL_STATIC_LIBPYTHON_RELEASE + ): + return False + + return any([ + url.startswith(prefix) + for url in urls + for prefix in _ASTRAL_RELEASE_URL_PREFIXES + ]) + def parse_runtime_manifest(content): """Parses the SHA256SUMS file content into a list of structs. diff --git a/python/private/python.bzl b/python/private/python.bzl index 9fed9393ae..c7ca018102 100644 --- a/python/private/python.bzl +++ b/python/private/python.bzl @@ -612,6 +612,8 @@ def _process_single_version_overrides(*, tag, _fail = fail, default): kwargs.setdefault(tag.python_version, {})["distutils_content"] = tag.distutils_content if tag.distutils: kwargs.setdefault(tag.python_version, {})["distutils"] = tag.distutils + if tag.libpython and tag.libpython != "auto": + kwargs.setdefault(tag.python_version, {})["libpython"] = tag.libpython def _process_single_version_platform_overrides(*, tag, _fail = fail, default): if not _validate_version(tag.python_version, _fail = _fail): @@ -641,6 +643,8 @@ def _process_single_version_platform_overrides(*, tag, _fail = fail, default): if tag.urls: available_versions[tag.python_version].setdefault("url", {})[tag.platform] = tag.urls + if tag.libpython and tag.libpython != "auto": + available_versions[tag.python_version].setdefault("libpython", {})[tag.platform] = tag.libpython # If platform is customized, or doesn't exist, (re)define one. if ((tag.target_compatible_with or tag.target_settings or tag.os_name or tag.arch) or @@ -719,6 +723,7 @@ def _process_global_overrides(*, tag, default, _fail = fail): forwarded_attrs = sorted(AUTH_ATTRS) + [ "base_urls", + "libpython", "register_all_versions", ] for key in forwarded_attrs: @@ -1348,6 +1353,19 @@ dependencies are introduced. doc = """Deprecated; do not use. This attribute has no effect.""", mandatory = False, ), + "libpython": attr.string( + default = "auto", + doc = """Whether to include shared libpython files. + +Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized +Astral Python Standalone builds from 20250517 onward exclude the shared +libraries; other runtimes retain them. + +:::{versionadded} VERSION_NEXT_PATCH +::: +""", + values = ["auto", "include", "exclude"], + ), "minor_mapping": attr.string_dict( mandatory = False, doc = """\ @@ -1424,6 +1442,19 @@ class. "Either {attr}`distutils` or {attr}`distutils_content` can be specified, but not both.", mandatory = False, ), + "libpython": attr.string( + default = "auto", + doc = """Whether to include shared libpython files. + +Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized +Astral Python Standalone builds from 20250517 onward exclude the shared +libraries; other runtimes retain them. + +:::{versionadded} VERSION_NEXT_PATCH +::: +""", + values = ["auto", "include", "exclude"], + ), "patch_strip": attr.int( mandatory = False, doc = "Same as the --strip argument of Unix patch.", @@ -1493,6 +1524,19 @@ The coverage tool to be used for a particular Python interpreter. This can overr `rules_python` defaults. """, ), + "libpython": attr.string( + default = "auto", + doc = """Whether to include shared libpython files. + +Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized +Astral Python Standalone builds from 20250517 onward exclude the shared +libraries; other runtimes retain them. + +:::{versionadded} VERSION_NEXT_PATCH +::: +""", + values = ["auto", "include", "exclude"], + ), "os_name": attr.string( doc = """ The host OS the runtime is compatible with. diff --git a/python/private/python_register_toolchains.bzl b/python/private/python_register_toolchains.bzl index c6f827be22..770c6a6f3b 100644 --- a/python/private/python_register_toolchains.bzl +++ b/python/private/python_register_toolchains.bzl @@ -40,6 +40,7 @@ def python_register_toolchains( register_toolchains = True, register_coverage_tool = False, set_python_version_constraint = False, + libpython = None, tool_versions = None, platforms = PLATFORMS, minor_mapping = None, @@ -68,6 +69,8 @@ def python_register_toolchains( set_python_version_constraint: {type}`bool` When set to `True`, `target_compatible_with` for the toolchains will include a version constraint. + libpython: {type}`str` controls shared libpython files: `auto`, + `include`, or `exclude`. tool_versions: {type}`dict` contains a mapping of version with SHASUM and platform info. If not supplied, the defaults in python/versions.bzl will be used. @@ -145,6 +148,7 @@ def python_register_toolchains( urls = urls, strip_prefix = strip_prefix, coverage_tool = coverage_tool, + libpython = libpython or tool_versions[python_version].get("libpython", {}).get(platform, "auto"), **kwargs ) if register_toolchains: diff --git a/python/private/python_repository.bzl b/python/private/python_repository.bzl index 9c44971117..3fd0928a01 100644 --- a/python/private/python_repository.bzl +++ b/python/private/python_repository.bzl @@ -17,6 +17,7 @@ load("//python:versions.bzl", "FREETHREADED", "INSTALL_ONLY") load(":auth.bzl", "get_auth") +load(":pbs_manifest.bzl", "is_astral_static_libpython_build") load(":repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") load(":text_util.bzl", "render") @@ -162,6 +163,12 @@ def _python_repository_impl(rctx): *python_version_info ) urls = rctx.attr.urls or [rctx.attr.url] + if rctx.attr.libpython == "include": + interpreter_has_static_libpython = False + elif rctx.attr.libpython == "exclude": + interpreter_has_static_libpython = True + else: + interpreter_has_static_libpython = is_astral_static_libpython_build(urls, release_filename) auth = get_auth(rctx, urls) if INSTALL_ONLY in release_filename: @@ -277,16 +284,18 @@ load("@rules_python//python/private:hermetic_runtime_repo_setup.bzl", "define_he package(default_visibility = ["//visibility:public"]) define_hermetic_runtime_toolchain_impl( - name = "define_runtime", - extra_files_glob_include = {extra_files_glob_include}, - extra_files_glob_exclude = {extra_files_glob_exclude}, - python_version = {python_version}, - python_bin = {python_bin}, - coverage_tool = {coverage_tool}, + name = "define_runtime", + extra_files_glob_include = {extra_files_glob_include}, + extra_files_glob_exclude = {extra_files_glob_exclude}, + interpreter_has_static_libpython = {interpreter_has_static_libpython}, + python_version = {python_version}, + python_bin = {python_bin}, + coverage_tool = {coverage_tool}, ) """.format( extra_files_glob_exclude = render.list(glob_exclude), extra_files_glob_include = render.list(glob_include), + interpreter_has_static_libpython = str(interpreter_has_static_libpython), python_bin = render.str(python_bin), python_version = render.str(rctx.attr.python_version), coverage_tool = render.str(coverage_tool), @@ -361,6 +370,11 @@ For more information see {attr}`py_runtime.coverage_tool`. doc = "Noop, will be removed in the next major release", mandatory = False, ), + "libpython": attr.string( + default = "auto", + doc = "Whether to include shared libpython files: auto, include, or exclude.", + values = ["auto", "include", "exclude"], + ), "netrc": attr.string( doc = ".netrc file to use for authentication; mirrors the eponymous attribute from http_archive", ), diff --git a/tests/py_zipapp/venv_zipapp_test.py b/tests/py_zipapp/venv_zipapp_test.py index bd26d533a3..b906073fce 100644 --- a/tests/py_zipapp/venv_zipapp_test.py +++ b/tests/py_zipapp/venv_zipapp_test.py @@ -84,6 +84,12 @@ def test_zipapp_structure(self): # On Windows, pyvenv.cfg and bin/python3 are generated at runtime. if os.name != "nt": + self.assertFalse( + any( + "/lib/libpython" in name and ".so" in name for name in namelist + ), + "Statically linked Python should not bundle libpython", + ) self.assertHasPathMatchingSuffix(namelist, "/pyvenv.cfg") # The venv directory name depends on the target name, so find it diff --git a/tests/python/python_tests.bzl b/tests/python/python_tests.bzl index ef7ccf034f..22cd5c26b0 100644 --- a/tests/python/python_tests.bzl +++ b/tests/python/python_tests.bzl @@ -419,6 +419,28 @@ def _test_auth_overrides(env): _tests.append(_test_auth_overrides) +def _test_libpython_override(env): + py = parse_modules( + module_ctx = python_ext.mctx( + python_ext.module( + name = "my_module", + is_root = True, + override = [ + python_ext.override(libpython = "exclude"), + ], + toolchain = [python_ext.toolchain(python_version = "3.12")], + ), + _rules_python_module(), + ), + logger = repo_utils.logger(verbosity_level = 0, name = "python"), + ) + + env.expect.that_dict(py.config.default).contains_at_least({ + "libpython": "exclude", + }) + +_tests.append(_test_libpython_override) + def _test_add_target_settings(env): py = parse_modules( module_ctx = python_ext.mctx( diff --git a/tests/python_bzlmod_ext/parse_runtime_manifest_tests.bzl b/tests/python_bzlmod_ext/parse_runtime_manifest_tests.bzl index 4493576147..8c9176aeb2 100644 --- a/tests/python_bzlmod_ext/parse_runtime_manifest_tests.bzl +++ b/tests/python_bzlmod_ext/parse_runtime_manifest_tests.bzl @@ -4,7 +4,7 @@ load("@bazel_skylib//lib:structs.bzl", "structs") load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:util.bzl", rt_util = "util") -load("//python/private:pbs_manifest.bzl", "parse_filename", "parse_runtime_manifest") # buildifier: disable=bzl-visibility +load("//python/private:pbs_manifest.bzl", "is_astral_static_libpython_build", "parse_filename", "parse_runtime_manifest") # buildifier: disable=bzl-visibility _tests = [] @@ -97,6 +97,47 @@ def _test_parse_filename_baseline_impl(env, target): _tests.append(_test_parse_filename_baseline) +def _test_is_astral_static_libpython_build(name): + rt_util.helper_target( + native.filegroup, + name = name + "_subject", + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_is_astral_static_libpython_build_impl, + ) + +def _test_is_astral_static_libpython_build_impl(env, target): + _ = target # @unused + + github_url = "https://github.com/astral-sh/python-build-standalone/releases/download/20250517/archive.tar.gz" + mirror_url = "https://releases.astral.sh/github/python-build-standalone/releases/download/20250517/archive.tar.gz" + cutoff_filename = "cpython-3.13.4+20250517-x86_64-unknown-linux-gnu-install_only.tar.gz" + + env.expect.that_bool(is_astral_static_libpython_build( + [github_url], + cutoff_filename, + )).equals(True) + env.expect.that_bool(is_astral_static_libpython_build( + [mirror_url], + cutoff_filename, + )).equals(True) + env.expect.that_bool(is_astral_static_libpython_build( + [github_url], + "cpython-3.13.3+20250516-x86_64-unknown-linux-gnu-install_only.tar.gz", + )).equals(False) + env.expect.that_bool(is_astral_static_libpython_build( + ["https://example.com/20250604/archive.tar.gz"], + cutoff_filename, + )).equals(False) + env.expect.that_bool(is_astral_static_libpython_build( + [github_url], + "custom-python.tar.gz", + )).equals(False) + +_tests.append(_test_is_astral_static_libpython_build) + def _test_parse_runtime_manifest(name): """Sets up the manifest file parsing test. diff --git a/tests/support/mocks/python_ext.bzl b/tests/support/mocks/python_ext.bzl index 4c68e87e50..419afe8188 100644 --- a/tests/support/mocks/python_ext.bzl +++ b/tests/support/mocks/python_ext.bzl @@ -32,6 +32,7 @@ def _override(**kwargs): "available_python_versions": [], "base_urls": ["https://github.com/astral-sh/python-build-standalone/releases/download"], "ignore_root_user_error": True, + "libpython": "auto", "minor_mapping": {}, "register_all_versions": False, "runtime_manifest_sha": "", @@ -53,6 +54,7 @@ def _single_version_override(**kwargs): attrs = { "distutils": None, "distutils_content": "", + "libpython": "auto", "patch_strip": 0, "patches": [], "python_version": "", @@ -68,6 +70,7 @@ def _single_version_platform_override(**kwargs): attrs = { "arch": "", "coverage_tool": None, + "libpython": "auto", "os_name": "", "patch_strip": 0, "patches": [], diff --git a/tests/tools/zipapp/zipper_test.py b/tests/tools/zipapp/zipper_test.py index e4f25c5e21..e0ff1557d5 100644 --- a/tests/tools/zipapp/zipper_test.py +++ b/tests/tools/zipapp/zipper_test.py @@ -115,6 +115,65 @@ def test_create_zip_with_direct_symlink(tmp_path): ) +def test_create_zip_with_source_symlink_marked_as_regular(tmp_path): + manifest_path = tmp_path / "manifest.txt" + output_zip = tmp_path / "output.zip" + + target_path = tmp_path / "python3.14" + target_path.write_text("python") + symlink_path = tmp_path / "python" + symlink_path.symlink_to(target_path.name) + manifest_path.write_text(f"rf-file|0|bin/python|{symlink_path}") + + create_zip(manifest_path, output_zip) + + with zipfile.ZipFile(output_zip, "r") as zf: + assert_zip_file_content( + zf, + "runfiles/my_ws/bin/python", + is_symlink_file=True, + target=target_path.name, + ) + + +def test_create_zip_with_sandboxed_source_symlink(tmp_path): + manifest_path = tmp_path / "manifest.txt" + output_zip = tmp_path / "output.zip" + + source_dir = tmp_path / "source" + source_dir.mkdir() + target_path = source_dir / "python3.14" + target_path.write_text("python") + source_symlink = source_dir / "python" + source_symlink.symlink_to(target_path.name) + + sandbox_dir = tmp_path / "sandbox" + sandbox_dir.mkdir() + sandbox_symlink = sandbox_dir / "python" + sandbox_symlink.symlink_to(source_symlink) + sandbox_regular = sandbox_dir / "python3.14" + sandbox_regular.symlink_to(target_path) + manifest_path.write_text( + "\n".join( + [ + f"rf-file|0|bin/python|{sandbox_symlink}", + f"rf-file|0|bin/python3.14|{sandbox_regular}", + ] + ) + ) + + create_zip(manifest_path, output_zip) + + with zipfile.ZipFile(output_zip, "r") as zf: + assert_zip_file_content( + zf, + "runfiles/my_ws/bin/python", + is_symlink_file=True, + target=target_path.name, + ) + assert_zip_file_content(zf, "runfiles/my_ws/bin/python3.14", content="python") + + def test_pathsep_normalization(tmp_path): manifest_path = tmp_path / "manifest.txt" output_zip = tmp_path / "output.zip" diff --git a/tools/zipapp/zipper.py b/tools/zipapp/zipper.py index 5a8eb8c6fd..c0e547284a 100644 --- a/tools/zipapp/zipper.py +++ b/tools/zipapp/zipper.py @@ -118,6 +118,23 @@ def normalize_zip_path(path): return path.replace("\\", "/") +def _source_symlink_target(content_path, is_symlink_str): + if is_symlink_str == "1": + return os.readlink(content_path) + if is_symlink_str != "0" or not os.path.islink(content_path): + return None + + target = os.readlink(content_path) + if not os.path.isabs(target): + return target + + # Bazel sandboxes expose regular inputs as absolute symlinks. Look through + # that indirection to detect whether the original source is also a symlink. + if os.path.islink(target): + return os.readlink(target) + return None + + def _write_entry(zf, entry, compress_type, seen, platform_pathsep): type_, is_symlink_str, zip_path, content_path = entry # Normalize slashes, otherwise the `seen` logic doesn't @@ -155,14 +172,13 @@ def _write_entry(zf, entry, compress_type, seen, platform_pathsep): else: is_symlink_str = "0" - is_symlink = is_symlink_str == "1" - - if is_symlink: + symlink_target = _source_symlink_target(content_path, is_symlink_str) + if symlink_target is not None: zi = zipfile.ZipInfo(zip_path) zi.date_time = (1980, 1, 1, 0, 0, 0) zi.create_system = 3 # Unix zi.compress_type = compress_type - target = convert_symlink_target(os.readlink(content_path), platform_pathsep) + target = convert_symlink_target(symlink_target, platform_pathsep) # Set permissions to 777 for symlink (standard) zi.external_attr = (S_IFLNK | 0o777) << 16 zf.writestr(zi, target) From 9d3665209e991b7359d7d6c077798484059d374c Mon Sep 17 00:00:00 2001 From: Showta Ishizaki Date: Sun, 27 Sep 2026 20:36:52 +0000 Subject: [PATCH 3/4] fix(pypi): skip a pip.parse version with no interpreter for the host (#4184) On Windows ARM64, evaluating the `pip` extension fails as soon as any module in the graph calls `pip.parse` for Python 3.9 or 3.10: ``` Unable to find interpreter for pip hub 'grpc_python_dependencies' for python_version=3.9: ... Expected to find python_3_9_host among registered versions: python_3_11_host python_3_12_host python_3_13_host ``` There is no CPython 3.9 or 3.10 build for `aarch64-pc-windows-msvc` in the runtime manifest, so those toolchains have no host interpreter there. `_pip_parse` already handles a close relative of this: a version missing from `minor_mapping` is skipped with an info message, and the hub's select fails later only if that version is actually used. This treats a version with no host interpreter the same way, right after that check. The `python_X_Y_host` name is built in one small helper, so the new check and `_detect_interpreter` look at the same thing. To check it, I ran `bazel query @bazel_pip_dev_deps//:all` in a plain checkout of Bazel master (rules_python 1.9.2, grpc 1.76.0) with and without this change back-ported to 1.9.2, on `windows-11-arm` and `ubuntu-latest`: ``` without with windows-arm64 rc=7 rc=0 3.9 and 3.10 skipped for grpc, protoc-gen-validate and rules_fuzzing linux-x86_64 rc=0 rc=0 nothing new skipped ``` https://github.com/zakinko/NetBSD-i386/actions/runs/36248007899 With `RULES_PYTHON_REPO_DEBUG_VERBOSITY=INFO` the Windows run prints the new message once for each of the six hub and version pairs, next to the existing "no registered toolchain" one for rules_fuzzing's 3.8. The new test in `hub_builder_tests.bzl` fails without the change with the same error, and `//tests/pypi/...` passes with it (254 tests, on macOS). --------- Co-authored-by: Richard Levasseur --- news/4184.fixed.md | 6 ++ python/private/pypi/hub_builder.bzl | 26 +++++++- tests/pypi/hub_builder/hub_builder_tests.bzl | 69 ++++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 news/4184.fixed.md diff --git a/news/4184.fixed.md b/news/4184.fixed.md new file mode 100644 index 0000000000..edd4167df7 --- /dev/null +++ b/news/4184.fixed.md @@ -0,0 +1,6 @@ +(pypi) A {obj}`pip.parse` call for a Python version that has no interpreter for +the host platform is now skipped, like a version with no registered toolchain, +instead of failing the whole {obj}`pip` extension. A dependency that lists, +say, Python 3.9 no longer breaks every other hub on hosts that have no CPython +3.9 build, such as Windows on ARM64 +([#4184](https://github.com/bazel-contrib/rules_python/issues/4184)). diff --git a/python/private/pypi/hub_builder.bzl b/python/private/pypi/hub_builder.bzl index 596cfb2cbe..d1b583dc32 100644 --- a/python/private/pypi/hub_builder.bzl +++ b/python/private/pypi/hub_builder.bzl @@ -179,6 +179,25 @@ def _pip_parse(self, module_ctx, pip_attr, python_version = None): )) return + if (pip_attr.python_interpreter_target == None and + not pip_attr.python_interpreter and + _host_interpreter_name(python_version) not in self._available_interpreters): + # The version has a toolchain, but none of its interpreters runs on + # this host, e.g. there is no CPython build for this os and cpu. Treat + # it like a version missing from minor_mapping instead of failing the + # extension, which would also take down every other version and hub, + # including ones this host can use. + self._logger.info(lambda: ( + "Ignoring pip python version '{version}' for hub " + + "'{hub}' in module '{module}' because there is no " + + "interpreter for it that runs on this host." + ).format( + hub = self.name, + module = self.module_name, + version = python_version, + )) + return + _set_get_index_urls(self, module_ctx, pip_attr) self._platforms[python_version] = _platforms( module_ctx, @@ -395,12 +414,13 @@ def _set_get_index_urls(self, mctx, pip_attr): ) return True +def _host_interpreter_name(python_version): + return "python_{}_host".format(python_version.replace(".", "_")) + def _detect_interpreter(self, pip_attr, python_version): python_interpreter_target = pip_attr.python_interpreter_target if python_interpreter_target == None and not pip_attr.python_interpreter: - python_name = "python_{}_host".format( - python_version.replace(".", "_"), - ) + python_name = _host_interpreter_name(python_version) if python_name not in self._available_interpreters: fail(( "Unable to find interpreter for pip hub '{hub_name}' for " + diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index f421c7e351..1d4a602dc9 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -529,6 +529,75 @@ new-package==0.0.1 --hash=sha256:deadb00f2 _tests.append(_test_simple_multiple_python_versions) +def _test_skip_python_version_without_host_interpreter(env): + # A module can ask for a Python version that has a toolchain but no + # interpreter for this host, e.g. a dependency listing 3.9 when rules_python + # ships no 3.9 CPython for aarch64-pc-windows-msvc. That version must be + # skipped instead of failing the whole extension, like a version missing + # from minor_mapping is. + builder = hub_builder( + env, + available_interpreters = { + "python_3_15_host": "unit_test_interpreter_target", + }, + minor_mapping = { + "3.15": "3.15.19", + "3.16": "3.16.9", + }, + ) + builder.pip_parse( + mocks.mctx( + mock_files = { + "requirements_3_15.txt": """ +simple==0.0.1 --hash=sha256:deadbeef +""", + }, + os_name = "linux", + arch_name = "amd64", + ), + _parse( + hub_name = "pypi", + python_version = "3.15", + requirements_lock = "requirements_3_15.txt", + ), + ) + builder.pip_parse( + mocks.mctx( + mock_files = { + "requirements_3_16.txt": """ +simple==0.0.2 --hash=sha256:deadb00f +""", + }, + os_name = "linux", + arch_name = "amd64", + ), + _parse( + hub_name = "pypi", + python_version = "3.16", + requirements_lock = "requirements_3_16.txt", + ), + ) + pypi = builder.build() + + pypi.exposed_packages().contains_exactly(["simple"]) + pypi.whl_map().contains_exactly({ + "simple": { + "pypi_315_simple": [ + whl_config_setting(version = "3.15"), + ], + }, + }) + pypi.whl_libraries().contains_exactly({ + "pypi_315_simple": { + "config_load": "@pypi//:config.bzl", + "dep_template": "@pypi//{name}:{target}", + "python_interpreter_target": "unit_test_interpreter_target", + "requirement": "simple==0.0.1 --hash=sha256:deadbeef", + }, + }) + +_tests.append(_test_skip_python_version_without_host_interpreter) + def _test_simple_with_markers(env): sub_tests = { ("osx", "aarch64"): "torch==2.4.1 --hash=sha256:deadbeef", From 4ce7231160e498b10785fc8e0d913f7c53fd6a8f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:38:27 +0000 Subject: [PATCH 4/4] chore(release): sync changelog for v2.4.0 backports (#4190) Updates CHANGELOG.md and removes news files for backports: - #4165 Work towards #4175 Release-Tracking-Issue: #4175 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- CHANGELOG.md | 14 ++++++++++++++ news/4165.changed.md | 9 --------- news/py-zipapp-size.fixed.md | 4 ---- python/private/python.bzl | 6 +++--- 4 files changed, 17 insertions(+), 16 deletions(-) delete mode 100644 news/4165.changed.md delete mode 100644 news/py-zipapp-size.fixed.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 72cf0d4e0b..0c3e01c57f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,15 @@ changelog](https://rules-python.readthedocs.io/en/latest/changelog.html). in {obj}`python.defaults`, {obj}`python_register_toolchains`, and {obj}`python_register_multi_toolchains` ([#4023](https://github.com/bazel-contrib/rules_python/pull/4023)). +* (toolchains) Shared `libpython*.so*` files are now excluded by default from + hermetic Python runtime files for recognized Astral `python-build-standalone` + builds (`20250517` and newer), which statically link `libpython` into the + interpreter. Older, custom, or unrecognized runtimes retain them. Set + `libpython = "include"` or `"exclude"` on {obj}`python.override`, + {obj}`python.single_version_override`, + {obj}`python.single_version_platform_override`, or + {obj}`python_register_toolchains` to override this behavior + ([#4165](https://github.com/bazel-contrib/rules_python/pull/4165)). {#v2-4-0-fixed} ### Fixed @@ -81,6 +90,10 @@ changelog](https://rules-python.readthedocs.io/en/latest/changelog.html). * (toolchain) Fixed a crash on Windows when precompiling is enabled: the precompiler's interpreter couldn't find its DLLs once relocated ([#4082](https://github.com/bazel-contrib/rules_python/issues/4082)). +* (zipapp) Reduced self-contained archive sizes by preserving Python executable + symlinks instead of storing each alias as another copy of the interpreter, and + by omitting shared `libpython` files from recognized statically linked Astral + runtime builds. {#v2-4-0-added} ### Added @@ -91,6 +104,7 @@ changelog](https://rules-python.readthedocs.io/en/latest/changelog.html). package directory, set it to `None` or `""` to set it to the root of the repo ([#4029](https://github.com/bazel-contrib/rules_python/issues/4029)). + {#v2-3-4} ## [2.3.4] - 2026-09-21 diff --git a/news/4165.changed.md b/news/4165.changed.md deleted file mode 100644 index 64e91b2347..0000000000 --- a/news/4165.changed.md +++ /dev/null @@ -1,9 +0,0 @@ -(toolchains) Shared `libpython*.so*` files are now excluded by default from -hermetic Python runtime files for recognized Astral `python-build-standalone` -builds (`20250517` and newer), which statically link `libpython` into the -interpreter. Older, custom, or unrecognized runtimes retain them. Set -`libpython = "include"` or `"exclude"` on {obj}`python.override`, -{obj}`python.single_version_override`, -{obj}`python.single_version_platform_override`, or -{obj}`python_register_toolchains` to override this behavior -([#4165](https://github.com/bazel-contrib/rules_python/pull/4165)). diff --git a/news/py-zipapp-size.fixed.md b/news/py-zipapp-size.fixed.md deleted file mode 100644 index 508882b9da..0000000000 --- a/news/py-zipapp-size.fixed.md +++ /dev/null @@ -1,4 +0,0 @@ -(zipapp) Reduced self-contained archive sizes by preserving Python executable -symlinks instead of storing each alias as another copy of the interpreter, and -by omitting shared `libpython` files from recognized statically linked Astral -runtime builds. diff --git a/python/private/python.bzl b/python/private/python.bzl index c7ca018102..8256e1b856 100644 --- a/python/private/python.bzl +++ b/python/private/python.bzl @@ -1361,7 +1361,7 @@ Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized Astral Python Standalone builds from 20250517 onward exclude the shared libraries; other runtimes retain them. -:::{versionadded} VERSION_NEXT_PATCH +:::{versionadded} 2.4.0 ::: """, values = ["auto", "include", "exclude"], @@ -1450,7 +1450,7 @@ Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized Astral Python Standalone builds from 20250517 onward exclude the shared libraries; other runtimes retain them. -:::{versionadded} VERSION_NEXT_PATCH +:::{versionadded} 2.4.0 ::: """, values = ["auto", "include", "exclude"], @@ -1532,7 +1532,7 @@ Valid values are `auto`, `include`, and `exclude`. With `auto`, recognized Astral Python Standalone builds from 20250517 onward exclude the shared libraries; other runtimes retain them. -:::{versionadded} VERSION_NEXT_PATCH +:::{versionadded} 2.4.0 ::: """, values = ["auto", "include", "exclude"],