feat: new py_image_layer by thesayyn · Pull Request #942 · aspect-build/rules_py

thesayyn · 2026-04-23T21:40:31Z

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): yes
Suggested release notes appear below: yes

TODO

Test plan

TODO

Covered by existing test cases
New test cases added
Manual testing; please provide instructions so we can reproduce: TODO

aspect-workflows · 2026-04-23T22:48:59Z

Bazel 8 (Test)

⚠️ Buildkite build #3307 failed.

Bazel 9 (Test)

⚠️ Buildkite build #3307 failed.

Bazel 8 (Test)

e2e

⚠️ Buildkite build #3307 failed.

Failed tests (1)

//cases/interpreter-features-836:test [k8-fastbuild]🔗

💡 To reproduce the test failures, run

bazel test //cases/interpreter-features-836:test

Bazel 9 (Test)

e2e

⚠️ Buildkite build #3307 failed.

Failed tests (2)

//cases/interpreter-features-836:test [k8-fastbuild]🔗
//cases/oci/py_venv_image_layer:py_amd64_image_command_test [k8-fastbuild]🔗

💡 To reproduce the test failures, run

bazel test //cases/oci/py_venv_image_layer:py_amd64_image_command_test //cases/interpreter-features-836:test

Bazel 8 (Test)

examples/uv_pip_compile

All tests were cache hits

1 test (100.0%) was fully cached saving 444ms.

Buildifier

Buildifier managed files require formatting

--- ./py/private/py_image_layer.bzl	2026-04-24 21:19:22.731041426 +0000
+++ /tmp/buildifier-tmp-1035270593	2026-04-24 21:19:34.790687811 +0000
@@ -375,6 +375,7 @@
             if _LayerInfo in py_tc:
                 tc_info = py_tc[_LayerInfo]
                 interpreter_layer = tc_info.interpreter_layer
+
                 # Only skip interpreter paths from the source layer when there
                 # IS a separate interpreter tar to route them to; otherwise the
                 # interpreter belongs in the default layer.
