Pass all TreeArtifacts to the ParameterWriteFileAction so they're expanded by the time the action runs.

Tested:
custom_blaze build experimental/users/kmensah/cc_proto_library:libnon_android_example.so --config android_x86 -s

RELNOTES:
Properly handle tree artifacts on the link command line coming from a cc_library dependency.
PiperOrigin-RevId: 189344192

2 files changed, 91 insertions, 12 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
index a857be4d72..01a50fc72b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
@@ -21,7 +21,10 @@ import com.google.common.base.Joiner;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ActionConfig;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException;
@@ -35,10 +38,13 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.Str
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValue;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValueBuilder;
 import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
+import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.testutil.TestUtils;
+import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
 import com.google.protobuf.TextFormat;
+import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -47,11 +53,9 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/**
- * Tests for toolchain features.
- */
+/** Tests for toolchain features. */
 @RunWith(JUnit4.class)
-public class CcToolchainFeaturesTest {
+public class CcToolchainFeaturesTest extends FoundationTestCase {
 
   /**
    * Creates a {@code Variables} configuration from a list of key/value pairs.
@@ -102,6 +106,17 @@ public class CcToolchainFeaturesTest {
     return enabledFeatures.build();
   }
 
+  private Artifact scratchArtifact(String s) {
+    Path execRoot = outputBase.getRelative("exec");
+    Path outputRoot = execRoot.getRelative("out");
+    ArtifactRoot root = ArtifactRoot.asDerivedRoot(execRoot, outputRoot);
+    try {
+      return new Artifact(scratch.overwriteFile(outputRoot.getRelative(s).toString()), root);
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   @Test
   public void testCodec() throws Exception {
     FeatureConfiguration emptyConfiguration =
@@ -1671,19 +1686,24 @@ public class CcToolchainFeaturesTest {
                 .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.OBJECT_FILES_FIELD_NAME))
         .isNull();
 
+    ImmutableList<Artifact> testArtifacts =
+        ImmutableList.of(scratchArtifact("foo"), scratchArtifact("bar"));
+
     assertThat(
-            LibraryToLinkValue.forObjectFileGroup(ImmutableList.of("foo", "bar"), false)
+            LibraryToLinkValue.forObjectFileGroup(testArtifacts, false)
                 .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.NAME_FIELD_NAME))
         .isNull();
     Iterable<? extends VariableValue> objects =
-        LibraryToLinkValue.forObjectFileGroup(ImmutableList.of("foo", "bar"), false)
+        LibraryToLinkValue.forObjectFileGroup(testArtifacts, false)
             .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.OBJECT_FILES_FIELD_NAME)
             .getSequenceValue(LibraryToLinkValue.OBJECT_FILES_FIELD_NAME);
     ImmutableList.Builder<String> objectNames = ImmutableList.builder();
     for (VariableValue object : objects) {
       objectNames.add(object.getStringValue("name"));
     }
-    assertThat(objectNames.build()).containsExactly("foo", "bar");
+    assertThat(objectNames.build())
+        .containsExactlyElementsIn(
+            Iterables.transform(testArtifacts, testArtifact -> testArtifact.getExecPathString()));
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index 89131cc559..beb4553179 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -19,6 +19,7 @@ import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.primitives.Ints;
@@ -704,9 +705,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
     CppLinkActionBuilder builder =
         createLinkBuilder(LinkTargetType.STATIC_LIBRARY)
             .setLibraryIdentifier("foo")
-            .addObjectFiles(ImmutableList.of(testTreeArtifact))
-            // Makes sure this doesn't use a params file.
-            .setFake(true);
+            .addObjectFiles(ImmutableList.of(testTreeArtifact));
 
     CppLinkAction linkAction = builder.build();
 
@@ -715,12 +714,72 @@ public class CppLinkActionTest extends BuildViewTestCase {
         ImmutableList.of(library0.getExecPathString(), library1.getExecPathString());
 
     // Should only reference the tree artifact.
-    verifyArguments(linkAction.getCommandLine(null), treeArtifactsPaths, treeFileArtifactsPaths);
+    verifyArguments(
+        linkAction.getLinkCommandLine().getRawLinkArgv(),
+        treeArtifactsPaths,
+        treeFileArtifactsPaths);
+
+    // Should only reference tree file artifacts.
+    verifyArguments(
+        linkAction.getLinkCommandLine().getRawLinkArgv(expander),
+        treeFileArtifactsPaths,
+        treeArtifactsPaths);
+  }
+
+  @Test
+  public void testLinksTreeArtifactLibraryForDeps() throws Exception {
+    // This test only makes sense if start/end lib archives are supported.
+    analysisMock.ccSupport().setupCrosstool(mockToolsConfig, "supports_start_end_lib: true");
+    useConfiguration("--start_end_lib");
+
+    SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
+
+    TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o");
+    TreeFileArtifact library1 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library1.o");
+
+    ArtifactExpander expander =
+        new ArtifactExpander() {
+          @Override
+          public void expand(Artifact artifact, Collection<? super Artifact> output) {
+            if (artifact.equals(testTreeArtifact)) {
+              output.add(library0);
+              output.add(library1);
+            }
+          };
+        };
+
+    Artifact archiveFile = scratchArtifact("library.a");
+
+    CppLinkActionBuilder builder =
+        createLinkBuilder(LinkTargetType.STATIC_LIBRARY)
+            .setLibraryIdentifier("foo")
+            .addLibrary(
+                LinkerInputs.newInputLibrary(
+                    archiveFile,
+                    ArtifactCategory.STATIC_LIBRARY,
+                    null,
+                    ImmutableList.<Artifact>of(testTreeArtifact),
+                    ImmutableMap.<Artifact, Artifact>of(),
+                    null));
+
+    CppLinkAction linkAction = builder.build();
+
+    Iterable<String> treeArtifactsPaths = ImmutableList.of(testTreeArtifact.getExecPathString());
+    Iterable<String> treeFileArtifactsPaths =
+        ImmutableList.of(library0.getExecPathString(), library1.getExecPathString());
+
+    // Should only reference the tree artifact.
+    verifyArguments(
+        linkAction.getLinkCommandLine().getRawLinkArgv(),
+        treeArtifactsPaths,
+        treeFileArtifactsPaths);
     verifyArguments(linkAction.getArguments(), treeArtifactsPaths, treeFileArtifactsPaths);
 
     // Should only reference tree file artifacts.
     verifyArguments(
-        linkAction.getCommandLine(expander), treeFileArtifactsPaths, treeArtifactsPaths);
+        linkAction.getLinkCommandLine().getRawLinkArgv(expander),
+        treeFileArtifactsPaths,
+        treeArtifactsPaths);
   }
 
   @Test