Expand $(location) and "Make variables" in env

scala/private/phases/phase_runenvironmentinfo_provider.bzl

@@ -0,0 +1,82 @@
+"""Phase implementing variable expansion for the env attribute"""
+
+def _expand_part(ctx, attr_name, part, targets, additional_vars):
+    """Perform `$(location)` and "Make variable" substitution for `expand_vars`.
+
+    As for why we're using the "deprecated" `ctx.expand_make_variables`:
+
+    - https://github.com/bazelbuild/bazel/issues/5859
+    - https://github.com/bazelbuild/bazel-skylib/pull/486
+
+    The "deprecated" comment in the `ctx.expand_make_variables` docstring has
+    existed from the beginning of the file's existence (2018-05-11):
+
+    - https://github.com/bazelbuild/bazel/commit/abbb9002c41bbd53588e7249756aab236f6fcb4b
+    """
+    expanded = ctx.expand_location(part, targets)
+    return ctx.expand_make_variables(attr_name, expanded, additional_vars)
+
+def expand_vars(ctx, attr_name, value, targets, additional_vars):
+    """Perform `$(location)` and "Make variable" substitution on an attribute.
+
+    - https://bazel.build/reference/be/make-variables#use
+
+    Args:
+        ctx: Rule context object
+        attr_name: name of the attribute (for error messages)
+        value: the attribute value
+        targets: a list of `Target` values to use with `$(location)` expansion
+        additional_vars: additional values to use with "Make variable" expansion
+
+    Returns:
+        the result of performing `$(location)` and "Make variable" substitution
+            on the specified attribute value
+    """
+    # Splitting on `$$` ensures that escaped `$` values prevent `$(location)`
+    # and "Make variable" substitution for those portions of `value`.
+    return "$".join([
+        _expand_part(ctx, attr_name, s, targets, additional_vars)
+        for s in value.split("$$")
+    ])
+
+def run_environment_info(ctx, additional_attr_names = []):
+    """Create a RunEnvironmentInfo provider from `ctx.attr.env` values.
+
+    Implements the "values are subject to `$(location)` and "Make variable"
+    substitution" contract for the common `env` attribute for binary and test
+    rules:
+
+    - 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
+
+    Assigns `ctx.attr.env_inherit` to `RunEnvironmentInfo.inherited_environment`
+    if present.
+
+    Args:
+        ctx: Rule context object
+        additional_attr_names: list of attribute names containing targets to use
+            when invoking `ctx.expand_location`; already includes "data",
+            "deps", and "srcs"
+
+    Returns:
+        a RunEnvironmentInfo object containing `ctx.attr.env` values after
+            expanding location and Make variables
+    """
+    targets = []
+    for attr_name in ["data"] + additional_attr_names:
+        targets.extend(getattr(ctx.attr, attr_name, []))
+
+    return RunEnvironmentInfo(
+        environment = {
+            k: expand_vars(ctx, "env", v, targets, ctx.var)
+            for k, v in ctx.attr.env.items()
+        },
+        inherited_environment = getattr(ctx.attr, "env_inherit", []),
+    )
+
+def phase_runenvironmentinfo_provider(ctx, _):
+    return struct(
+        external_providers = {"RunEnvironmentInfo": run_environment_info(ctx)},
+    )

scala/private/phases/phases.bzl

@@ -62,7 +62,10 @@ load(
     _phase_scalainfo_provider_non_macro = "phase_scalainfo_provider_non_macro",
 )
 load("//scala/private:phases/phase_semanticdb.bzl", _phase_semanticdb = "phase_semanticdb")
