From 233e28359109aef6243584469a909a7c95594e75 Mon Sep 17 00:00:00 2001 From: Frank Liu Date: Thu, 12 Nov 2026 08:00:43 +0000 Subject: [PATCH 1/3] fix(zipapp): store venv runfiles symlinks as symlinks, not copies Runfiles symlinks whose target file is also packed in the zipapp (e.g. the venv site-packages entries for shared libraries, which point into the pip repos) are now stored as relative symlinks to the packed file instead of a second copy of its content. --- news/zipapp-venv-symlinks.fixed.md | 3 +++ python/private/zipapp/py_zipapp_rule.bzl | 34 ++++++++++++++++++++++-- tests/py_zipapp/BUILD.bazel | 4 +++ tests/py_zipapp/main.py | 3 +++ tests/py_zipapp/venv_zipapp_test.py | 30 +++++++++++++++++++++ 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 news/zipapp-venv-symlinks.fixed.md diff --git a/news/zipapp-venv-symlinks.fixed.md b/news/zipapp-venv-symlinks.fixed.md new file mode 100644 index 0000000000..58ae8a5977 --- /dev/null +++ b/news/zipapp-venv-symlinks.fixed.md @@ -0,0 +1,3 @@ +(zipapp) {obj}`py_zipapp_binary` stores runfiles symlinks whose target is also +packed in the zipapp, such as the venv `site-packages` entries that point into +pip repositories, as relative symlinks instead of a second copy of the file. diff --git a/python/private/zipapp/py_zipapp_rule.bzl b/python/private/zipapp/py_zipapp_rule.bzl index be00b04264..3bef6b38e7 100644 --- a/python/private/zipapp/py_zipapp_rule.bzl +++ b/python/private/zipapp/py_zipapp_rule.bzl @@ -93,6 +93,30 @@ def _map_zip_root_symlinks(entry): def _map_explicit_symlinks(entry): return "symlink|" + entry.runfiles_path + "|" + entry.link_to_path +def _map_runfiles_symlinks(ctx, runfiles): + """Maps runfiles symlinks, avoiding duplicate copies of packed files. + + A runfiles symlink whose target file is also stored at its own runfiles + path (e.g. a venv site-packages entry, which points into a pip repo) is + stored as a relative symlink to that file instead of a second copy of its + content. + """ + packed = {f: None for f in runfiles.files.to_list()} + lines = [] + for prefix, entries, map_entry in [ + (ctx.workspace_name, runfiles.symlinks, _map_zip_symlinks), + ("", runfiles.root_symlinks, _map_zip_root_symlinks), + ]: + for entry in entries.to_list(): + if entry.target_file in packed: + lines.append("symlink|{}|{}".format( + paths.join(prefix, entry.path), + runfiles_root_path(ctx, entry.target_file.short_path), + )) + else: + lines.append(map_entry(entry)) + return lines + def _build_manifest(ctx, manifest, runfiles, explicit_symlinks, inputs): manifest.add_all( # NOTE: Accessing runfiles.empty_filenames materializes them. A lambda @@ -103,8 +127,14 @@ def _build_manifest(ctx, manifest, runfiles, explicit_symlinks, inputs): ) manifest.add_all(runfiles.files, map_each = _map_zip_runfiles) - manifest.add_all(runfiles.symlinks, map_each = _map_zip_symlinks) - manifest.add_all(runfiles.root_symlinks, map_each = _map_zip_root_symlinks) + + # On Windows, creating symlinks when extracting needs extra privileges, so + # only the explicit venv symlinks are stored as symlinks there. + if py_internal.get_legacy_external_runfiles(ctx) or is_windows_platform(ctx): + manifest.add_all(runfiles.symlinks, map_each = _map_zip_symlinks) + manifest.add_all(runfiles.root_symlinks, map_each = _map_zip_root_symlinks) + else: + manifest.add_all(_map_runfiles_symlinks(ctx, runfiles)) manifest.add_all(explicit_symlinks, map_each = _map_explicit_symlinks) inputs.add(runfiles.files) diff --git a/tests/py_zipapp/BUILD.bazel b/tests/py_zipapp/BUILD.bazel index ede19baff8..ff42539c96 100644 --- a/tests/py_zipapp/BUILD.bazel +++ b/tests/py_zipapp/BUILD.bazel @@ -103,6 +103,10 @@ py_library( deps = [ ":pkgdep", ":some_dep", + # A pip dep with a native extension. The venv links each shared + # library file individually, so it's a runfiles symlink to a file + # that's also packed in the zipapp. + "@dev_pip//markupsafe", ], ) diff --git a/tests/py_zipapp/main.py b/tests/py_zipapp/main.py index 595bed754e..827b50516e 100644 --- a/tests/py_zipapp/main.py +++ b/tests/py_zipapp/main.py @@ -51,6 +51,9 @@ def main(): import pkgdep.pkgmod print(f"dep: {pkgdep.pkgmod}") + import markupsafe._speedups + + print(f"native dep: {markupsafe._speedups}") except ImportError as e: e.add_note( "Failed to import a dependency.\n" + "sys.path:\n" + "\n".join(sys.path) diff --git a/tests/py_zipapp/venv_zipapp_test.py b/tests/py_zipapp/venv_zipapp_test.py index b906073fce..a6b5830dee 100644 --- a/tests/py_zipapp/venv_zipapp_test.py +++ b/tests/py_zipapp/venv_zipapp_test.py @@ -1,5 +1,6 @@ import contextlib import os +import posixpath import subprocess import unittest import zipfile @@ -26,6 +27,7 @@ def test_zipapp_runnable(self): ) self.assertIn("Hello from zipapp", output) self.assertIn("dep:", output) + self.assertIn("native dep:", output) def assertHasPathMatchingSuffix(self, namelist, suffix, msg=None): if not any(name.endswith(suffix) for name in namelist): @@ -105,6 +107,34 @@ def test_zipapp_structure(self): python_bin = f"{venv_root}/bin/python3" self.assertZipEntryIsSymlink(zf, python_bin) + # The venv links a pip dep's shared library individually. It's + # stored as a relative symlink to the copy packed from the pip + # repo, not as a second copy of the library. + venv_file = next( + ( + name + for name in namelist + if name.startswith(venv_root + "/lib/") + and "/site-packages/markupsafe/_speedups." in name + and name.endswith(".so") + ), + None, + ) + self.assertIsNotNone(venv_file, "\n".join(namelist)) + self.assertZipEntryIsSymlink(zf, venv_file) + target = posixpath.normpath( + posixpath.join( + posixpath.dirname(venv_file), + zf.read(venv_file).decode("utf-8"), + ) + ) + self.assertIn(target, namelist) + self.assertNotIn(".venv/", target) + self.assertTrue( + (zf.getinfo(target).external_attr >> 16) & 0o170000 != 0o120000, + f"{target} should be the packed file, not another symlink", + ) + # Verify _bazel_site_init.py exists in site-packages self.assertHasPathMatchingSuffix( namelist, "/site-packages/_bazel_site_init.py" From 9601a922b292895523384c5666c05972d55d1924 Mon Sep 17 00:00:00 2001 From: Frank Liu Date: Thu, 12 Nov 2026 08:36:40 +0000 Subject: [PATCH 2/3] fix(zipapp): also store symlinks with legacy external runfiles The packed copy of an external file is stored at the same runfiles path regardless of --legacy_external_runfiles, so the symlink target exists in both modes. Bazel 7 enables it by default. --- python/private/zipapp/py_zipapp_rule.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/zipapp/py_zipapp_rule.bzl b/python/private/zipapp/py_zipapp_rule.bzl index 3bef6b38e7..868da3f353 100644 --- a/python/private/zipapp/py_zipapp_rule.bzl +++ b/python/private/zipapp/py_zipapp_rule.bzl @@ -130,7 +130,7 @@ def _build_manifest(ctx, manifest, runfiles, explicit_symlinks, inputs): # On Windows, creating symlinks when extracting needs extra privileges, so # only the explicit venv symlinks are stored as symlinks there. - if py_internal.get_legacy_external_runfiles(ctx) or is_windows_platform(ctx): + if is_windows_platform(ctx): manifest.add_all(runfiles.symlinks, map_each = _map_zip_symlinks) manifest.add_all(runfiles.root_symlinks, map_each = _map_zip_root_symlinks) else: From 6340da4b9dee15fb6139303f1e400182fd42ee9d Mon Sep 17 00:00:00 2001 From: Frank Liu Date: Thu, 12 Nov 2026 08:44:47 +0000 Subject: [PATCH 3/3] fix(zipapp): keep the file for a runfiles symlink at its target's own path A runfiles symlink whose path is its target file's own runfiles path would be stored as a symlink to itself, and symlink entries take precedence over the file at the same path. --- python/private/zipapp/py_zipapp_rule.bzl | 14 +++++++++----- tests/py_zipapp/BUILD.bazel | 9 +++++++++ tests/py_zipapp/self_root_symlink.bzl | 15 +++++++++++++++ tests/py_zipapp/self_root_symlink.txt | 1 + tests/py_zipapp/venv_zipapp_test.py | 15 +++++++++++++++ 5 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 tests/py_zipapp/self_root_symlink.bzl create mode 100644 tests/py_zipapp/self_root_symlink.txt diff --git a/python/private/zipapp/py_zipapp_rule.bzl b/python/private/zipapp/py_zipapp_rule.bzl index 868da3f353..19baa18770 100644 --- a/python/private/zipapp/py_zipapp_rule.bzl +++ b/python/private/zipapp/py_zipapp_rule.bzl @@ -108,11 +108,15 @@ def _map_runfiles_symlinks(ctx, runfiles): ("", runfiles.root_symlinks, _map_zip_root_symlinks), ]: for entry in entries.to_list(): - if entry.target_file in packed: - lines.append("symlink|{}|{}".format( - paths.join(prefix, entry.path), - runfiles_root_path(ctx, entry.target_file.short_path), - )) + link_path = paths.normalize(paths.join(prefix, entry.path)) + target_path = paths.normalize( + runfiles_root_path(ctx, entry.target_file.short_path), + ) + + # A link at the target's own path would point to itself and take + # the place of the file, so keep storing the file there. + if entry.target_file in packed and link_path != target_path: + lines.append("symlink|{}|{}".format(link_path, target_path)) else: lines.append(map_entry(entry)) return lines diff --git a/tests/py_zipapp/BUILD.bazel b/tests/py_zipapp/BUILD.bazel index ff42539c96..7e2e6edea0 100644 --- a/tests/py_zipapp/BUILD.bazel +++ b/tests/py_zipapp/BUILD.bazel @@ -5,6 +5,7 @@ load("//python:py_test.bzl", "py_test") load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") # buildifier: disable=bzl-visibility load("//python/zipapp:py_zipapp_binary.bzl", "py_zipapp_binary") load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT") +load(":self_root_symlink.bzl", "self_root_symlink") py_binary( name = "venv_bin", @@ -17,12 +18,20 @@ py_binary( "//python/config_settings:bootstrap_impl": "script", }, }), + data = [":self_root_symlink"], main = "main.py", # Pyrefly's bazel-check validator rejects explicit import paths with '.' components from :bin_deps. tags = ["no-pyrefly"], deps = [":bin_deps"], ) +# A runfiles root symlink at its target's own runfiles path. The zipapp must +# store the file there, not a symlink to itself. +self_root_symlink( + name = "self_root_symlink", + src = "self_root_symlink.txt", +) + py_zipapp_binary( name = "venv_zipapp", binary = ":venv_bin", diff --git a/tests/py_zipapp/self_root_symlink.bzl b/tests/py_zipapp/self_root_symlink.bzl new file mode 100644 index 0000000000..7b0e8d165e --- /dev/null +++ b/tests/py_zipapp/self_root_symlink.bzl @@ -0,0 +1,15 @@ +"""A rule whose runfiles root symlink is at its target file's own path.""" + +def _self_root_symlink_impl(ctx): + f = ctx.file.src + return [DefaultInfo(runfiles = ctx.runfiles( + files = [f], + root_symlinks = {ctx.workspace_name + "/" + f.short_path: f}, + ))] + +self_root_symlink = rule( + implementation = _self_root_symlink_impl, + attrs = { + "src": attr.label(allow_single_file = True, mandatory = True), + }, +) diff --git a/tests/py_zipapp/self_root_symlink.txt b/tests/py_zipapp/self_root_symlink.txt new file mode 100644 index 0000000000..38a43a4faa --- /dev/null +++ b/tests/py_zipapp/self_root_symlink.txt @@ -0,0 +1 @@ +A file whose runfiles root symlink is at its own path. diff --git a/tests/py_zipapp/venv_zipapp_test.py b/tests/py_zipapp/venv_zipapp_test.py index a6b5830dee..55e75fed7c 100644 --- a/tests/py_zipapp/venv_zipapp_test.py +++ b/tests/py_zipapp/venv_zipapp_test.py @@ -135,6 +135,21 @@ def test_zipapp_structure(self): f"{target} should be the packed file, not another symlink", ) + # A runfiles root symlink at its target's own path is stored as the + # file, not as a symlink to itself. + self_link = next( + (n for n in namelist if n.endswith("/py_zipapp/self_root_symlink.txt")), + None, + ) + self.assertIsNotNone(self_link, "\n".join(namelist)) + info = zf.getinfo(self_link) + self.assertNotEqual( + (info.external_attr >> 16) & 0o170000, + 0o120000, + f"{self_link} should be a file, not a symlink", + ) + self.assertIn(b"its own path", zf.read(self_link)) + # Verify _bazel_site_init.py exists in site-packages self.assertHasPathMatchingSuffix( namelist, "/site-packages/_bazel_site_init.py"