Add option to dynamically link in the compilation output of cc_test and

cc_binary if we are doing dynamic linking anyway. The additional option is
only for a smooth rollout and will go away once everything works as expected.

The benefit is in an incremental save-build-test-cycle. When editing a test (or
any of it's transitive headers), the linker input currently changes, meaning we
need to do the full link again before the test can run. However, in conjunction
with the option --interface_shared_objects, this actually isn't necessary. With
this new option, the test source gets compiled into a .so file and while that
file changes with each source change, the .ifso file often does not change.
Thus, the costly link linking to gether all the .so and .ifso files can be
retrieved from cache. Thus, this change should remove this linking step from
the critical path of many save-build-test-cycles.

--
PiperOrigin-RevId: 142244352
MOS_MIGRATED_REVID=142244352

5 files changed, 75 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 1cb110aab1..f1146956d7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -83,7 +83,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   private static Runfiles collectRunfiles(
       RuleContext context,
       CcLinkingOutputs linkingOutputs,
-      CppCompilationContext cppCompilationContext,
+      CcLibraryHelper.Info info,
       LinkStaticness linkStaticness,
       NestedSet<Artifact> filesToBuild,
       Iterable<Artifact> fakeLinkerInputs,
@@ -93,7 +93,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles());
     Function<TransitiveInfoCollection, Runfiles> runfilesMapping =
         CppRunfilesProvider.runfilesFunction(linkStaticness != LinkStaticness.DYNAMIC);
-    boolean linkshared = isLinkShared(context);
     builder.addTransitiveArtifacts(filesToBuild);
     // Add the shared libraries to the runfiles. This adds any shared libraries that are in the
     // srcs of this target.
@@ -104,11 +103,18 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     // Add the C++ runtime libraries if linking them dynamically.
     if (linkStaticness == LinkStaticness.DYNAMIC) {
       builder.addTransitiveArtifacts(toolchain.getDynamicRuntimeLinkInputs());
+      CppConfiguration cppConfiguration = context.getFragment(CppConfiguration.class);
+      if (cppConfiguration.getLinkDynamicBinariesSeparately()) {
+        builder.addArtifacts(
+            LinkerInputs.toLibraryArtifacts(
+                info.getCcLinkingOutputs().getExecutionDynamicLibraries()));
+      }
     }
     // For cc_binary and cc_test rules, there is an implicit dependency on
     // the malloc library package, which is specified by the "malloc" attribute.
     // As the BUILD encyclopedia says, the "malloc" attribute should be ignored
     // if linkshared=1.
+    boolean linkshared = isLinkShared(context);
     if (!linkshared) {
       TransitiveInfoCollection malloc = CppHelper.mallocForTarget(context);
       builder.addTarget(malloc, RunfilesProvider.DEFAULT_RUNFILES);
@@ -133,6 +139,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         sourcesBuilder.add(cppSource.getSource());
       }
       builder.addSymlinksToArtifacts(sourcesBuilder.build());
+      CppCompilationContext cppCompilationContext = info.getCppCompilationContext();
       builder.addSymlinksToArtifacts(cppCompilationContext.getDeclaredIncludeSrcs());
       // Add additional files that are referenced from the compile command, like module maps
       // or header modules.
@@ -147,11 +154,10 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext context)
       throws InterruptedException, RuleErrorException {
-    return CcBinary.init(semantics, context, /*fake =*/ false, /*isTest =*/ false);
+    return CcBinary.init(semantics, context, /*fake =*/ false);
   }
 
-  public static ConfiguredTarget init(
-      CppSemantics semantics, RuleContext ruleContext, boolean fake, boolean isTest)
+  public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake)
       throws InterruptedException, RuleErrorException {
     ruleContext.checkSrcsSamePackage(true);
     FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext);
@@ -159,11 +165,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
     PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext);
 
-    LinkTargetType linkType =
-        isLinkShared(ruleContext) ? LinkTargetType.DYNAMIC_LIBRARY : LinkTargetType.EXECUTABLE;
-    List<String> linkopts = common.getLinkopts();
-    LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration);
-
     semantics.validateAttributes(ruleContext);
     if (ruleContext.hasErrors()) {
       return null;
@@ -175,8 +176,9 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
             .addSources(common.getSources())
             .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext)))
             .setFake(fake)
