From 7e9c88610590f65a15a888807b0e81c8f20ae2e7 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Wed, 6 May 2026 16:23:42 -0700 Subject: [PATCH] refactor: split py_venv binary vs lib rules --- py/private/py_venv/py_venv.bzl | 124 +++++++++++++++++++++++---------- 1 file changed, 88 insertions(+), 36 deletions(-) diff --git a/py/private/py_venv/py_venv.bzl b/py/private/py_venv/py_venv.bzl index 4b9043de3..9df68acd1 100644 --- a/py/private/py_venv/py_venv.bzl +++ b/py/private/py_venv/py_venv.bzl @@ -174,7 +174,9 @@ def _py_venv_rule_impl(ctx): ), ] -_attrs = dict({ +# Attrs read by both the executable and lib variants — venv assembly, +# package-collision detection, py_library inputs. +_lib_attrs = dict({ "dep_group": attr.string( default = "", doc = """The name of a configured dependency group within which to resolve dependencies. @@ -199,10 +201,6 @@ two rules share the underlying collision detector. default = "warning", values = ["error", "warning", "ignore"], ), - "interpreter_options": attr.string_list( - doc = "Additional options to pass to the Python interpreter.", - default = [], - ), "include_system_site_packages": attr.bool( default = False, doc = """`pyvenv.cfg` feature flag for the `include-system-site-packages` key.""", @@ -211,29 +209,10 @@ two rules share the underlying collision detector. default = False, doc = """`pyvenv.cfg` feature flag for the `aspect-include-user-site-packages` extension key.""", ), - "debug": attr.bool( - default = False, - ), - "env": attr.string_dict( - doc = """Environment variables to set when running the venv (and the -sibling py_binary/py_test consumer when `expose_venv = True` is used). -Binary-level `env` wins on key conflicts.""", - default = {}, - ), - "env_inherit": attr.string_list( - doc = """Names of environment variables to pass through from the invoking -environment. Forwarded to the sibling py_binary/py_test consumer -(when `expose_venv = True` is used) alongside `env`.""", - default = [], - ), # Required for py_version attribute "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", ), - "_run_tmpl": attr.label( - allow_single_file = True, - default = ":venv.tmpl.sh", - ), "_runfiles_lib": attr.label( default = "@bazel_tools//tools/bash/runfiles", ), @@ -253,7 +232,35 @@ environment. Forwarded to the sibling py_binary/py_test consumer ), }) -_attrs.update(**_py_library.attrs) +_lib_attrs.update(**_py_library.attrs) + +# Attrs only the executable variant reads — launcher template, REPL +# flags, env vars forwarded via RunEnvironmentInfo. +_attrs = _lib_attrs | dict({ + "interpreter_options": attr.string_list( + doc = "Additional options to pass to the Python interpreter.", + default = [], + ), + "debug": attr.bool( + default = False, + ), + "env": attr.string_dict( + doc = """Environment variables to set when running the venv (and the +sibling py_binary/py_test consumer when `expose_venv = True` is used). +Binary-level `env` wins on key conflicts.""", + default = {}, + ), + "env_inherit": attr.string_list( + doc = """Names of environment variables to pass through from the invoking +environment. Forwarded to the sibling py_binary/py_test consumer +(when `expose_venv = True` is used) alongside `env`.""", + default = [], + ), + "_run_tmpl": attr.label( + allow_single_file = True, + default = ":venv.tmpl.sh", + ), +}) _py_venv = rule( doc = """Build a Python virtual environment and execute its interpreter.""", @@ -264,6 +271,39 @@ _py_venv = rule( cfg = python_version_transition, ) +def _py_venv_lib_rule_impl(ctx): + """Non-executable variant of `_py_venv` — same providers but no + launcher and no RunEnvironmentInfo (Bazel rejects it on + non-executable targets; py_binary.bzl gates its read on + `if RunEnvironmentInfo in venv`).""" + shared = _assemble_shared(ctx) + return [ + DefaultInfo(runfiles = shared.runfiles), + VirtualenvInfo( + bin_python = shared.venv.bin_python, + venv_name = shared.venv.venv_name, + imports = shared.imports_depset, + wheels = shared.wheels_depset, + transitive_sources = shared.srcs_depset, + ), + coverage_common.instrumented_files_info( + ctx, + source_attributes = ["srcs"], + dependency_attributes = ["deps"], + extensions = ["py"], + ), + ] + +# Internal-only non-executable variant. Uses `_lib_attrs` — the +# launcher-only attrs (`debug`, `interpreter_options`, `_run_tmpl`, +# `env`, `env_inherit`) aren't part of its rule contract. +_py_venv_lib = rule( + implementation = _py_venv_lib_rule_impl, + attrs = _lib_attrs, + toolchains = [PY_TOOLCHAIN], + cfg = python_version_transition, +) + def _wrap_with_debug(rule): def helper(**kwargs): kwargs["debug"] = select({ @@ -290,27 +330,36 @@ _VENV_ONLY_ATTRS = [ "include_user_site_packages", "python_version", "dep_group", - "env", - "env_inherit", ] + +# Shared attrs are only forwarded when the venv is the executable +# variant (`expose_venv = True`). The lib variant doesn't read them, so +# leave them on the launcher kwargs only. _SHARED_ATTRS = [ # Launcher constructs `python main.py`; venv forwards them # to its REPL so `bazel run :name.venv` matches the binary's flags. "interpreter_options", + # Forwarded so `bazel run :name.venv` sees the same env as `bazel + # run :name`. The launcher reads venv's RunEnvironmentInfo too, so + # the duplicate set is harmless (same keys, same values). + "env", + "env_inherit", ] -def _split_kwargs_for_venv(kwargs): - """Build the kwargs dict to pass to `py_venv`. `kwargs` is mutated: - venv-only attrs are popped; shared attrs are copied (left in - `kwargs` so they also reach the launcher). +def _split_kwargs_for_venv(kwargs, expose_venv): + """Build the kwargs dict to pass to the venv rule. `kwargs` is + mutated: venv-only attrs are popped; shared attrs are copied (left + in `kwargs` so they also reach the launcher) — but only for the + executable variant. """ venv_kwargs = {} for name in _VENV_ONLY_ATTRS: if name in kwargs: venv_kwargs[name] = kwargs.pop(name) - for name in _SHARED_ATTRS: - if name in kwargs: - venv_kwargs[name] = kwargs[name] + if expose_venv: + for name in _SHARED_ATTRS: + if name in kwargs: + venv_kwargs[name] = kwargs[name] return venv_kwargs def py_binary_with_venv(py_rule, name, main, srcs = [], deps = [], data = [], imports = [], tags = None, testonly = None, visibility = None, isolated = True, expose_venv = False, **kwargs): @@ -327,7 +376,7 @@ def py_binary_with_venv(py_rule, name, main, srcs = [], deps = [], data = [], im `include_*_site_packages`, `interpreter_options`) land on the sibling venv. """ - venv_kwargs = _split_kwargs_for_venv(kwargs) + venv_kwargs = _split_kwargs_for_venv(kwargs, expose_venv) venv_kwargs["srcs"] = srcs venv_kwargs["deps"] = deps venv_kwargs["imports"] = imports @@ -340,12 +389,14 @@ def py_binary_with_venv(py_rule, name, main, srcs = [], deps = [], data = [], im venv_label = "{}.venv".format(name) venv_visibility = visibility venv_tags = None + venv_rule = py_venv else: venv_label = "_{}.venv".format(safe_name) venv_visibility = ["//visibility:private"] venv_tags = ["manual"] + venv_rule = _py_venv_lib - py_venv( + venv_rule( name = venv_label, testonly = testonly, visibility = venv_visibility, @@ -357,6 +408,7 @@ def py_binary_with_venv(py_rule, name, main, srcs = [], deps = [], data = [], im name = name, main = main, srcs = srcs, + data = data, tags = tags, testonly = testonly, visibility = visibility,