From a63d8ebb84caba9d1741a9d9fec725a7db52e25e Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 24 Jan 2023 09:19:07 -0800 Subject: [PATCH] Fix a bug where frozen targets list was mutated while expanding env attribute for cc_test. Add a test covering the behavior. PiperOrigin-RevId: 504294442 Change-Id: If9f96b631ca958e746613563c56103044c973277 --- .../builtins_bzl/common/cc/cc_helper.bzl | 5 +++-- .../google/devtools/build/lib/rules/cpp/BUILD | 1 + .../build/lib/rules/cpp/CcCommonTest.java | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 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 index 98f27a234d553f..359f3aedad0603 100644 --- 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 index 22d443fd9b147e..674e96836aef46 100644 --- 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 index 137e26ee3f19ae..5df4ceb1c6b67b 100644 --- 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(