fix(release): Configure the interpreter shim via a prebuilt toolchain (#568)

This fixes a breaking regression in v1.5.0 caused by the new interpreter
shim being referenced by source label and not via a toolchain which can
be supplied with prebuild artifacts.

Included in this change is a refactor consolidating our source of truth
for the list of prebuilt tools to configure down to `tools.bzl` which is
now shared with `tools/release` among other things, preventing how I got
myself into this mess.

As a further bit of paranoia this changeset also reworks the integration
tests so that instead of doing e2e testing with assets from a checkpoint
release (previously 1.1.0) we instead do a full build of the release
candidate assets, update the asset integrity file, run a local
fileserver hosting them and then do builds using the locally hosted
assets.

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes/no
- Breaking change (forces users to change their own code or config):
yes/no
- Suggested release notes appear below: yes/no

### Test plan

- [x] Existing test cases still green
- [x] Preexisting and inadequate e2e tests updated to cover this case

Author: arrdem

e2e/use_release/minimal_download_test.sh

@@ -9,13 +9,41 @@ if [ "$ARCH" == "x86_64" ]; then
     ALLOWED="rules_py_tools.${OS}_amd64"
 fi
 
+# FIXME: Find a port we can bind.
+PORT=7654
+touch devserver.pid
+PIDFILE=$(realpath ./devserver.pid)
+
+(
+    cd ../../
+
+    # First we produce a release artifact matrix
+    mkdir artifacts
+    DEST=$(realpath artifacts) bazel run //tools/release:copy_release_artifacts
+
+    # We kick off a dev http server on localhost
+    #
+    # Bazel will block until the devserver starts, then it forks to the background
+    # which will unblock the Bazel server & shell execution.
+    bazel run //tools/e2e:fileserver -- --port=$PORT --dir="$(realpath ./artifacts)" --background --pidfile="$PIDFILE"
+
+    # Now we need to update the integrity file
+    bazel run //tools/e2e:integrity -- --dir="$(realpath ./artifacts)" --target="$(realpath ./tools/integrity.bzl)"
+
+    # Note that we don't have to scrub the bazel server because we're using a separate output_base below for the cases.
+)
+
+# Set an environment flag which will make rules_py treat localhost as a mirror/artifact source
+export RULES_PY_RELEASE_URL="http://localhost:$PORT/{filename}"
+
 #############
 # Test bzlmod
 (
     cd ../..
     # Create the .orig file, whether there's a mismatch or not
     patch -p1 --backup < .bcr/patches/*.patch
 )
+
 OUTPUT_BASE=$(mktemp -d)
 output=$(bazel "--output_base=$OUTPUT_BASE" run --enable_bzlmod //src:main)
 if [[ "$output" != "hello world" ]]; then
@@ -77,3 +105,6 @@ bazel test --test_output=streamed //...
   bazel run //examples/py_venv:external_venv
   bazel run --stamp //examples/py_venv:external_venv
 )
+
+# Shut down the devserver
+kill "$(cat $PIDFILE)"

py/private/py_venv/py_venv.bzl

@@ -5,7 +5,7 @@ load("@aspect_bazel_lib//lib:paths.bzl", "BASH_RLOCATION_FUNCTION", "to_rlocatio
 load("//py/private:py_library.bzl", _py_library = "py_library_utils")
 load("//py/private:py_semantics.bzl", _py_semantics = "semantics")
 load("//py/private:transitions.bzl", "python_version_transition")
-load("//py/private/toolchain:types.bzl", "PY_TOOLCHAIN")
+load("//py/private/toolchain:types.bzl", "PY_TOOLCHAIN", "SHIM_TOOLCHAIN")
 
 VirtualenvInfo = provider(
     doc = """
@@ -74,6 +74,8 @@ def _py_venv_base_impl(ctx):
 
     py_toolchain = _py_semantics.resolve_toolchain(ctx)
 
+    py_shim = ctx.toolchains[SHIM_TOOLCHAIN]
+
     # Check for duplicate virtual dependency names. Those that map to the same resolution target would have been merged by the depset for us.
     virtual_resolution = _py_library.resolve_virtuals(ctx)
     imports_depset = _py_library.make_imports_depset(ctx, extra_imports_depsets = virtual_resolution.imports)
@@ -142,7 +144,7 @@ def _py_venv_base_impl(ctx):
         executable = ctx.file._venv_tool,
         arguments = [
             "--location=" + venv_dir.path,
-            "--python=" + ctx.file._interpreter_shim.path,
+            "--python=" + py_shim.bin.path,
             "--pth-file=" + site_packages_pth_file.path,
             "--env-file=" + env_file.path,
             "--bin-dir=" + ctx.bin_dir.path,
@@ -158,9 +160,9 @@ def _py_venv_base_impl(ctx):
             ctx.runfiles(files = [
                 site_packages_pth_file,
                 env_file,
-                ctx.file._interpreter_shim,
                 ctx.file._venv_tool,
             ]),
+            py_shim.default_info.default_runfiles,
         ]).files,
         outputs = [
             venv_dir,
@@ -300,11 +302,6 @@ A collision can occur when multiple packages providing the same file are install
     "_interpreter_version_flag": attr.label(
         default = "//py:interpreter_version",
     ),
-    "_interpreter_shim": attr.label(
-        allow_single_file = True,
-        doc = "An interpreter shim to use. Should be a single file executable.",
-        default = "//py/tools/venv_shim",
-    ),
     # Required for py_version attribute
     "_allowlist_function_transition": attr.label(
         default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
@@ -367,6 +364,7 @@ py_venv_base = struct(
     test_attrs = _test_attrs,
     toolchains = [
         PY_TOOLCHAIN,
+        SHIM_TOOLCHAIN,
     ],
     cfg = python_version_transition,
 )

py/private/toolchain/BUILD.bazel

@@ -15,6 +15,11 @@ toolchain_type(
     visibility = ["//visibility:public"],
 )
 
+toolchain_type(
+    name = "shim_toolchain_type",
+    visibility = ["//visibility:public"],
+)
+
 bzl_library(
     name = "autodetecting",
     srcs = ["autodetecting.bzl"],
@@ -25,7 +30,9 @@ bzl_library(
 bzl_library(
     name = "tools",
     srcs = ["tools.bzl"],
-    visibility = ["//py:__subpackages__"],
+    visibility = [
+        "//py:__subpackages__",
+    ],
 )
 
 bzl_library(

py/private/toolchain/repo.bzl

@@ -17,7 +17,9 @@ This guidance tells us how to avoid that: we put the toolchain targets in the al
 with only the toolchain attribute pointing into the platform-specific repositories.
 """
 
-load("//py/private/toolchain:tools.bzl", "RUST_BIN_CFG", "TOOLCHAIN_PLATFORMS")
+load("//py/private/toolchain:tools.bzl", "TOOLCHAIN_PLATFORMS", "TOOL_CFGS")
+load("//tools:integrity.bzl", "RELEASED_BINARY_INTEGRITY")
+load("//tools:version.bzl", "VERSION")
 
 def _toolchains_repo_impl(repository_ctx):
     build_content = """# Generated by toolchains_repo.bzl
@@ -27,7 +29,7 @@ def _toolchains_repo_impl(repository_ctx):
 # so you don't normally need to interact with these targets.
 
 """
-    for tool, cfg in RUST_BIN_CFG.items():
+    for bin in TOOL_CFGS:
         for [platform, meta] in TOOLCHAIN_PLATFORMS.items():
             build_content += """
 # Declare a toolchain Bazel will select for running {tool} on the {cfg} platform.
@@ -37,11 +39,12 @@ toolchain(
     # Bazel does not follow this attribute during analysis, so the referenced repo
     # will only be fetched if this toolchain is selected.
     toolchain = "@{user_repository_name}.{platform}//:{tool}_toolchain",
-    toolchain_type = "@aspect_rules_py//py/private/toolchain:{tool}_toolchain_type",
+    toolchain_type = "{toolchain_type}",
 )
 """.format(
-                cfg = cfg,
-                tool = tool,
+                cfg = bin.cfg,
+                tool = bin.name,
+                toolchain_type = bin.toolchain_type,
                 platform = platform,
                 user_repository_name = repository_ctx.attr.user_repository_name,
                 compatible_with = meta.compatible_with,
@@ -75,3 +78,56 @@ prerelease_toolchains_repo = repository_rule(
     By doing this, we can avoid those register_toolchains callsites needing to be conditional on IS_PRERELEASE
     """,
 )
+
+def _prebuilt_tool_repo_impl(rctx):
+    build_content = """\
+# Generated by @aspect_rules_py//py/private/toolchain:tools.bzl
+load("@aspect_rules_py//py/private/toolchain:tools.bzl", "py_tool_toolchain")
+
+package(default_visibility = ["//visibility:public"])
+"""
+
+    # For manual testing, override these environment variables
+    # TODO: use rctx.getenv when available, see https://github.com/bazelbuild/bazel/pull/20944
+    release_fork = "aspect-build"
+    release_version = VERSION
+    if "RULES_PY_RELEASE_FORK" in rctx.os.environ:
+        release_fork = rctx.os.environ["RULES_PY_RELEASE_FORK"]
+    if "RULES_PY_RELEASE_VERSION" in rctx.os.environ:
+        release_version = rctx.os.environ["RULES_PY_RELEASE_VERSION"]
+
+    url_template = "https://github.com/{release_fork}/rules_py/releases/download/v{release_version}/{filename}"
+    if "RULES_PY_RELEASE_URL" in rctx.os.environ:
+        url_template = rctx.os.environ["RULES_PY_RELEASE_URL"]
+
+    for tool in TOOL_CFGS:
+        filename = "-".join([
+            tool.name,
+            TOOLCHAIN_PLATFORMS[rctx.attr.platform].arch,
+            TOOLCHAIN_PLATFORMS[rctx.attr.platform].vendor_os_abi,
+        ])
+        url = url_template.format(
+            release_fork = release_fork,
+            release_version = release_version,
+            filename = filename,
+        )
+        rctx.download(
+            url = url,
+            sha256 = RELEASED_BINARY_INTEGRITY[filename],
+            executable = True,
+            output = tool.name,
+        )
+        build_content += """py_tool_toolchain(name = "{tool}_toolchain", bin = "{tool}", template_var = "{tool_upper}_BIN")\n""".format(
+            tool = tool.name,
+            tool_upper = tool.name.upper(),
+        )
+
+    rctx.file("BUILD.bazel", build_content)
+
+prebuilt_tool_repo = repository_rule(
+    doc = "Download pre-built binary tools and create concrete toolchains for them",
+    implementation = _prebuilt_tool_repo_impl,
+    attrs = {
+        "platform": attr.string(mandatory = True, values = TOOLCHAIN_PLATFORMS.keys()),
+    },
+)

py/private/toolchain/shim/BUILD.bazel

@@ -0,0 +1,9 @@
+load("//py/private/toolchain:tools.bzl", "source_toolchain")
+
+source_toolchain(
+    name = "shim",
+    bin = "//py/tools/venv_shim",
+    # FIXME: Use the target cfg?
+    # See notes in the venv setup
+    toolchain_type = "//py/private/toolchain:shim_toolchain_type",
+)

py/private/toolchain/tools.bzl

@@ -1,15 +1,35 @@
 """Declaration of concrete toolchains for our Rust tools"""
 
-load("//tools:integrity.bzl", "RELEASED_BINARY_INTEGRITY")
-load("//tools:version.bzl", "VERSION")
+def PrebuiltToolConfig(
+        target,
+        cfg = "target",
+        name = None,
+        toolchain = None,
+        toolchain_type = None):
+    name = name or Label(target).name
+
+    # FIXME: The source_toolchain macro creates two targets, so we need to match them both
+    # But that makes this not really a label which is weird
+    toolchain = toolchain or "@aspect_rules_py//py/private/toolchain/{}/...".format(name)
+    toolchain_type = toolchain_type or "@aspect_rules_py//py/private/toolchain:{}_toolchain_type".format(name)
+
+    return struct(
+        target = target,
+        cfg = cfg,
+        name = name,
+        toolchain = toolchain,
+        toolchain_type = toolchain_type,
+    )
 
 # The expected config for each tool, whether it runs in an action or at runtime
-RUST_BIN_CFG = {
-    # unpack wheels happens inside an action
-    "unpack": "exec",
-    # creating the virtualenv happens when the binary is running
-    "venv": "target",
-}
+#
+# Note that this is the source of truth for how toolchains get registered and
+# for how they get prebuilt/patched in.
+TOOL_CFGS = [
+    PrebuiltToolConfig("//py/tools/unpack_bin:unpack", cfg = "exec"),
+    PrebuiltToolConfig("//py/tools/venv_bin:venv"),
+    PrebuiltToolConfig("//py/tools/venv_shim:shim"),
+]
 
 TOOLCHAIN_PLATFORMS = {
     "darwin_amd64": struct(
@@ -102,52 +122,3 @@ def source_toolchain(name, toolchain_type, bin):
         toolchain = toolchain_rule,
         toolchain_type = toolchain_type,
     )
-
-def _prebuilt_tool_repo_impl(rctx):
-    build_content = """\
-# Generated by @aspect_rules_py//py/private/toolchain:tools.bzl
-load("@aspect_rules_py//py/private/toolchain:tools.bzl", "py_tool_toolchain")
-
-package(default_visibility = ["//visibility:public"])
-"""
-
-    # For manual testing, override these environment variables
-    # TODO: use rctx.getenv when available, see https://github.com/bazelbuild/bazel/pull/20944
-    release_fork = "aspect-build"
-    release_version = VERSION
-    if "RULES_PY_RELEASE_FORK" in rctx.os.environ:
-        release_fork = rctx.os.environ["RULES_PY_RELEASE_FORK"]
-    if "RULES_PY_RELEASE_VERSION" in rctx.os.environ:
-        release_version = rctx.os.environ["RULES_PY_RELEASE_VERSION"]
-
-    for tool in RUST_BIN_CFG.keys():
-        filename = "-".join([
-            tool,
-            TOOLCHAIN_PLATFORMS[rctx.attr.platform].arch,
-            TOOLCHAIN_PLATFORMS[rctx.attr.platform].vendor_os_abi,
-        ])
-        url = "https://github.com/{}/rules_py/releases/download/v{}/{}".format(
-            release_fork,
-            release_version,
-            filename,
-        )
-        rctx.download(
-            url = url,
-            sha256 = RELEASED_BINARY_INTEGRITY[filename],
-            executable = True,
-            output = tool,
-        )
-        build_content += """py_tool_toolchain(name = "{tool}_toolchain", bin = "{tool}", template_var = "{tool_upper}_BIN")\n""".format(
-            tool = tool,
-            tool_upper = tool.upper(),
-        )
-
-    rctx.file("BUILD.bazel", build_content)
-
-prebuilt_tool_repo = repository_rule(
-    doc = "Download pre-built binary tools and create concrete toolchains for them",
-    implementation = _prebuilt_tool_repo_impl,
-    attrs = {
-        "platform": attr.string(mandatory = True, values = TOOLCHAIN_PLATFORMS.keys()),
-    },
-)

py/private/toolchain/types.bzl

@@ -4,5 +4,6 @@ PY_TOOLCHAIN = "@bazel_tools//tools/python:toolchain_type"
 SH_TOOLCHAIN = "@bazel_tools//tools/sh:toolchain_type"
 
 # Toolchain type for the virtual env creation tools.
-VENV_TOOLCHAIN = "@aspect_rules_py//py/private/toolchain:venv_toolchain_type"
+SHIM_TOOLCHAIN = "@aspect_rules_py//py/private/toolchain:shim_toolchain_type"
 UNPACK_TOOLCHAIN = "@aspect_rules_py//py/private/toolchain:unpack_toolchain_type"
+VENV_TOOLCHAIN = "@aspect_rules_py//py/private/toolchain:venv_toolchain_type"

py/toolchains.bzl

@@ -3,8 +3,8 @@
 load("@aspect_bazel_lib//lib:repositories.bzl", "register_tar_toolchains")
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file")
 load("//py/private/toolchain:autodetecting.bzl", _register_autodetecting_python_toolchain = "register_autodetecting_python_toolchain")
-load("//py/private/toolchain:repo.bzl", "prerelease_toolchains_repo", "toolchains_repo")
-load("//py/private/toolchain:tools.bzl", "TOOLCHAIN_PLATFORMS", "prebuilt_tool_repo")
+load("//py/private/toolchain:repo.bzl", "prebuilt_tool_repo", "prerelease_toolchains_repo", "toolchains_repo")
+load("//py/private/toolchain:tools.bzl", "TOOLCHAIN_PLATFORMS", "TOOL_CFGS")
 load("//tools:version.bzl", "IS_PRERELEASE")
 
 register_autodetecting_python_toolchain = _register_autodetecting_python_toolchain
@@ -25,10 +25,8 @@ def rules_py_toolchains(name = DEFAULT_TOOLS_REPOSITORY, register = True, is_pre
     if is_prerelease:
         prerelease_toolchains_repo(name = name)
         if register:
-            native.register_toolchains(
-                "@aspect_rules_py//py/private/toolchain/venv/...",
-                "@aspect_rules_py//py/private/toolchain/unpack/...",
-            )
+            for tool in TOOL_CFGS:
+                native.register_toolchains(tool.toolchain)
     else:
         for platform in TOOLCHAIN_PLATFORMS.keys():
             prebuilt_tool_repo(name = ".".join([name, platform]), platform = platform)

tools/e2e/BUILD.bazel

@@ -0,0 +1,13 @@
+load("//py:defs.bzl", "py_binary")
+
+py_binary(
+    name = "fileserver",
+    srcs = ["devserver.py"],
+    main = "devserver.py",
+)
+
+py_binary(
+    name = "integrity",
+    srcs = ["devintegrity.py"],
+    main = "devintegrity.py",
+)