@@ -666,7 +667,9 @@
         if dep_label in pkg_by_label:
             continue
         tar_out = _declare_group_tar(
-            ctx, bsdtar, bsdtar_files,
+            ctx,
+            bsdtar,
+            bsdtar_files,
             "{}_{}.tar.gz".format(ctx.attr.name, group_name),
             group_name,
             dep[DefaultInfo].files,
@@ -704,7 +707,9 @@
             tar_out = prebuilt_group_tars[group_name]
         else:
             tar_out = _declare_group_tar(
-                ctx, bsdtar, bsdtar_files,
+                ctx,
+                bsdtar,
+                bsdtar_files,
                 "{}_{}.tar.gz".format(ctx.attr.name, group_name),
                 group_name,
                 depset(transitive = fp_by_group[group_name]),
@@ -726,7 +731,9 @@
     ungrouped_pkgs = [p for p in all_pkgs if len(p.layers) == 0 and p.merge_group == None]
     if ungrouped_pkgs:
         squashed_tar = _declare_group_tar(
-            ctx, bsdtar, bsdtar_files,
+            ctx,
+            bsdtar,
+            bsdtar_files,
             "{}_squashed.tar.gz".format(ctx.attr.name),
             "packages",
             depset(transitive = [p.files for p in ungrouped_pkgs]),
@@ -737,7 +744,9 @@
         dep_tars.append(squashed_tar)
 
     source_tar = _declare_group_tar(
-        ctx, bsdtar, bsdtar_files,
+        ctx,
+        bsdtar,
+        bsdtar_files,
         "{}_default.tar.gz".format(ctx.attr.name),
         "default",
     

... 23 chars truncated

💡 Run the following to apply the suggested formatting fixes

bazel run //:buildifier

akafael · 2026-04-28T12:41:13Z

+    seen_labels = {}
+    all_pkgs = []
+    for pkg in info.pip_packages.to_list():
+        if pkg.label not in seen_labels:
+            seen_labels[pkg.label] = True
+            all_pkgs.append(pkg)
+
+    pkg_by_label = {pkg.label: pkg for pkg in all_pkgs}


This code is doing a bit of double work. It manually filters for unique labels using a dictionary and a list, then immediately builds another dictionary at the end.

Since the final goal seems to be creating a dictionary mapped by labels (pkg_by_label), we can skip the intermediate list and the "seen" check entirely to reduce the memory usage.

Suggestion:

Option 1; (Shorter version, that keeps the last occurrence of each package)

Suggested change

seen_labels = {}

all_pkgs = []

for pkg in info.pip_packages.to_list():

if pkg.label not in seen_labels:

seen_labels[pkg.label] = True

all_pkgs.append(pkg)

pkg_by_label = {pkg.label: pkg for pkg in all_pkgs}

pkg_by_label = {pkg.label: pkg for pkg in info.pip_packages.to_list()}

all_pkgs = pkg_by_label.values()

Option 2: (Slightly modified version that keeps the first occurrence of each package)

Suggested change

seen_labels = {}

all_pkgs = []

for pkg in info.pip_packages.to_list():

if pkg.label not in seen_labels:

seen_labels[pkg.label] = True

all_pkgs.append(pkg)

pkg_by_label = {pkg.label: pkg for pkg in all_pkgs}

pkg_by_label = {}

for pkg in info.pip_packages.to_list():

if pkg.label not in pkg_by_label:

pkg_by_label[pkg.label] = pkg

all_pkgs = pkg_by_label.values()

``

oh nice, i actually did not write this code, so probably there is a lot of optimizations here.

This new version is really nice! It provides some great optimizations considering the previews version. These are just suggestions to improve even it further.

akafael · 2026-04-28T19:06:32Z

+        all_tars.append(squashed_tar)
+        dep_tars.append(squashed_tar)
+
+    source_tar = _declare_group_tar(
+        ctx, bsdtar, bsdtar_files,
+        "{}_default.tar.gz".format(ctx.attr.name),
+        "default",
+        info.source_files,
+        _source_map,
+        "Creating source layer for %s" % ctx.label,
+    )
+    all_tars.append(source_tar)


Suggestion: Eliminate Redundant Array Appends in _py_image_layer_impl

In _py_image_layer_impl, tar_out items are manually appended to both all_tars and dep_tars quite often.

Since dep_tars is essentially identical to all_tars except for the source_tar appended at the very end, you can entirely drop the dep_tars variable during iteration and simply clone the list at the end.

Proposed solution:

Just append to all_tars normally inside your loops

Before appending the source layer, clone all_tars to act as dep_tars

Suggested change

all_tars.append(squashed_tar)

dep_tars.append(squashed_tar)

source_tar = _declare_group_tar(

ctx, bsdtar, bsdtar_files,

"{}_default.tar.gz".format(ctx.attr.name),

"default",

info.source_files,

_source_map,

"Creating source layer for %s" % ctx.label,

)

all_tars.append(source_tar)

dep_tars = list(all_tars)

source_tar = _declare_group_tar(

ctx, bsdtar, bsdtar_files,

"{}_default.tar.gz".format(ctx.attr.name),

"default",

info.source_files,

_source_map,

"Creating source layer for %s" % ctx.label,

)

all_tars.append(source_tar)

jbedard · 2026-04-29T02:55:42Z

    deps = ["//py/private/pytest_shard"],
 )

+_rules_python_py_binary(


We need to move toward removing the rules_python dep. What is the reason for this?

jbedard · 2026-04-29T02:57:10Z

    imports = [".."],
    # This library contributes to the container test, testing we can pull in and use a library from another
    # package in the repo.
-    visibility = [


jbedard · 2026-04-29T02:59:26Z

+
+_TAR_TOOLCHAIN = "@tar.bzl//tar/toolchain:type"
+
+_WHL_INSTALL_PREFIX = "aspect_rules_py++uv+whl_install__"


bazel 8 vs 9 have different separators (+ vs ~) so this will fail

feat: new py_image_layer

b91a49a

thesayyn force-pushed the new_layering branch from cb2e771 to b91a49a Compare April 23, 2026 22:48

thesayyn added 3 commits April 23, 2026 18:46

1p tiers

f5267a1

fix

937af06

interpreter sharing

e0c0f11

akafael reviewed Apr 28, 2026

View reviewed changes

jbedard reviewed Apr 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: new py_image_layer#942

feat: new py_image_layer#942
thesayyn wants to merge 4 commits intomainfrom
new_layering

thesayyn commented Apr 23, 2026

Uh oh!

aspect-workflows Bot commented Apr 23, 2026 •

edited

Loading

Uh oh!

akafael Apr 28, 2026 •

edited

Loading

Uh oh!

thesayyn Apr 28, 2026

Uh oh!

akafael Apr 28, 2026

Uh oh!

akafael Apr 28, 2026

Uh oh!

jbedard Apr 29, 2026

Uh oh!

jbedard Apr 29, 2026

Uh oh!

jbedard Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		_TAR_TOOLCHAIN = "@tar.bzl//tar/toolchain:type"

		_WHL_INSTALL_PREFIX = "aspect_rules_py++uv+whl_install__"

Uh oh!

Conversation

thesayyn commented Apr 23, 2026

Changes are visible to end-users: yes

Test plan

Uh oh!

aspect-workflows Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

e2e

e2e

examples/uv_pip_compile

Buildifier

Uh oh!

akafael Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aspect-workflows Bot commented Apr 23, 2026 •

edited

Loading

akafael Apr 28, 2026 •

edited

Loading