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/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/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/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/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/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/python/private/python.bzl b/python/private/python.bzl index 9fed9393ae..8256e1b856 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} 2.4.0 +::: +""", + 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} 2.4.0 +::: +""", + 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} 2.4.0 +::: +""", + 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/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", 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/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 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)