Skip to content

Commit 363a2cc

Browse files
committed
Expand $(location) and "Make variables" in env
Implements the "values are subject to `$(location)` and "Make variable" substitution" contract for the common `env` attribute for binary and test rules. Fixes bazel-contrib#1770. --- Makes `env` attribute semantics for Scala binary and test targets conformant with those documented in the Bazel docs: - https://bazel.build/reference/be/common-definitions#common-attributes-binaries - https://bazel.build/reference/be/common-definitions#common-attributes-tests - https://bazel.build/reference/be/make-variables#use - https://bazel.build/rules/lib/providers/RunEnvironmentInfo Most of the new functions could live in a separate repository at some point. As for why we're using the "deprecated" `ctx.expand_make_variables`: - bazelbuild/bazel#5859 - bazelbuild/bazel-skylib#486 The "deprecated" comment in the `ctx.expand_make_variables` docstring has existed from the beginning of the file's existence (2018-05-11): - bazelbuild/bazel@abbb900
1 parent 72ee942 commit 363a2cc

File tree

13 files changed

+303
-45
lines changed

13 files changed

+303
-45
lines changed
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
"""Phase implementing variable expansion for the env attribute"""
2+
3+
def _expand_part(ctx, attr_name, part, targets, additional_vars):
4+
"""Perform `$(location)` and "Make variable" substitution for `expand_vars`.
5+
6+
As for why we're using the "deprecated" `ctx.expand_make_variables`:
7+
8+
- https://github.com/bazelbuild/bazel/issues/5859
9+
- https://github.com/bazelbuild/bazel-skylib/pull/486
10+
11+
The "deprecated" comment in the `ctx.expand_make_variables` docstring has
12+
existed from the beginning of the file's existence (2018-05-11):
13+
14+
- https://github.com/bazelbuild/bazel/commit/abbb9002c41bbd53588e7249756aab236f6fcb4b
15+
"""
16+
expanded = ctx.expand_location(part, targets)
17+
return ctx.expand_make_variables(attr_name, expanded, additional_vars)
18+
19+
def expand_vars(ctx, attr_name, value, targets, additional_vars):
20+
"""Perform `$(location)` and "Make variable" substitution on an attribute.
21+
22+
- https://bazel.build/reference/be/make-variables#use
23+
24+
Args:
25+
ctx: Rule context object
26+
attr_name: name of the attribute (for error messages)
27+
value: the attribute value
28+
targets: a list of `Target` values to use with `$(location)` expansion
29+
additional_vars: additional values to use with "Make variable" expansion
30+
31+
Returns:
32+
the result of performing `$(location)` and "Make variable" substitution
33+
on the specified attribute value
34+
"""
35+
# Splitting on `$$` ensures that escaped `$` values prevent `$(location)`
36+
# and "Make variable" substitution for those portions of `value`.
37+
return "$".join([
38+
_expand_part(ctx, attr_name, s, targets, additional_vars)
39+
for s in value.split("$$")
40+
])
41+
42+
def run_environment_info(ctx, additional_attr_names = []):
43+
"""Create a RunEnvironmentInfo provider from `ctx.attr.env` values.
44+
45+
Implements the "values are subject to `$(location)` and "Make variable"
46+
substitution" contract for the common `env` attribute for binary and test
47+
rules:
48+
49+
- https://bazel.build/reference/be/common-definitions#common-attributes-binaries
50+
- https://bazel.build/reference/be/common-definitions#common-attributes-tests
51+
- https://bazel.build/reference/be/make-variables#use
52+
- https://bazel.build/rules/lib/providers/RunEnvironmentInfo
53+
54+
Assigns `ctx.attr.env_inherit` to `RunEnvironmentInfo.inherited_environment`
55+
if present.
56+
57+
Args:
58+
ctx: Rule context object
59+
additional_attr_names: list of attribute names containing targets to use
60+
when invoking `ctx.expand_location`; already includes "data",
61+
"deps", and "srcs"
62+
63+
Returns:
64+
a RunEnvironmentInfo object containing `ctx.attr.env` values after
65+
expanding location and Make variables
66+
"""
67+
targets = []
68+
for attr_name in ["data"] + additional_attr_names:
69+
targets.extend(getattr(ctx.attr, attr_name, []))
70+
71+
return RunEnvironmentInfo(
72+
environment = {
73+
k: expand_vars(ctx, "env", v, targets, ctx.var)
74+
for k, v in ctx.attr.env.items()
75+
},
76+
inherited_environment = getattr(ctx.attr, "env_inherit", []),
77+
)
78+
79+
def phase_runenvironmentinfo_provider(ctx, _):
80+
return struct(
81+
external_providers = {"RunEnvironmentInfo": run_environment_info(ctx)},
82+
)

