Fix a bug where frozen targets list was mutated while expanding env a…

…ttribute for cc_test. Add a test covering the behavior. PiperOrigin-RevId: 504294442 Change-Id: If9f96b631ca958e746613563c56103044c973277
databricks · Jan 24, 2023 · a63d8eb · a63d8eb
1 parent bb0b886
commit a63d8eb
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 2 deletions.
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
@@ -762,9 +762,10 @@ def _expand_nested_variable(ctx, additional_vars, exp, execpath = True, targets
         if not execpath:
             if exp.startswith("location"):
                 exp = exp.replace("location", "rootpath", 1)
+        data_targets = []
         if ctx.attr.data != None:
-            targets.extend(ctx.attr.data)
-        return ctx.expand_location("$({})".format(exp), targets = targets)
+            data_targets = ctx.attr.data
+        return ctx.expand_location("$({})".format(exp), targets = targets + data_targets)
 
     # Recursively expand nested make variables, but since there is no recursion
     # in Starlark we will do it via for loop.

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -147,6 +147,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
+        "//src/main/java/com/google/devtools/build/lib/analysis:run_environment_info",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.OutputGroupInfo;
+import com.google.devtools.build.lib.analysis.RunEnvironmentInfo;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -809,6 +810,21 @@ public void testExpandedLinkopts() throws Exception {
                     .getPathString()));
   }
 
+  @Test
+  public void testExpandedEnv() throws Exception {
+    scratch.file(
+        "a/BUILD",
+        "genrule(name = 'linker', cmd='generate', outs=['a.lds'])",
+        "cc_test(",
+        "    name='bin_test',",
+        "    srcs=['b.cc'],",
+        "    env={'SOME_KEY': '-Wl,@$(location a.lds)'},",
+        "    deps=['a.lds'])");
+    ConfiguredTarget starlarkTarget = getConfiguredTarget("//a:bin_test");
+    RunEnvironmentInfo provider = starlarkTarget.get(RunEnvironmentInfo.PROVIDER);
+    assertThat(provider.getEnvironment()).containsEntry("SOME_KEY", "-Wl,@a/a.lds");
+  }
+
   @Test
   public void testProvidesLinkerScriptToLinkAction() throws Exception {
     scratch.file(