Skip to content

fix(py_venv): patch bazel-runfiles root detection#1209

Open
jimmyt857 wants to merge 1 commit into
aspect-build:mainfrom
jimmyt857:jimmy/fix-bazel-runfiles-root
Open

fix(py_venv): patch bazel-runfiles root detection#1209
jimmyt857 wants to merge 1 commit into
aspect-build:mainfrom
jimmyt857:jimmy/fix-bazel-runfiles-root

Conversation

@jimmyt857

@jimmyt857 jimmyt857 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

When a py_venv target imports the PyPI bazel-runfiles package from a uv-installed wheel, bazel-runfiles derives its Python runfiles root by walking upward from runfiles/runfiles.py. That assumption is correct for the copy vendored under rules_python/python/runfiles, but not for a wheel installed under a whl_install repository. In the wheel case, the derived root points inside the wheel install tree, not at Bazel's runfiles root.

That breaks repository mapping and Rlocation() calls that rely on CurrentRepository(), unless every caller passes an explicit source_repo. Under Bazel, the launcher already exports RUNFILES_DIR or RUNFILES_MANIFEST_FILE, so those environment variables are the authoritative runfiles root.

This change extends the _virtualenv.py startup shim to patch runfiles.runfiles._FindPythonRunfilesRoot when Bazel runfiles env is present. The patch is lazy: if runfiles.runfiles is already imported, it patches the module immediately; otherwise it installs an import hook and patches the module when it is imported.

Because _virtualenv.py is included in several OCI layer snapshot fixtures, this also refreshes those snapshot listings for the expected _virtualenv.py byte-size change.

Regression Test

Adds e2e/cases/uv-bazel-runfiles-root, a standalone uv project depending on bazel-runfiles==1.1.0. Before the runtime patch, the test fails because _FindPythonRunfilesRoot() returns a path inside the wheel install tree instead of $RUNFILES_DIR. After the patch, the test also verifies that Rlocation() can find a runfile without passing an explicit source_repo.

Testing

  • Before the code fix, (cd e2e && bazel test //cases/uv-bazel-runfiles-root:test_runfiles_root --test_output=errors) failed with AssertionError on the runfiles-root assertion.
  • bazel run //:gazelle
  • pre-commit run buildifier --files py/private/py_venv/_virtualenv.py e2e/MODULE.bazel e2e/cases/uv-bazel-runfiles-root/BUILD.bazel e2e/cases/uv-bazel-runfiles-root/setup.MODULE.bazel
  • pre-commit run prettier --files py/private/py_venv/_virtualenv.py e2e/MODULE.bazel e2e/cases/uv-bazel-runfiles-root/BUILD.bazel e2e/cases/uv-bazel-runfiles-root/pyproject.toml e2e/cases/uv-bazel-runfiles-root/setup.MODULE.bazel e2e/cases/uv-bazel-runfiles-root/test_runfiles_root.py e2e/cases/uv-bazel-runfiles-root/uv.lock e2e/cases/uv-bazel-runfiles-root/runfiles_root_data.txt
  • pre-commit run typos --files py/private/py_venv/_virtualenv.py e2e/MODULE.bazel e2e/cases/uv-bazel-runfiles-root/BUILD.bazel e2e/cases/uv-bazel-runfiles-root/pyproject.toml e2e/cases/uv-bazel-runfiles-root/setup.MODULE.bazel e2e/cases/uv-bazel-runfiles-root/test_runfiles_root.py e2e/cases/uv-bazel-runfiles-root/uv.lock e2e/cases/uv-bazel-runfiles-root/runfiles_root_data.txt
  • bazel test //py/private/py_venv:all //py/tests/py-internal-venv:all //py/tests/external-deps:all //py/tests/py_venv_conflict:all
  • (cd e2e && bazel test //cases/uv-bazel-runfiles-root:test_runfiles_root --test_output=errors)
  • Regenerated the affected OCI layer listings with the write_source_file updater targets.
  • (cd e2e && bazel test //cases/oci/py_image_layer:my_app_layers_test_test //cases/oci/py_image_layer:my_app_layers_fp_test_test //cases/oci/py_image_layer:my_app_layers_multi_test_test //cases/oci/py_venv_image_layer:my_app_amd64_layers_test //cases/oci/py_venv_image_layer:my_app_arm64_layers_test //cases/uv-deps-650/crossbuild:app_amd64_layers_test //cases/uv-deps-650/crossbuild:app_arm64_layers_test //cases/uv-bazel-runfiles-root:test_runfiles_root --test_output=errors)
  • (cd e2e && USE_BAZEL_VERSION=9.x bazel test //cases/oci/py_image_layer:my_app_layers_test_test //cases/oci/py_image_layer:my_app_layers_fp_test_test //cases/oci/py_image_layer:my_app_layers_multi_test_test //cases/oci/py_venv_image_layer:my_app_amd64_layers_test //cases/oci/py_venv_image_layer:my_app_arm64_layers_test //cases/uv-bazel-runfiles-root:test_runfiles_root --test_tag_filters=-skip-on-bazel9 --test_output=errors)

@github-actions

github-actions Bot commented Jun 27, 2026

Copy link
Copy Markdown

py_binary startup benchmark

Version Mean (ms) Median (ms) ± stddev vs BCR vs main Build (s)
BCR 1.11.7 (baseline) 188.761 187.680 ±5.415 38.76
HEAD main 59.209 59.218 ±0.585 -68.6% 9.52
This PR 60.368 59.673 ±3.256 -68.0% +2.0% 6.70

Measured with hyperfine --warmup 5 --runs 50 on Linux
Gate: PR vs HEAD main (threshold: 10%). BCR is shown only as a historical baseline.
Build time: cold bazel build //:bench with isolated output base, no disk cache.

sys.path quality

Version sys.path entries distinct site-packages roots duplicate realpaths
BCR 1.11.7 (baseline) 6 1 0
HEAD main 7 2 0
This PR 7 2 0

sys.path quality measured by bench_syspath inside the assembled venv. Duplicate realpaths indicate symlink redundancy; many distinct site-packages roots suggest an inefficient venv layout.

✅ No regression detected (PR is +2.0% vs HEAD main)

@jimmyt857 jimmyt857 force-pushed the jimmy/fix-bazel-runfiles-root branch from 22bc0d1 to 8a9c80b Compare June 27, 2026 04:27
@xangcastle xangcastle self-requested a review June 27, 2026 04:44
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.

1 participant