From 28fc8a1f7f7524d1b730da2a41341ce4aa1fea35 Mon Sep 17 00:00:00 2001 From: erenon Date: Tue, 2 Feb 2021 02:43:44 -0800 Subject: [PATCH] Expand make variables in defines of cc_* rules To match the documented behavior: "List of defines to add to the compile line. Subject to "Make" variable substitution and Bourne shell tokenization." https://docs.bazel.build/versions/master/be/c-cpp.html#cc_binary.defines Fixes #12482 Closes #12483. PiperOrigin-RevId: 355126491 --- .../build/lib/rules/cpp/CcCommon.java | 18 +++++++++++- .../build/lib/rules/cpp/CcCommonTest.java | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index f7b017d5a1a7db..3203038ef66f59 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -14,16 +14,19 @@ package com.google.devtools.build.lib.rules.cpp; import static com.google.devtools.build.lib.packages.BuildType.LABEL; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.MakeVariableSupplier; @@ -525,7 +528,20 @@ public List getNonTransitiveDefines() { private List getDefinesFromAttribute(String attr) { List defines = new ArrayList<>(); - for (String define : ruleContext.getExpander().list(attr)) { + + // collect labels that can be subsituted in defines + ImmutableMap.Builder> builder = ImmutableMap.builder(); + + if (ruleContext.attributes().has("deps", LABEL_LIST)) { + for (TransitiveInfoCollection current : ruleContext.getPrerequisites("deps")) { + builder.put( + AliasProvider.getDependencyLabel(current), + current.getProvider(FileProvider.class).getFilesToBuild().toList()); + } + } + + // tokenize defines and substitute make variables + for (String define : ruleContext.getExpander().withExecLocations(builder.build()).list(attr)) { List tokens = new ArrayList<>(); try { ShellUtils.tokenize(tokens, define); 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 36397e120bf080..e0205b2b07946b 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 @@ -247,6 +247,34 @@ public void testIsolatedDefines() throws Exception { .inOrder(); } + @Test + public void testExpandedDefinesAgainstDeps() throws Exception { + ConfiguredTarget expandedDefines = + scratchConfiguredTarget( + "expanded_defines", + "expand_deps", + "cc_library(name = 'expand_deps',", + " srcs = ['defines.cc'],", + " deps = ['//foo'],", + " defines = ['FOO=$(location //foo)'])"); + assertThat(expandedDefines.get(CcInfo.PROVIDER).getCcCompilationContext().getDefines()) + .containsExactly( + String.format("FOO=%s/foo/libfoo.a", getRuleContext(expandedDefines).getBinFragment())); + } + + @Test + public void testExpandedDefinesAgainstSrcs() throws Exception { + ConfiguredTarget expandedDefines = + scratchConfiguredTarget( + "expanded_defines", + "expand_srcs", + "cc_library(name = 'expand_srcs',", + " srcs = ['defines.cc'],", + " defines = ['FOO=$(location defines.cc)'])"); + assertThat(expandedDefines.get(CcInfo.PROVIDER).getCcCompilationContext().getDefines()) + .containsExactly("FOO=expanded_defines/defines.cc"); + } + @Test public void testStartEndLib() throws Exception { getAnalysisMock()