-load("//scala/private:phases/phase_test_environment.bzl", _phase_test_environment = "phase_test_environment")
+load(
+    "//scala/private:phases/phase_runenvironmentinfo_provider.bzl",
+    _phase_runenvironmentinfo_provider = "phase_runenvironmentinfo_provider",
+)
 load(
     "//scala/private:phases/phase_write_executable.bzl",
     _phase_write_executable_common = "phase_write_executable_common",
@@ -149,8 +152,8 @@ phase_runfiles_common = _phase_runfiles_common
 # default_info
 phase_default_info = _phase_default_info
 
-# test_environment
-phase_test_environment = _phase_test_environment
+# runenvironmentinfo_provider
+phase_runenvironmentinfo_provider = _phase_runenvironmentinfo_provider
 
 # scalafmt
 phase_scalafmt = _phase_scalafmt

scala/private/rules/scala_binary.bzl

@@ -23,6 +23,7 @@ load(
     "phase_dependency_common",
     "phase_java_wrapper_common",
     "phase_merge_jars",
+    "phase_runenvironmentinfo_provider",
     "phase_runfiles_common",
     "phase_scalac_provider",
     "phase_scalacopts",
@@ -54,12 +55,14 @@ def _scala_binary_impl(ctx):
             ("runfiles", phase_runfiles_common),
             ("write_executable", phase_write_executable_common),
             ("default_info", phase_default_info),
+            ("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
         ],
     )
 
 _scala_binary_attrs = {
     "main_class": attr.string(mandatory = True),
     "classpath_resources": attr.label_list(allow_files = True),
+    "env": attr.string_dict(default = {}),
     "jvm_flags": attr.string_list(),
     "runtime_jdk": attr.label(
         default = "@rules_java//toolchains:current_java_runtime",

scala/private/rules/scala_junit_test.bzl

@@ -24,12 +24,12 @@ load(
     "phase_java_wrapper_common",
     "phase_jvm_flags",
     "phase_merge_jars",
+    "phase_runenvironmentinfo_provider",
     "phase_runfiles_common",
     "phase_scalac_provider",
     "phase_scalacopts",
     "phase_scalainfo_provider_non_macro",
     "phase_semanticdb",
-    "phase_test_environment",
     "phase_write_executable_junit_test",
     "phase_write_manifest",
     "run_phases",
@@ -62,7 +62,7 @@ def _scala_junit_test_impl(ctx):
             ("jvm_flags", phase_jvm_flags),
             ("write_executable", phase_write_executable_junit_test),
             ("default_info", phase_default_info),
-            ("test_environment", phase_test_environment),
+            ("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
         ],
     )
 

scala/private/rules/scala_test.bzl

@@ -23,12 +23,12 @@ load(
     "phase_dependency_common",
     "phase_java_wrapper_common",
     "phase_merge_jars",
+    "phase_runenvironmentinfo_provider",
     "phase_runfiles_scalatest",
     "phase_scalac_provider",
     "phase_scalacopts",
     "phase_scalainfo_provider_non_macro",
     "phase_semanticdb",
-    "phase_test_environment",
     "phase_write_executable_scalatest",
     "phase_write_manifest",
     "run_phases",
@@ -56,7 +56,7 @@ def _scala_test_impl(ctx):
             ("coverage_runfiles", phase_coverage_runfiles),
             ("write_executable", phase_write_executable_scalatest),
             ("default_info", phase_default_info),
-            ("test_environment", phase_test_environment),
+            ("runenvironmentinfo_provider", phase_runenvironmentinfo_provider),
         ],
     )
 

test/BUILD

@@ -15,6 +15,7 @@ load(
 )
 load("//scala:scala_cross_version.bzl", "repositories")
 load(":check_statsfile.bzl", "check_statsfile")
+load(":env_vars.bzl", "env_vars")
 
 package(
     default_testonly = 1,
@@ -751,3 +752,57 @@ scala_library(
     ],
     deps = [":InlinableExported"],
 )
+
+TEST_ENV = {
+    "LOCATION": "West of House",
+    "DEP_PATH": "$(rootpath :HelloLib)",
+    "DATA_PATH": "$(rootpath //test/data:foo.txt)",
+    "BINDIR": "$(BINDIR)",
+    "FROM_TOOLCHAIN_VAR": "$(FOOBAR)",
+    "ESCAPED": "$$(rootpath //test/data:foo.txt) $$(BINDIR) $$UNKNOWN",
+}
+
+scala_binary(
+    name = "EnvAttributeBinary",
+    srcs = ["EnvAttributeBinary.scala"],
+    main_class = "scalarules.test.EnvAttributeBinary",
+    data = ["//test/data:foo.txt"],
+    deps = [":HelloLib"],
+    env = TEST_ENV | {"SRC_PATH": "$(rootpath EnvAttributeBinary.scala)"},
+    toolchains = [":test_vars"],
+    unused_dependency_checker_mode = "off",
+    testonly = True,
+)
+
+scala_test(
+    name = "EnvAttributeTest",
+    size = "small",
+    srcs = ["EnvAttributeTest.scala"],
+    data = ["//test/data:foo.txt"],
+    deps = [":HelloLib"],
+    env = TEST_ENV | {"SRC_PATH": "$(rootpath EnvAttributeTest.scala)"},
+    toolchains = [":test_vars"],
+    unused_dependency_checker_mode = "off",
+)
+
+scala_junit_test(
+    name = "EnvAttributeJunitTest",
+    size = "small",
+    srcs = ["src/main/scala/scalarules/test/junit/EnvAttributeJunitTest.scala"],
+    suffixes = ["Test"],
+    data = ["//test/data:foo.txt"],
+    deps = [":HelloLib"],
+    env = TEST_ENV | {
+        "SRC_PATH": (
+            "$(rootpath " +
+            "src/main/scala/scalarules/test/junit/EnvAttributeJunitTest.scala)"
+        ),
+    },
+    toolchains = [":test_vars"],
+    unused_dependency_checker_mode = "off",
+)
+
+env_vars(
+    name = "test_vars",
+    vars = {"FOOBAR": "bazquux"},
+)

test/EnvAttributeBinary.scala

@@ -0,0 +1,20 @@
+package scalarules.test
+
+object EnvAttributeBinary {
+  def main(args: Array[String]) {
+    val envVars = Array(
+      "LOCATION",
+      "DATA_PATH",
+      "DEP_PATH",
+      "SRC_PATH",
+      "BINDIR",
+      "FROM_TOOLCHAIN_VAR",
+      "ESCAPED",
+    )
+    val env = System.getenv()
+
+    for (envVar <- envVars) {
+      println(envVar + ": " + env.get(envVar))
+    }
+  }
+}

test/EnvAttributeTest.scala

@@ -0,0 +1,32 @@
+package scalarules.test
+
+import org.scalatest.flatspec._
+
+class EnvAttributeTest extends AnyFlatSpec {
+  val env = System.getenv()
+
+  "the env attribute" should "contain a plain value" in {
+    assert(env.get("LOCATION") == "West of House")
+  }
+
+  "the env attribute" should "expand location variables" in {
+    assert(env.get("DATA_PATH") == "test/data/foo.txt", "in DATA_PATH")
+    assert(env.get("DEP_PATH") == "test/HelloLib.jar", "in DEP_PATH")
+    assert(env.get("SRC_PATH") == "test/EnvAttributeTest.scala", "in SRC_PATH")
+  }
+
+  "the env attribute" should "expand Make variables" in {
+    assert(env.get("BINDIR").startsWith("bazel-out/"))
+  }
+
+  "the env attribute" should "expand toolchain supplied variables" in {
+    assert(env.get("FROM_TOOLCHAIN_VAR") == "bazquux")
+  }
+
+  "the env attribute" should "not expand escaped variables" in {
+    assert(
+      env.get("ESCAPED") ==
+      "$(rootpath //test/data:foo.txt) $(BINDIR) $UNKNOWN"
+    )
+  }
+}

test/env_vars.bzl

@@ -0,0 +1,8 @@
+"""Implements a custom environment variable map"""
+
+env_vars = rule(
+    implementation = lambda ctx: [
+        platform_common.TemplateVariableInfo(ctx.attr.vars),
+    ],
+    attrs = {"vars": attr.string_dict(mandatory = True)},
+)

test/shell/test_env_attribute_expansion.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+#
+# Tests that `scala_binary` properly expands `env` attribute values.
+# See: scala/private/phases/phase_expand_environment.bzl
+
+set -euo pipefail
+
+dir="$( cd "${BASH_SOURCE[0]%/*}" && echo "${PWD%/test/shell}" )"
+test_source="${dir}/test/shell/${BASH_SOURCE[0]#*test/shell/}"
+# shellcheck source=./test_runner.sh
+. "${dir}"/test/shell/test_runner.sh
+
+setup_suite() {
+  original_dir="$PWD"
+  setup_test_tmpdir_for_file "$original_dir" "$test_source"
+  test_tmpdir="$PWD"
+  cd "$original_dir"
+}
+
+teardown_suite() {
+  rm -rf "$test_tmpdir"
+}
+
+test_scala_binary_env_attribute_expansion() {
+  local bindir="$(bazel info bazel-bin)"
+  bindir="bazel-out/${bindir#*/bazel-out/}"
+
+  bazel run //test:EnvAttributeBinary > "${test_tmpdir}/actual.txt"
+
+  printf '%s\n' \
+    'LOCATION: West of House' \
+    'DATA_PATH: test/data/foo.txt' \
+    'DEP_PATH: test/HelloLib.jar' \
+    'SRC_PATH: test/EnvAttributeBinary.scala' \
+    "BINDIR: ${bindir}" \
+    'FROM_TOOLCHAIN_VAR: bazquux' \
+    'ESCAPED: $(rootpath //test/data:foo.txt) $(BINDIR) $UNKNOWN' \
+      > "${test_tmpdir}/expected.txt"
+
+  diff -u --strip-trailing-cr "${test_tmpdir}"/{expected,actual}.txt
+}
+
+setup_suite
+run_tests "$test_source" "$(get_test_runner "${1:-local}")"
+teardown_suite