Move interface so building to action configs

This cl moves the conditional building of interface libraries from LinkCommandLine to action configs and features. It provides link_dynamic_library.sh to keep blaze backwards compatible. The script and related code can be deleted once all crosstools are updated.

RELNOTES: No.

--
MOS_MIGRATED_REVID=135799041

6 files changed, 174 insertions, 37 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
index 4ecc1c0a5f..521d56a927 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
@@ -32,7 +32,8 @@ public final class BazelMockCcSupport extends MockCcSupport {
       new Predicate<String>() {
         @Override
         public boolean apply(String label) {
-          return !label.startsWith("@blaze_tools//tools/cpp/stl");
+          return !label.startsWith("@blaze_tools//tools/cpp/stl")
+              && !label.startsWith("@blaze_tools//tools/cpp/link_dynamic_library");
         }
       };
 
@@ -75,6 +76,7 @@ public final class BazelMockCcSupport extends MockCcSupport {
   public void setup(MockToolsConfig config) throws IOException {
     config.create(
         "/bazel_tools_workspace/tools/cpp/BUILD",
+        "package(default_visibility=['//visibility:public'])",
         "cc_library(name = 'stl')",
         "cc_library(name = 'malloc')",
         "cc_toolchain_suite(",
@@ -119,11 +121,20 @@ public final class BazelMockCcSupport extends MockCcSupport {
         "    linker_files = ':empty',",
         "    module_map = 'crosstool.cppmap', supports_header_parsing = 1,",
         "    objcopy_files = ':empty', static_runtime_libs = [':empty'], strip_files = ':empty',",
+        ")",
+        "filegroup(",
+        "    name = 'link_dynamic_library',",
+        "    srcs = ['link_dynamic_library.sh'],",
         ")");
 
     config.create(
         "/bazel_tools_workspace/tools/cpp/CROSSTOOL",
         readCrosstoolFile());
+    if (config.isRealFileSystem()) {
+      config.linkTool("tools/cpp/link_dynamic_library.sh");
+    } else {
+      config.create("tools/cpp/link_dynamic_library.sh", "");
+    }
     config.create(
         "/bazel_tools_workspace/tools/objc/BUILD",
         "xcode_config(name = 'host_xcodes')");
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
index 0a7c700005..5901aced64 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -35,10 +35,7 @@ import java.io.IOException;
  */
 public abstract class MockCcSupport {
 
-  /**
-   * Filter to remove implicit crosstool artifact and module map inputs
-   * of C/C++ rules.
-   */
+  /** Filter to remove implicit crosstool artifact and module map inputs of C/C++ rules. */
   public static final Predicate<Artifact> CC_ARTIFACT_FILTER =
       new Predicate<Artifact>() {
         @Override
@@ -46,6 +43,7 @@ public abstract class MockCcSupport {
           String basename = artifact.getExecPath().getBaseName();
           String pathString = artifact.getExecPathString();
           return !pathString.startsWith("third_party/crosstool/")
+              && !pathString.startsWith("tools/cpp/link_dynamic_library")
               && !(pathString.contains("/internal/_middlemen") && basename.contains("crosstool"))
               && !pathString.startsWith("_bin/build_interface_so")
               && !pathString.endsWith(".cppmap");
@@ -502,7 +500,16 @@ public abstract class MockCcSupport {
         "package(default_visibility = ['//visibility:public'])",
         "cc_library(name = 'stl')",
         "alias(name='toolchain', actual='//third_party/crosstool')",
-        "cc_library(name = 'malloc')");
+        "cc_library(name = 'malloc')",
+        "filegroup(",
+        "    name = 'link_dynamic_library',",
+        "    srcs = ['link_dynamic_library.sh'],",
+        ")");
+    if (config.isRealFileSystem()) {
+      config.linkTool("tools/cpp/link_dynamic_library.sh");
+    } else {
+      config.create("tools/cpp/link_dynamic_library.sh", "");
+    }
   }
 
   protected void createCrosstoolPackage(MockToolsConfig config, boolean addEmbeddedRuntimes)
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
index 2099df68a1..c8de88c0a4 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -262,8 +262,6 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
         LinkerInputs.toLibraryArtifacts(action.getLinkCommandLine().getLinkerInputs()));
     assertThat(cppLinkInfo.getInputFileList()).containsExactlyElementsIn(inputs);
     assertEquals(action.getPrimaryOutput().getExecPathString(), cppLinkInfo.getOutputFile());
-    Artifact interfaceOutput = action.getLinkCommandLine().getInterfaceOutput();
-    assertEquals(interfaceOutput.getExecPathString(), cppLinkInfo.getInterfaceOutputFile());
     assertEquals(action.getLinkCommandLine().getLinkTargetType().name(),
         cppLinkInfo.getLinkTargetType());
     assertEquals(action.getLinkCommandLine().getLinkStaticness().name(),
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 eda2715b38..cd92b39d1a 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
@@ -21,6 +21,7 @@ import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -54,10 +55,13 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class CppLinkActionTest extends BuildViewTestCase {
   private RuleContext createDummyRuleContext() throws Exception {
-    return view.getRuleContextForTesting(reporter, scratchConfiguredTarget(
-        "dummyRuleContext", "dummyRuleContext",
-        // CppLinkAction creation requires a CcToolchainProvider.
-        "cc_library(name = 'dummyRuleContext')"),
+    return view.getRuleContextForTesting(
+        reporter,
+        scratchConfiguredTarget(
+            "dummyRuleContext",
+            "dummyRuleContext",
+            // CppLinkAction creation requires a CcToolchainProvider.
+            "cc_library(name = 'dummyRuleContext')"),
         new StubAnalysisEnvironment() {
           @Override
           public void registerAction(ActionAnalysisMetadata... action) {
@@ -65,16 +69,23 @@ public class CppLinkActionTest extends BuildViewTestCase {
           }
 
           @Override
+          public Artifact getEmbeddedToolArtifact(String embeddedPath) {
+            return scratchArtifact("tools/interface_so_builder");
+          }
+
+          @Override
           public Artifact getDerivedArtifact(PathFragment rootRelativePath, Root root) {
             return CppLinkActionTest.this.getDerivedArtifact(
                 rootRelativePath, root, ActionsTestUtil.NULL_ARTIFACT_OWNER);
           }
-        }, masterConfig);
+        },
+        masterConfig);
   }
   
   private final FeatureConfiguration getMockFeatureConfiguration() throws Exception {
     return CcToolchainFeaturesTest.buildFeatures(
-            CppLinkActionConfigs.getCppLinkActionConfigs(CppLinkPlatform.LINUX))
+            CppLinkActionConfigs.getCppLinkActionConfigs(
+                CppLinkPlatform.LINUX, ImmutableSet.<String>of(), "dynamic_library_linker_tool"))
         .getFeatureConfiguration(
             Link.LinkTargetType.EXECUTABLE.getActionName(),
             Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(),
@@ -107,11 +118,10 @@ public class CppLinkActionTest extends BuildViewTestCase {
     CppLinkAction linkAction =
         createLinkBuilder(
                 Link.LinkTargetType.EXECUTABLE,
-                "out",
+                "dummyRuleContext/out",
                 ImmutableList.<Artifact>of(),
                 ImmutableList.<LibraryToLink>of(),
-                featureConfiguration,
-                false)
+                featureConfiguration)
             .build();
     assertThat(linkAction.getArgv()).contains("some_flag");
   }
@@ -160,11 +170,10 @@ public class CppLinkActionTest extends BuildViewTestCase {
     CppLinkAction linkAction =
         createLinkBuilder(
                 Link.LinkTargetType.EXECUTABLE,
-                "out",
+                "dummyRuleContext/out",
                 ImmutableList.<Artifact>of(),
                 ImmutableList.<LibraryToLink>of(),
-                featureConfiguration,
-                false)
+                featureConfiguration)
             .build();
     assertThat(linkAction.getEnvironment()).containsEntry("foo", "bar");
   }
@@ -315,11 +324,10 @@ public class CppLinkActionTest extends BuildViewTestCase {
     CppLinkAction linkAction =
         createLinkBuilder(
                 Link.LinkTargetType.EXECUTABLE,
-                "binary2",
+                "dummyRuleContext/binary2",
                 objects.build(),
                 ImmutableList.<LibraryToLink>of(),
-                new FeatureConfiguration(),
-                false)
+                new FeatureConfiguration())
             .setFake(true)
             .build();
 
@@ -350,8 +358,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
       String outputPath,
       Iterable<Artifact> nonLibraryInputs,
       ImmutableList<LibraryToLink> libraryInputs,
-      FeatureConfiguration featureConfiguration,
-      boolean shouldIncludeToolchain)
+      FeatureConfiguration featureConfiguration)
       throws Exception {
     RuleContext ruleContext = createDummyRuleContext();
     CppLinkActionBuilder builder =
@@ -359,10 +366,10 @@ public class CppLinkActionTest extends BuildViewTestCase {
                 ruleContext,
                 new Artifact(
                     new PathFragment(outputPath),
-                    getTargetConfiguration().getBinDirectory(
-                        ruleContext.getRule().getRepository())),
+                    getTargetConfiguration()
+                        .getBinDirectory(ruleContext.getRule().getRepository())),
                 ruleContext.getConfiguration(),
-                shouldIncludeToolchain ? CppHelper.getToolchain(ruleContext) : null)
+                CppHelper.getToolchain(ruleContext))
             .addObjectFiles(nonLibraryInputs)
             .addLibraries(NestedSetBuilder.wrap(Order.LINK_ORDER, libraryInputs))
             .setLinkType(type)
@@ -382,8 +389,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
         output.getPathString(),
         ImmutableList.<Artifact>of(),
         ImmutableList.<LibraryToLink>of(),
-        new FeatureConfiguration(),
-        true);
+        new FeatureConfiguration());
   }
 
   public Artifact getOutputArtifact(String relpath) {
@@ -424,6 +430,68 @@ public class CppLinkActionTest extends BuildViewTestCase {
   }
 
   @Test
+  public void testInterfaceOutputForDynamicLibrary() throws Exception {
+    FeatureConfiguration featureConfiguration =
+        CcToolchainFeaturesTest.buildFeatures(
+                "feature {",
+                "   name: 'build_interface_libraries'",
+                "   flag_set {",
+                "       action: '" + LinkTargetType.DYNAMIC_LIBRARY.getActionName() + "',",
+                "       flag_group {",
+                "           flag: '%{generate_interface_library}'",
+                "           flag: '%{interface_library_builder_path}'",
+                "           flag: '%{interface_library_input_path}'",
+                "           flag: '%{interface_library_output_path}'",
+                "       }",
+                "   }",
+                "}",
+                "feature {",
+                "   name: 'dynamic_library_linker_tool'",
+                "   flag_set {",
+                "       action: 'c++-link-dynamic-library'",
+                "       flag_group {",
+                "           flag: 'dynamic_library_linker_tool'",
+                "       }",
+                "   }",
+                "}",
+                "feature {",
+                "    name: 'has_configured_linker_path'",
+                "}",
+                "action_config {",
+                "   config_name: '" + LinkTargetType.DYNAMIC_LIBRARY.getActionName() + "'",
+                "   action_name: '" + LinkTargetType.DYNAMIC_LIBRARY.getActionName() + "'",
+                "   tool {",
+                "       tool_path: 'custom/crosstool/scripts/link_dynamic_library.sh'",
+                "   }",
+                "   implies: 'has_configured_linker_path'",
+                "   implies: 'build_interface_libraries'",
+                "   implies: 'dynamic_library_linker_tool'",
+                "}")
+            .getFeatureConfiguration(
+                "build_interface_libraries",
+                "dynamic_library_linker_tool",
+                LinkTargetType.DYNAMIC_LIBRARY.getActionName());
+    CppLinkActionBuilder builder =
+        createLinkBuilder(
+                LinkTargetType.DYNAMIC_LIBRARY,
+                "foo.so",
+                ImmutableList.<Artifact>of(),
+                ImmutableList.<LibraryToLink>of(),
+                featureConfiguration)
+            .setLibraryIdentifier("foo")
+            .setInterfaceOutput(scratchArtifact("FakeInterfaceOutput.ifso"));
+
+    List<String> commandLine = builder.build().getCommandLine();
+    assertThat(commandLine.size()).isGreaterThan(6);
+    assertThat(commandLine.get(0)).endsWith("custom/crosstool/scripts/link_dynamic_library.sh");
+    assertThat(commandLine.get(1)).isEqualTo("yes");
+    assertThat(commandLine.get(2)).isEqualTo("tools/interface_so_builder");
+    assertThat(commandLine.get(3)).endsWith("foo.so");
+    assertThat(commandLine.get(4)).isEqualTo("FakeInterfaceOutput.ifso");
+    assertThat(commandLine.get(5)).isEqualTo("dynamic_library_linker_tool");
+  }
+
+  @Test
   public void testStaticLinkWithDynamicIsError() throws Exception {
     CppLinkActionBuilder builder =
         createLinkBuilder(LinkTargetType.STATIC_LIBRARY)
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryLinkingTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryLinkingTest.java
index 3adb69ec75..30adaf693c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryLinkingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryLinkingTest.java
@@ -21,13 +21,11 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-
+import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.util.List;
-
 /**
  * Test for shared library linking {@link CppLinkAction}.
  */
@@ -35,8 +33,9 @@ import java.util.List;
 public final class LibraryLinkingTest extends BuildViewTestCase {
   private List<String> getLinkOpts(CppLinkAction linkAction, String... optionPatterns)
       throws Exception {
-    // Strip the first parameter from the argv, which is the gcc command.
-    return linkAction.getRawLinkArgv().subList(1, optionPatterns.length + 3);
+    // Strip the first parameters from the argv, which are the dynamic library script
+    // (usually tools/cpp/link_dynamic_library.sh), and its arguments.
+    return linkAction.getRawLinkArgv().subList(6, optionPatterns.length + 6);
   }
 
   private void assertLinkopts(CppLinkAction linkAction, String... optionPatterns) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
index 7e9a9dce85..5cd2ce3a91 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
+import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -162,7 +163,60 @@ public class LinkBuildVariablesTest extends BuildViewTestCase {
   }
 
   /**
-   * TODO(pcloudy): Add test for testing that necessary build variables are populated
-   * when alwayslink=1.
+   * TODO(pcloudy): Add test for testing that necessary build variables are populated when
+   * alwayslink=1.
    */
+  @Test
+  public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws Exception {
+    scratch.file("x/BUILD", "cc_library(", "   name = 'foo',", "   srcs = ['a.cc'],", ")");
+    scratch.file("x/a.cc");
+
+    ConfiguredTarget target = getConfiguredTarget("//x:foo");
+    Variables variables = getLinkBuildVariables(target, LinkTargetType.DYNAMIC_LIBRARY);
+
+    String interfaceLibraryBuilder =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE));
+    String interfaceLibraryInput =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE));
+    String interfaceLibraryOutput =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE));
+    String generateInterfaceLibrary =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE));
+
+    assertThat(generateInterfaceLibrary).isEqualTo("yes");
+    assertThat(interfaceLibraryInput).endsWith("libfoo.so");
+    assertThat(interfaceLibraryOutput).endsWith("libfoo.ifso");
+    assertThat(interfaceLibraryBuilder).endsWith("build_interface_so");
+  }
+
+  @Test
+  public void testInterfaceLibraryBuildingVariablesWhenGenerationNotAllowed() throws Exception {
+    scratch.file("x/BUILD", "cc_library(", "   name = 'foo',", "   srcs = ['a.cc'],", ")");
+    scratch.file("x/a.cc");
+
+    ConfiguredTarget target = getConfiguredTarget("//x:foo");
+    Variables variables = getLinkBuildVariables(target, LinkTargetType.STATIC_LIBRARY);
+
+    String interfaceLibraryBuilder =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE));
+    String interfaceLibraryInput =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE));
+    String interfaceLibraryOutput =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE));
+    String generateInterfaceLibrary =
+        Iterables.getOnlyElement(
+            getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE));
+
+    assertThat(generateInterfaceLibrary).isEqualTo("no");
+    assertThat(interfaceLibraryInput).endsWith("ignored");
+    assertThat(interfaceLibraryOutput).endsWith("ignored");
+    assertThat(interfaceLibraryBuilder).endsWith("ignored");
+  }
 }