Skip to content

refactor: split py_venv binary vs lib rules #980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jbedard wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: split py_venv binary vs lib rules #980

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 88 additions & 36 deletions py/private/py_venv/py_venv.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.""",
Expand All @@ -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",
),
Expand All @@ -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.""",
Expand All @@ -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({
Expand All @@ -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 <flags> 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):
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down