Skip to content

fix(zipapp): store venv runfiles symlinks as symlinks, not copies - #4480

Open
gfrankliu wants to merge 3 commits into
bazel-contrib:mainfrom
gfrankliu:fix/zipapp-venv-symlinks
Open

gfrankliu wants to merge 3 commits into
bazel-contrib:mainfrom
gfrankliu:fix/zipapp-venv-symlinks

Conversation

@gfrankliu

@gfrankliu gfrankliu commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

With venvs_site_packages=yes, the venv links some site-packages entries file
by file, as runfiles symlinks into the pip repos: every shared library since
#3538, plus the files of directories that can't be linked as a whole. The
zipapp manifest maps these to rf-symlink/rf-root-symlink entries, which
store the target's content. The same file is also packed at its own runfiles
path (rf-file), so each one is stored twice, e.g.

runfiles/+pip+..._markupsafe_.../site-packages/markupsafe/_speedups.cpython-314-x86_64-linux-gnu.so
runfiles/_main/.../_venv_bin.venv/lib/python3.14/site-packages/markupsafe/_speedups.cpython-314-x86_64-linux-gnu.so  (second full copy)

Native extensions are usually the largest files in a package, so this adds up.
In our repo:

zipapp before after
gRPC server (grpcio, cryptography, protobuf, numpy, ...) 92.6 MB 71.9 MB
device flashing tool 34.9 MB 29.2 MB
uploader (google-cloud-bigquery, pyarrow, pandas, ...), venv copies 300 entries, 226 MB 3 entries

To fix, _build_manifest emits a runfiles symlink whose target_file is also
packed in the zipapp as the zipper's existing symlink|<path>|<target> entry,
a relative symlink to the packed copy, the same way explicit venv symlinks are
stored. zip_main_template's extract_zip already recreates symlinks when
extracting, so the venv layout on disk is unchanged. Symlinks whose target is
not packed, and Windows targets, keep the old behavior. The packed copy's path
doesn't depend on --legacy_external_runfiles, so it applies with Bazel 7's
default too. On Windows, creating a symlink when extracting needs
Developer Mode or admin rights; venv file links there are already stored as
symlinks through the explicit venv symlinks, so nothing changes for them.

We rebuilt the zipapps above with this change and ran them outside Bazel; the
native imports (grpc's cygrpc, cryptography's _rust, protobuf's upb) load
through the symlinks.

  • Adds @dev_pip//markupsafe (a wheel with a native extension) to the
    py_zipapp test binary. venv_zipapp_test imports markupsafe._speedups
    from the zipapp and asserts its venv entry is a symlink to the packed file
    from the pip repo. Without the fix it fails with "is not a symlink".

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.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Aliases matching their target paths create unreadable self-referencing symlinks, and structure tests incorrectly require symlinks in legacy-runfiles mode.

2 open issues
What changed in this PR

This PR reduces rules_python zipapp size by storing eligible venv runfiles aliases as relative symlinks rather than duplicate file contents.

Changes:

  • Reuses packed target files through symlinks, retaining existing behavior on Windows and with legacy external runfiles.
  • Adds MarkupSafe native-extension import and archive-layout checks.
  • Documents the size optimization in a release note.
File Description
tests/​py_zipapp/​venv_zipapp_test.py Checks native imports and packed venv symlink targets.
tests/​py_zipapp/​main.py Imports MarkupSafe’s native extension in the test application.
tests/​py_zipapp/​BUILD.bazel Adds MarkupSafe to the zipapp test dependencies.
python/​private/​zipapp/​py_zipapp_rule.bzl Emits relative symlinks for aliases whose targets are also packed.
news/​zipapp-venv-symlinks.fixed.md Describes the reduced duplication in zipapps.

Review effort: Balanced

Comment on lines +111 to +117
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))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 6340da4: a runfiles symlink at its target's own runfiles path keeps the existing file mapping. Added a self_root_symlink fixture to venv_zipapp_test; without the guard it fails with "should be a file, not a symlink".

None,
)
self.assertIsNotNone(venv_file, "\n".join(namelist))
self.assertZipEntryIsSymlink(zf, venv_file)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 9601a92 by removing the --legacy_external_runfiles fallback instead: the zipper stores an external file at the same runfiles path in both modes, so the symlink target exists either way. The tests pass on Bazel 7.7.2, where the flag defaults to true.

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.
… 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants