commit 46a8e09334a03eeb8518a4913dcd98f19ddc9c62
author Joel Williamson <joel.s.williamson@gmail.com> Thu Feb 24 05:16:56 2022 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Feb 24 05:18:12 2022 -0800
tree f7d183fe8a1229cc22103631ea41f9837c3854e3
parent 912e44611bd6c48822471dd0cfacda6024d7666f

Use data dependencies in Make variable substitution

This is a fix for #13930

Closes #13938.

PiperOrigin-RevId: 430677775

src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java

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 fa1a04d..00194b0 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
@@ -580,19 +580,29 @@
     List<String> defines = new ArrayList<>();
 
     // collect labels that can be subsituted in defines
-    ImmutableMap.Builder<Label, ImmutableCollection<Artifact>> builder = ImmutableMap.builder();
+    Map<Label, ImmutableCollection<Artifact>> map = Maps.newLinkedHashMap();
 
     if (ruleContext.attributes().has("deps", LABEL_LIST)) {
       for (TransitiveInfoCollection current : ruleContext.getPrerequisites("deps")) {
-        builder.put(
+        map.put(
             AliasProvider.getDependencyLabel(current),
             current.getProvider(FileProvider.class).getFilesToBuild().toList());
       }
     }
 
+    if (ruleContext.attributes().has("data", LABEL_LIST)) {
+      for (TransitiveInfoCollection current : ruleContext.getPrerequisites("data")) {
+        Label dataDependencyLabel = AliasProvider.getDependencyLabel(current);
+        if (!map.containsKey(dataDependencyLabel)) {
+          map.put(
+              dataDependencyLabel,
+              current.getProvider(FileProvider.class).getFilesToBuild().toList());
+        }
+      }
+    }
     // tokenize defines and substitute make variables
     for (String define :
-        ruleContext.getExpander().withExecLocations(builder.buildOrThrow()).list(attr)) {
+        ruleContext.getExpander().withExecLocations(ImmutableMap.copyOf(map)).list(attr)) {
       List<String> tokens = new ArrayList<>();
       try {
         ShellUtils.tokenize(tokens, define);

src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java

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 bcd5028..3afcc8b 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
@@ -270,6 +270,21 @@
   }
 
   @Test
+  public void testExpandedDefinesAgainstData() throws Exception {
+    scratch.file("data/BUILD", "filegroup(name = 'data', srcs = ['data.txt'])");
+    ConfiguredTarget expandedDefines =
+        scratchConfiguredTarget(
+            "expanded_defines",
+            "expand_srcs",
+            "cc_library(name = 'expand_srcs',",
+            "           srcs = ['defines.cc'],",
+            "           data = ['//data'],",
+            "           defines = ['FOO=$(location //data)'])");
+    assertThat(expandedDefines.get(CcInfo.PROVIDER).getCcCompilationContext().getDefines())
+        .containsExactly("FOO=data/data.txt");
+  }
+
+  @Test
   public void testStartEndLib() throws Exception {
     getAnalysisMock()
         .ccSupport()