Address PR #942 review comments + buildifier fix#960
Address PR #942 review comments + buildifier fix#960thesayyn wants to merge 23 commits intonew_layeringfrom
Conversation
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
… py_binary (#949) ### Changes are visible to end-users: no ### Test plan - Covered by existing tests
### Changes are visible to end-users: no ### Test plan - New test cases added
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
Added note about using pytest_main and wrapper macro for py_test. --- ### Changes are visible to end-users: yes/no <!-- If no, please delete this section. --> - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
Add a UV module extension/toolchain for fetching UV via bazel. ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The `uv` module extension's new `toolchain()` tag downloads a hermetic uv and publishes `@uv` (host alias) plus `@uv//:all` (per-platform toolchains). ### Test plan - Covered by existing test cases - New test cases added - Manual testing; `bazel run @UV -- --help`
|
|
|
Please dont spam like this lol 🙏 |
- normalize_label: handle Bazel 8 (`+`) and Bazel 9 (`~`) module-extension separators by anchoring on `whl_install__` instead of the full prefix. - _py_image_layer_impl: collapse double-bookkeeping of all_tars/dep_tars into a single snapshot before the source layer is appended; simplify pip dedup to a single dict pass (per @akafael). - py/private/BUILD.bazel: drop @rules_python load and use rules_py's own py_binary rule for the validator binary (per @jbedard). - adder visibility: revert to the prior explicit list (per @jbedard). - buildifier: split multi-line _declare_group_tar calls; blank-line fix.
0fa9da8 to
46ea075
Compare
|
The e2e cases (cases/oci/py_image_layer, py_venv_image_layer) live in a separate Bazel module via local_path_override. They depend on @aspect_rules_py//py/tests/internal-deps/adder, which requires cross-module visibility — only //visibility:public satisfies that. Reverting the explicit list breaks e2e analysis with: target '@@aspect_rules_py+//py/tests/internal-deps/adder:adder' is not visible from target '//cases/oci/py_image_layer:my_app_bin' This was the original change in #942; restoring it with a comment explaining why public is required for this fixture.
main commits f1fec29 (refactor: do not duplicate site_packages into DefaultInfo+runfiles of py_binary) and e735612 (fix: check main by resolved file basename) shifted the py_binary launcher script content by 6 bytes. Refresh the three py_image_layer goldens that pin its size. Venv launcher goldens (py_venv_image_layer) likely need similar updates but the buildkite log didn't surface their specific diffs; will iterate on the next CI run.
main commit 6dc87d5 (chore: upgrade llvm module to 0.7.1) changed how `--stripopt=--strip-all` strips the venv-staged python binary on linux amd64. Refresh the snapshot. Arm64 likely needs similar but the buildkite log only surfaced amd64's diff; iterating.
The new py_image_layer puts the binary at ./app.runfiles/_main/<short_path> by default. The test had the binary at /cases/interpreter-features-836/ check_no_turtle and the venv at .../check_no_turtle.runfiles/_main/... which never matched the actual layout (the file-existence tests were trivially passing on bogus non-existent paths). Mirror what cases/oci/py_venv_image_layer does: remap to root=/app via strip_prefix, run the binary as /app, and check venv files at /app.runfiles/_main/cases/interpreter-features-836/.no-turtle-test/... which is where py_image_layer actually lays them out.



Addresses review comments on #942 and the buildifier CI failure. Targets
new_layeringso the changes flow into the existing PR.Changes
normalize_label(per @jbedard): replaced the hardcodedaspect_rules_py++uv+whl_install__prefix match with an_extract_whl_install_pkghelper anchored on the stablewhl_install__substring, so the same code path works under both Bazel 8 (+) and Bazel 9 (~) module-extension separators._py_image_layer_impl(per @akafael):seen_labels+all_pkgslist +pkg_by_labelrebuild into a single dedup-by-label dict pass;all_pkgs = pkg_by_label.values().dep_tarsdouble-bookkeeping inside the loops — snapshotdep_tars = list(all_tars)once before the source layer is appended.py/private/BUILD.bazel(per @jbedard): dropped the@rules_python//python:defs.bzlload; the validator now uses rules_py's ownpy_binaryrule.py/tests/internal-deps/adder/BUILD.bazel(per @jbedard): reverted visibility back to the prior explicit list. Flagging that this may breake2e/cases/oci/{py_image_layer,py_venv_image_layer}consumers — happy to widen the list if CI complains._declare_group_tarcalls split per arg.Test plan
//cases/interpreter-features-836:testand//cases/oci/py_venv_image_layer:py_amd64_image_command_test— the venv command test was Bazel-9-only and may resolve via the separator fix; the interpreter-features test is independent and worth checking buildkite logs for.Generated by Claude Code