-            .setLinkType(linkType)
-            .addPrecompiledFiles(precompiledFiles);
+            .setLinkType(LinkTargetType.STATIC_LIBRARY)
+            .addPrecompiledFiles(precompiledFiles)
+            .enableInterfaceSharedObjects();
 
     CcLibraryHelper.Info info = helper.build();
     CppCompilationContext cppCompilationContext = info.getCppCompilationContext();
@@ -197,18 +199,23 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       return null;
     }
 
+    LinkTargetType linkType =
+        isLinkShared(ruleContext) ? LinkTargetType.DYNAMIC_LIBRARY : LinkTargetType.EXECUTABLE;
+    List<String> linkopts = common.getLinkopts();
+    LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration);
+
     CppLinkActionBuilder linkActionBuilder =
         determineLinkerArguments(
             ruleContext,
             common,
             precompiledFiles,
-            ccCompilationOutputs,
+            info,
             cppCompilationContext.getTransitiveCompilationPrerequisites(),
             fake,
             binary,
             linkStaticness,
             linkopts);
-    linkActionBuilder.setUseTestOnlyFlags(isTest);
+    linkActionBuilder.setUseTestOnlyFlags(ruleContext.isTestTarget());
 
     CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext);
     if (linkStaticness == LinkStaticness.DYNAMIC) {
@@ -316,7 +323,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         collectRunfiles(
             ruleContext,
             linkingOutputs,
-            cppCompilationContext,
+            info,
             linkStaticness,
             filesToBuild,
             fakeLinkerInputs,
@@ -367,15 +374,18 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     }
 
     return ruleBuilder
-        .add(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
-        .add(
+        .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
+        .addProvider(
             CppDebugPackageProvider.class,
             new CppDebugPackageProvider(
                 ruleContext.getLabel(), strippedFile, executable, explicitDwpFile))
         .setRunfilesSupport(runfilesSupport, executable)
-        .addProvider(LipoContextProvider.class, new LipoContextProvider(
-            cppCompilationContext, ImmutableMap.copyOf(scannableMap),
-            ImmutableMap.copyOf(sourceFileMap)))
+        .addProvider(
+            LipoContextProvider.class,
+            new LipoContextProvider(
+                cppCompilationContext,
+                ImmutableMap.copyOf(scannableMap),
+                ImmutableMap.copyOf(sourceFileMap)))
         .addProvider(CppLinkAction.Context.class, linkContext)
         .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider())
         .build();
@@ -389,7 +399,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       RuleContext context,
       CcCommon common,
       PrecompiledFiles precompiledFiles,
-      CcCompilationOutputs compilationOutputs,
+      CcLibraryHelper.Info info,
       ImmutableSet<Artifact> compilationPrerequisites,
       boolean fake,
       Artifact binary,
@@ -401,17 +411,26 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
             .setCrosstoolInputs(CppHelper.getToolchain(context).getLink())
             .addNonCodeInputs(compilationPrerequisites);
 
-    // Determine the object files to link in.
-    boolean usePic = CppHelper.usePic(context, !isLinkShared(context));
-    Iterable<Artifact> objectFiles = compilationOutputs.getObjectFiles(usePic);
-
-    if (fake) {
-      builder.addFakeObjectFiles(objectFiles);
+    // Either link in the .o files generated for the sources of this target or link in the
+    // generated dynamic library they are compiled into.
+    CppConfiguration cppConfiguration = context.getFragment(CppConfiguration.class);
+    if (cppConfiguration.getLinkDynamicBinariesSeparately()
+        && linkStaticness == LinkStaticness.DYNAMIC) {
+      for (LibraryToLink library : info.getCcLinkingOutputs().getDynamicLibraries()) {
+        builder.addLibrary(library);
+      }
     } else {
-      builder.addObjectFiles(objectFiles);
+      boolean usePic = CppHelper.usePic(context, !isLinkShared(context));
+      Iterable<Artifact> objectFiles = info.getCcCompilationOutputs().getObjectFiles(usePic);
+
+      if (fake) {
+        builder.addFakeObjectFiles(objectFiles);
+      } else {
+        builder.addObjectFiles(objectFiles);
+      }
     }
 
-    builder.addLTOBitcodeFiles(compilationOutputs.getLtoBitcodeFiles());
+    builder.addLTOBitcodeFiles(info.getCcCompilationOutputs().getLtoBitcodeFiles());
     builder.addNonCodeInputs(common.getLinkerScripts());
 
     // Determine the libraries to link in.
@@ -662,20 +681,20 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
             CppHelper.usePic(ruleContext, false));
     builder
         .setFilesToBuild(filesToBuild)
-        .add(CppCompilationContext.class, cppCompilationContext)
-        .add(TransitiveLipoInfoProvider.class, transitiveLipoInfo)
-        .add(
+        .addProvider(CppCompilationContext.class, cppCompilationContext)
+        .addProvider(TransitiveLipoInfoProvider.class, transitiveLipoInfo)
+        .addProvider(
             CcExecutionDynamicLibrariesProvider.class,
             new CcExecutionDynamicLibrariesProvider(
                 collectExecutionDynamicLibraryArtifacts(
                     ruleContext, linkingOutputs.getExecutionDynamicLibraries())))
-        .add(
+        .addProvider(
             CcNativeLibraryProvider.class,
             new CcNativeLibraryProvider(
                 collectTransitiveCcNativeLibraries(
                     ruleContext, linkingOutputs.getDynamicLibraries())))
-        .add(InstrumentedFilesProvider.class, instrumentedFilesProvider)
-        .add(
+        .addProvider(InstrumentedFilesProvider.class, instrumentedFilesProvider)
+        .addProvider(
             CppDebugFileProvider.class,
             new CppDebugFileProvider(
                 dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts()))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 54be53e229..d7d2e7a08a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -274,10 +274,12 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
         .addProviders(info.getProviders())
         .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider())
         .addOutputGroups(info.getOutputGroups())
-        .add(InstrumentedFilesProvider.class, instrumentedFilesProvider)
-        .add(RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles))
+        .addProvider(InstrumentedFilesProvider.class, instrumentedFilesProvider)
+        .addProvider(
+            RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles))
         // Remove this?
-        .add(CppRunfilesProvider.class, new CppRunfilesProvider(staticRunfiles, sharedRunfiles))
+        .addProvider(
+            CppRunfilesProvider.class, new CppRunfilesProvider(staticRunfiles, sharedRunfiles))
         .addOutputGroup(
             OutputGroupProvider.HIDDEN_TOP_LEVEL,
             collectHiddenTopLevelArtifacts(ruleContext, info.getCcCompilationOutputs()))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
index acfb5c33ed..80a406262c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
@@ -32,6 +32,6 @@ public abstract class CcTest implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext context)
       throws InterruptedException, RuleErrorException {
-    return CcBinary.init(semantics, context, /*fake =*/ false, /*isTest =*/ true);
+    return CcBinary.init(semantics, context, /*fake =*/ false);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 489f71b3d0..ff87b5ac09 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -1598,6 +1598,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return dynamicMode;
   }
 
+  public boolean getLinkDynamicBinariesSeparately() {
+    return cppOptions.linkDynamicBinariesSeparately;
+  }
+
   /*
    * If true then the directory name for non-LIPO targets will have a '-lipodata' suffix in
    * AutoFDO mode.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index fbc8fb80fe..ecbe6bb70b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -229,6 +229,17 @@ public class CppOptions extends FragmentOptions {
   public DynamicModeFlag dynamicMode;
 
   @Option(
+    name = "experimental_link_dynamic_binaries_separately",
+    defaultValue = "false",
+    category = "semantics",
+    help =
+        "This flag is experimental and may go away at any time.  "
+            + "If true, dynamically linked binary targets will build and link their own srcs as "
+            + "a dynamic library instead of directly linking it in."
+  )
+  public boolean linkDynamicBinariesSeparately;
+
+  @Option(
     name = "force_pic",
     defaultValue = "false",
     category = "semantics",