scala/private/phases/phase_test_environment.bzl

Lines changed: 0 additions & 38 deletions
This file was deleted.

scala/private/phases/phases.bzl

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ load(
6262
_phase_scalainfo_provider_non_macro = "phase_scalainfo_provider_non_macro",
6363
)
6464
load("//scala/private:phases/phase_semanticdb.bzl", _phase_semanticdb = "phase_semanticdb")
65-
load("//scala/private:phases/phase_test_environment.bzl", _phase_test_environment = "phase_test_environment")
65+
load(
66+
"//scala/private:phases/phase_runenvironmentinfo_provider.bzl",
67+
_phase_runenvironmentinfo_provider = "phase_runenvironmentinfo_provider",
68+
)
6669
load(
6770
"//scala/private:phases/phase_write_executable.bzl",
6871
_phase_write_executable_common = "phase_write_executable_common",
@@ -149,8 +152,8 @@ phase_runfiles_common = _phase_runfiles_common
149152
# default_info
150153
phase_default_info = _phase_default_info
151154

152-
# test_environment
153-
phase_test_environment = _phase_test_environment
155+
# runenvironmentinfo_provider
156+
phase_runenvironmentinfo_provider = _phase_runenvironmentinfo_provider
154157

155158
# scalafmt
156159
phase_scalafmt = _phase_scalafmt

scala/private/rules/scala_binary.bzl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ load(
2323
"phase_dependency_common",
2424
"phase_java_wrapper_common",
2525
"phase_merge_jars",
26+
"phase_runenvironmentinfo_provider",
2627
"phase_runfiles_common",
2728
"phase_scalac_provider",
2829
"phase_scalacopts",
@@ -54,12 +55,14 @@ def _scala_binary_impl(ctx):
5455
("runfiles", phase_runfiles_common),
5556
("write_executable", phase_write_executable_common),
5657
("default_info", phase_default_info),
58+
("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
5759
],
5860
)
5961

6062
_scala_binary_attrs = {
6163
"main_class": attr.string(mandatory = True),
6264
"classpath_resources": attr.label_list(allow_files = True),
65+
"env": attr.string_dict(default = {}),
6366
"jvm_flags": attr.string_list(),
6467
"runtime_jdk": attr.label(
6568
default = "@rules_java//toolchains:current_java_runtime",

scala/private/rules/scala_junit_test.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ load(
2424
"phase_java_wrapper_common",
2525
"phase_jvm_flags",
2626
"phase_merge_jars",
27+
"phase_runenvironmentinfo_provider",
2728
"phase_runfiles_common",
2829
"phase_scalac_provider",
2930
"phase_scalacopts",
3031
"phase_scalainfo_provider_non_macro",
3132
"phase_semanticdb",
32-
"phase_test_environment",
3333
"phase_write_executable_junit_test",
3434
"phase_write_manifest",
3535
"run_phases",
@@ -62,7 +62,7 @@ def _scala_junit_test_impl(ctx):
6262
("jvm_flags", phase_jvm_flags),
6363
("write_executable", phase_write_executable_junit_test),
6464
("default_info", phase_default_info),
65-
("test_environment", phase_test_environment),
65+
("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
6666
],
6767
)
6868

scala/private/rules/scala_test.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ load(
2323
"phase_dependency_common",
2424
"phase_java_wrapper_common",
2525
"phase_merge_jars",
26+
"phase_runenvironmentinfo_provider",
2627
"phase_runfiles_scalatest",
2728
"phase_scalac_provider",
2829
"phase_scalacopts",
2930
"phase_scalainfo_provider_non_macro",
3031
"phase_semanticdb",
31-
"phase_test_environment",
3232
"phase_write_executable_scalatest",
3333
"phase_write_manifest",
3434
"run_phases",
@@ -56,7 +56,7 @@ def _scala_test_impl(ctx):
5656
("coverage_runfiles", phase_coverage_runfiles),
5757
("write_executable", phase_write_executable_scalatest),
5858
("default_info", phase_default_info),
59-
("test_environment", phase_test_environment),
59+
("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
6060
],
6161
)
6262

test/BUILD

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ load(
1515
)
1616
load("//scala:scala_cross_version.bzl", "repositories")
1717
load(":check_statsfile.bzl", "check_statsfile")
18+
load(":env_vars.bzl", "env_vars")
1819

1920
package(
2021
default_testonly = 1,
@@ -751,3 +752,57 @@ scala_library(
751752
],
752753
deps = [":InlinableExported"],
753754
)
755+
756+
TEST_ENV = {
757+
"LOCATION": "West of House",
758+
"DEP_PATH": "$(rootpath :HelloLib)",
759+
"DATA_PATH": "$(rootpath //test/data:foo.txt)",
760+
"BINDIR": "$(BINDIR)",
761+
"FROM_TOOLCHAIN_VAR": "$(FOOBAR)",
762+
"ESCAPED": "$$(rootpath //test/data:foo.txt) $$(BINDIR) $$UNKNOWN",
763+
}
764+
765+
scala_binary(
766+
name = "EnvAttributeBinary",
767+
srcs = ["EnvAttributeBinary.scala"],
768+
main_class = "scalarules.test.EnvAttributeBinary",
769+
data = ["//test/data:foo.txt"],
770+
deps = [":HelloLib"],
771+
env = TEST_ENV | {"SRC_PATH": "$(rootpath EnvAttributeBinary.scala)"},
772+
toolchains = [":test_vars"],
773+
unused_dependency_checker_mode = "off",
774+
testonly = True,
775+
)
776+
777+
scala_test(
778+
name = "EnvAttributeTest",
779+
size = "small",
780+
srcs = ["EnvAttributeTest.scala"],
781+
data = ["//test/data:foo.txt"],
782+
deps = [":HelloLib"],
783+
env = TEST_ENV | {"SRC_PATH": "$(rootpath EnvAttributeTest.scala)"},
784+
toolchains = [":test_vars"],
785+
unused_dependency_checker_mode = "off",
786+
)
787+
788+
scala_junit_test(
789+
name = "EnvAttributeJunitTest",
790+
size = "small",
791+
srcs = ["src/main/scala/scalarules/test/junit/EnvAttributeJunitTest.scala"],
792+
suffixes = ["Test"],
793+
data = ["//test/data:foo.txt"],
794+
deps = [":HelloLib"],
795+
env = TEST_ENV | {
796+
"SRC_PATH": (
797+
"$(rootpath " +
798+
"src/main/scala/scalarules/test/junit/EnvAttributeJunitTest.scala)"
799+
),
800+
},
801+
toolchains = [":test_vars"],
802+
unused_dependency_checker_mode = "off",
803+
)
804+
805+
env_vars(
806+
name = "test_vars",
807+
vars = {"FOOBAR": "bazquux"},
808+
)

test/EnvAttributeBinary.scala

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package scalarules.test
2+
3+
object EnvAttributeBinary {
4+
def main(args: Array[String]) {
5+
val envVars = Array(
6+
"LOCATION",
7+
"DATA_PATH",
8+
"DEP_PATH",
9+
"SRC_PATH",
10+
"BINDIR",
11+
"FROM_TOOLCHAIN_VAR",
12+
"ESCAPED",
13+
)
14+
val env = System.getenv()
15+
16+
for (envVar <- envVars) {
17+
println(envVar + ": " + env.get(envVar))
18+
}
19+
}
20+
}

test/EnvAttributeTest.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package scalarules.test
2+
3+
import org.scalatest.flatspec._
4+
5+
class EnvAttributeTest extends AnyFlatSpec {
6+
val env = System.getenv()
7+
8+
"the env attribute" should "contain a plain value" in {
9+
assert(env.get("LOCATION") == "West of House")
10+
}
11+
12+
"the env attribute" should "expand location variables" in {
13+
assert(env.get("DATA_PATH") == "test/data/foo.txt", "in DATA_PATH")
14+
assert(env.get("DEP_PATH") == "test/HelloLib.jar", "in DEP_PATH")
15+
assert(env.get("SRC_PATH") == "test/EnvAttributeTest.scala", "in SRC_PATH")
16+
}
17+
18+
"the env attribute" should "expand Make variables" in {
19+
assert(env.get("BINDIR").startsWith("bazel-out/"))
20+
}
21+
22+
"the env attribute" should "expand toolchain supplied variables" in {
23+
assert(env.get("FROM_TOOLCHAIN_VAR") == "bazquux")
24+
}
25+
26+
"the env attribute" should "not expand escaped variables" in {
27+
assert(
28+
env.get("ESCAPED") ==
29+
"$(rootpath //test/data:foo.txt) $(BINDIR) $UNKNOWN"
30+
)
31+
}
32+
}

test/env_vars.bzl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
"""Implements a custom environment variable map"""
2+
3+
env_vars = rule(
4+
implementation = lambda ctx: [
5+
platform_common.TemplateVariableInfo(ctx.attr.vars),
6+
],
7+
attrs = {"vars": attr.string_dict(mandatory = True)},
8+
)

0 commit comments

Comments
 (0)