Skip to content

Commit 5109e4a

Browse files
authored
feat(py_venv_*): Implement env/env_inherit semantics (#616)
These attributes were implemented for `py_binary` and `py_test`,[^1] but not their venv-based equivalents, where they were present, but not wired-up to anything. The semantics of these properties are established for native rules through the Build Encyclopedia's "Common Definitions";[^2][^3] Starlark rules aren't required to follow this, but it is usually useful and least surprising for them to do so. [^1]: https://github.com/aspect-build/rules_py/blob/c29d3d8911136dab2d01469a99a53b7e6cfa2b20/py/private/py_binary.bzl#L131 [^2]: https://bazel.build/reference/be/common-definitions#test.env [^3]: https://bazel.build/reference/be/common-definitions#test.env_inherit --- ### 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(\*) (\*) Technically, this changes the semantics of user-controlled attributes of public symbols. However, these attributes were non-functional previously; users were getting the same behaviour as not setting these properties. This change makes these properties useful in satisfying the clear intent the user communicates when setting these attributes. - Suggested release notes appear below: yes feat(venv): implement env/env_inherit semantics for py_venv_binary/py_venv_test ### Test plan - New test cases added
1 parent 152fb3c commit 5109e4a

File tree

3 files changed

+42
-9
lines changed

3 files changed

+42
-9
lines changed

py/private/py_venv/py_venv.bzl

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,6 @@ def _py_venv_base_impl(ctx):
8080
"BAZEL_TARGET_NAME": ctx.attr.name,
8181
}
8282

83-
passed_env = dict(ctx.attr.env)
84-
for k, v in passed_env.items():
85-
passed_env[k] = expand_variables(
86-
ctx,
87-
expand_locations(ctx, v, ctx.attr.data),
88-
attribute_name = "env",
89-
)
90-
9183
ctx.actions.write(
9284
output = env_file,
9385
content = "\n".join(_dict_to_exports(default_env)).strip(),
@@ -247,6 +239,14 @@ def _py_venv_binary_impl(ctx):
247239
is_executable = True,
248240
)
249241

242+
passed_env = dict(ctx.attr.env)
243+
for k, v in passed_env.items():
244+
passed_env[k] = expand_variables(
245+
ctx,
246+
expand_locations(ctx, v, ctx.attr.data),
247+
attribute_name = "env",
248+
)
249+
250250
return [
251251
DefaultInfo(
252252
files = depset([
@@ -255,6 +255,10 @@ def _py_venv_binary_impl(ctx):
255255
executable = ctx.outputs.executable,
256256
runfiles = rfs,
257257
),
258+
RunEnvironmentInfo(
259+
environment = passed_env,
260+
inherited_environment = getattr(ctx.attr, "env_inherit", []),
261+
),
258262
]
259263

260264
_attrs = dict({
@@ -368,7 +372,6 @@ _binary_attrs = dict({
368372
})
369373

370374
_test_attrs = dict({
371-
# FIXME: Where does this come from, do we need to keep it?
372375
"env_inherit": attr.string_list(
373376
doc = "Specifies additional environment variables to inherit from the external environment when the test is executed by bazel test.",
374377
default = [],

py/tests/py-internal-venv/BUILD.bazel

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,16 @@ py_venv_test(
1313
"@pypi_cowsay//:pkg",
1414
],
1515
)
16+
17+
py_venv_test(
18+
name = "test_env_vars",
19+
srcs = ["test_env_vars.py"],
20+
env = {
21+
"ONE": "un",
22+
"TWO": "deux",
23+
"RULEDIR": "$(RULEDIR)",
24+
"LOCATION": "$(location :test_env_vars.py)",
25+
"DEFINE": "$(SOME_VAR)",
26+
},
27+
main = "test_env_vars.py",
28+
)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import os
2+
3+
4+
def test_env(env, expected):
5+
assert env in os.environ, f"Expected environ to have key '{env}'"
6+
7+
_actual = os.environ.get(env)
8+
assert _actual == expected, f"Expected environ key '{env}' to equal '{expected}', but got '{_actual}'"
9+
10+
11+
test_env('ONE', 'un')
12+
test_env('TWO', 'deux')
13+
test_env('LOCATION', "py/tests/py-internal-venv/test_env_vars.py")
14+
test_env('DEFINE', "SOME_VALUE")
15+
test_env('BAZEL_TARGET', "//py/tests/py-internal-venv:test_env_vars")
16+
test_env('BAZEL_WORKSPACE', "aspect_rules_py")
17+
test_env('BAZEL_TARGET_NAME', "test_env_vars")

0 commit comments

Comments
 (0)