C++: Remove unnecessary runfiles providers.

PiperOrigin-RevId: 201969238

5 files changed, 78 insertions, 135 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 6737e5dde1..f38d0fbc36 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
@@ -91,7 +91,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   public static final String INTERMEDIATE_DWP_DIR = "_dwps";
 
   private static Runfiles collectRunfiles(
-      RuleContext context,
+      RuleContext ruleContext,
       FeatureConfiguration featureConfiguration,
       CcToolchainProvider toolchain,
       CcLinkingOutputs linkingOutputs,
@@ -103,16 +103,29 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       boolean fake,
       ImmutableSet<CppSource> cAndCppSources,
       boolean linkCompileOutputSeparately) {
-    Runfiles.Builder builder = new Runfiles.Builder(
-        context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles());
+    Runfiles.Builder builder =
+        new Runfiles.Builder(
+            ruleContext.getWorkspaceName(),
+            ruleContext.getConfiguration().legacyExternalRunfiles());
     Function<TransitiveInfoCollection, Runfiles> runfilesMapping =
-        CcRunfiles.runfilesFunction(linkingMode != Link.LinkingMode.DYNAMIC);
+        CppHelper.runfilesFunction(ruleContext, linkingMode != Link.LinkingMode.DYNAMIC);
     builder.addTransitiveArtifacts(filesToBuild);
     // Add the shared libraries to the runfiles. This adds any shared libraries that are in the
     // srcs of this target.
     builder.addArtifacts(linkingOutputs.getLibrariesForRunfiles(true));
-    builder.addRunfiles(context, RunfilesProvider.DEFAULT_RUNFILES);
-    builder.add(context, runfilesMapping);
+    builder.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
+    // TODO(plf): Why do we need .so files produced by cc_library in data dependencies of cc_binary?
+    // This can probably be removed safely.
+    for (TransitiveInfoCollection transitiveInfoCollection :
+        ruleContext.getPrerequisites("data", Mode.DONT_CHECK)) {
+      builder.merge(
+          CppHelper.runfilesFunction(ruleContext, /* linkingStatically= */ true)
+              .apply(transitiveInfoCollection));
+      builder.merge(
+          CppHelper.runfilesFunction(ruleContext, /* linkingStatically= */ false)
+              .apply(transitiveInfoCollection));
+    }
+    builder.add(ruleContext, runfilesMapping);
     // Add the C++ runtime libraries if linking them dynamically.
     if (linkingMode == Link.LinkingMode.DYNAMIC) {
       builder.addTransitiveArtifacts(toolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
@@ -125,9 +138,9 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     // 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);
+    boolean linkshared = isLinkShared(ruleContext);
     if (!linkshared) {
-      TransitiveInfoCollection malloc = CppHelper.mallocForTarget(context);
+      TransitiveInfoCollection malloc = CppHelper.mallocForTarget(ruleContext);
       builder.addTarget(malloc, RunfilesProvider.DEFAULT_RUNFILES);
       builder.addTarget(malloc, runfilesMapping);
     }
@@ -155,7 +168,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       // or header modules.
       builder.addSymlinksToArtifacts(ccCompilationContext.getAdditionalInputs());
       builder.addSymlinksToArtifacts(
-          ccCompilationContext.getTransitiveModules(usePic(context, toolchain)));
+          ccCompilationContext.getTransitiveModules(usePic(ruleContext, toolchain)));
     }
     return builder.build();
   }
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 7c8791b697..7025f3c888 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
@@ -66,31 +66,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
       CppFileTypes.ALWAYS_LINK_LIBRARY, CppFileTypes.ALWAYS_LINK_PIC_LIBRARY,
       CppFileTypes.SHARED_LIBRARY, CppFileTypes.VERSIONED_SHARED_LIBRARY);
 
-  private static Runfiles collectRunfiles(
-      RuleContext context,
-      FeatureConfiguration featureConfiguration,
-      CcLinkingOutputs ccLinkingOutputs,
-      CcToolchainProvider ccToolchain,
-      boolean neverLink,
-      boolean addDynamicRuntimeInputArtifactsToRunfiles,
-      boolean linkingStatically) {
-    Runfiles.Builder builder = new Runfiles.Builder(
-        context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles());
-
-    // neverlink= true creates a library that will never be linked into any binary that depends on
-    // it, but instead be loaded as an extension. So we need the dynamic library for this in the
-    // runfiles.
-    builder.addArtifacts(ccLinkingOutputs.getLibrariesForRunfiles(linkingStatically && !neverLink));
-    builder.add(context, CcRunfiles.runfilesFunction(linkingStatically));
-
-    builder.addDataDeps(context);
-
-    if (addDynamicRuntimeInputArtifactsToRunfiles) {
-      builder.addTransitiveArtifacts(ccToolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
-    }
-    return builder.build();
-  }
-
   @Override
   public ConfiguredTarget create(RuleContext context)
       throws InterruptedException, RuleErrorException, ActionConflictException {
@@ -329,7 +304,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
           LinkerInputs.toNonSolibArtifacts(linkedLibraries.getDynamicLibrariesForRuntime()));
     }
 
-    CcLinkingOutputs linkingOutputs = linkingInfo.getCcLinkingOutputs();
     if (!featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) {
       warnAboutEmptyLibraries(ruleContext, compilationInfo.getCcCompilationOutputs(), linkStatic);
     }
@@ -343,38 +317,33 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
             instrumentedObjectFiles, /* withBaselineCoverage= */ true);
     CppHelper.maybeAddStaticLinkMarkerProvider(targetBuilder, ruleContext);
 
-    Runfiles staticRunfiles =
-        collectRunfiles(
-            ruleContext,
-            featureConfiguration,
-            linkingOutputs,
-            ccToolchain,
-            neverLink,
-            addDynamicRuntimeInputArtifactsToRunfiles,
-            true);
-    Runfiles sharedRunfiles =
-        collectRunfiles(
-            ruleContext,
-            featureConfiguration,
-            linkingOutputs,
-            ccToolchain,
-            neverLink,
-            addDynamicRuntimeInputArtifactsToRunfiles,
-            false);
+    Runfiles.Builder builder = new Runfiles.Builder(ruleContext.getWorkspaceName());
+    builder.addDataDeps(ruleContext);
+    builder.add(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
+    if (addDynamicRuntimeInputArtifactsToRunfiles) {
+      builder.addTransitiveArtifacts(ccToolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
+    }
+    Runfiles runfiles = builder.build();
+    Runfiles.Builder defaultRunfiles =
+        new Runfiles.Builder(ruleContext.getWorkspaceName())
+            .merge(runfiles)
+            .addArtifacts(linkingInfo.getCcLinkingOutputs().getLibrariesForRunfiles(!neverLink));
+
+    Runfiles.Builder dataRunfiles =
+        new Runfiles.Builder(ruleContext.getWorkspaceName())
+            .merge(runfiles)
+            .addArtifacts(linkingInfo.getCcLinkingOutputs().getLibrariesForRunfiles(false));
 
     targetBuilder
         .setFilesToBuild(filesToBuild)
         .addProviders(compilationInfo.getProviders())
         .addProviders(linkingInfo.getProviders())
-        .addNativeDeclaredProvider(
-            overrideRunfilesProvider(staticRunfiles, sharedRunfiles, linkingInfo))
         .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider())
         .addOutputGroups(
             CcCommon.mergeOutputGroups(
                 ImmutableList.of(compilationInfo.getOutputGroups(), linkingInfo.getOutputGroups())))
         .addProvider(InstrumentedFilesProvider.class, instrumentedFilesProvider)
-        .addProvider(
-            RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles))
+        .addProvider(RunfilesProvider.withData(defaultRunfiles.build(), dataRunfiles.build()))
         .addOutputGroup(
             OutputGroupInfo.HIDDEN_TOP_LEVEL,
             collectHiddenTopLevelArtifacts(
@@ -385,19 +354,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
                 ruleContext, compilationInfo.getCcCompilationOutputs()));
   }
 
-  private static CcLinkingInfo overrideRunfilesProvider(
-      Runfiles staticRunfiles, Runfiles sharedRunfiles, LinkingInfo linkingInfo) {
-    CcLinkingInfo ccLinkingInfo =
-        (CcLinkingInfo) linkingInfo.getProviders().getProvider(CcLinkingInfo.PROVIDER.getKey());
-    CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
-    ccLinkingInfoBuilder.setCcLinkParamsStore(ccLinkingInfo.getCcLinkParamsStore());
-    ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
-        ccLinkingInfo.getCcDynamicLibrariesForRuntime());
-    ccLinkingInfoBuilder.setCcRunfiles(new CcRunfiles(staticRunfiles, sharedRunfiles));
-
-    return ccLinkingInfoBuilder.build();
-  }
-
   private static NestedSet<Artifact> collectHiddenTopLevelArtifacts(
       RuleContext ruleContext,
       CcToolchainProvider toolchain,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
index 8ffc398ae6..855c9cbb63 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
@@ -30,8 +30,6 @@ import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.LanguageDependentFragment;
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder;
@@ -531,11 +529,7 @@ public final class CcLinkingHelper {
       providers.add(new CcNativeLibraryProvider(collectNativeCcLibraries(ccLinkingOutputs)));
     }
 
-    Runfiles cppStaticRunfiles = collectCppRunfiles(ccLinkingOutputs, true);
-    Runfiles cppSharedRunfiles = collectCppRunfiles(ccLinkingOutputs, false);
-
     CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
-    ccLinkingInfoBuilder.setCcRunfiles(new CcRunfiles(cppStaticRunfiles, cppSharedRunfiles));
     ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
         collectDynamicLibrariesForRuntimeArtifacts(
             ccLinkingOutputs.getDynamicLibrariesForRuntime()));
@@ -615,19 +609,6 @@ public final class CcLinkingHelper {
     outputGroups.put(DYNAMIC_LIBRARY_OUTPUT_GROUP_NAME, dynamicLibrary.build());
   }
 
-  private Runfiles collectCppRunfiles(
-      CcLinkingOutputs ccLinkingOutputs, boolean linkingStatically) {
-    Runfiles.Builder builder =
-        new Runfiles.Builder(
-            ruleContext.getWorkspaceName(),
-            ruleContext.getConfiguration().legacyExternalRunfiles());
-    builder.addTargets(deps, RunfilesProvider.DEFAULT_RUNFILES);
-    builder.addTargets(deps, CcRunfiles.runfilesFunction(linkingStatically));
-    // Add the shared libraries to the runfiles.
-    builder.addArtifacts(ccLinkingOutputs.getLibrariesForRunfiles(linkingStatically));
-    return builder.build();
-  }
-
   private AbstractCcLinkParamsStore createCcLinkParamsStore(
       final CcLinkingOutputs ccLinkingOutputs,
       final CcCompilationContext ccCompilationContext,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcRunfiles.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcRunfiles.java
index d2b7a490fa..cdebe1d627 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcRunfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcRunfiles.java
@@ -14,9 +14,7 @@
 
 package com.google.devtools.build.lib.rules.cpp;
 
-import com.google.common.base.Function;
 import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcRunfilesApi;
@@ -28,60 +26,19 @@ import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcRunfilesApi;
  * dynamically linked binary. Both contain dynamic libraries needed at runtime and data
  * dependencies.
  */
+// TODO(plf): Remove class once Skylark rules have been migrated off it.
 @Immutable
 @AutoCodec
 public final class CcRunfiles implements CcRunfilesApi {
 
-  private final Runfiles runfilesForLinkingStatically;
-  private final Runfiles runfilesForLinkingDynamically;
-
-  @AutoCodec.Instantiator
-  public CcRunfiles(Runfiles runfilesForLinkingStatically, Runfiles runfilesForLinkingDynamically) {
-    this.runfilesForLinkingStatically = runfilesForLinkingStatically;
-    this.runfilesForLinkingDynamically = runfilesForLinkingDynamically;
-  }
-
   @Override
   public Runfiles getRunfilesForLinkingStatically() {
-    return runfilesForLinkingStatically;
+    return Runfiles.EMPTY;
   }
 
   @Override
   public Runfiles getRunfilesForLinkingDynamically() {
-    return runfilesForLinkingDynamically;
+    return Runfiles.EMPTY;
   }
 
-  /**
-   * Returns a function that gets the static C++ runfiles from a {@link TransitiveInfoCollection} or
-   * the empty runfiles instance if it does not contain that provider.
-   */
-  public static final Function<TransitiveInfoCollection, Runfiles> RUNFILES_FOR_LINKING_STATICALLY =
-      input -> {
-        CcLinkingInfo provider = input.get(CcLinkingInfo.PROVIDER);
-        CcRunfiles ccRunfiles = provider == null ? null : provider.getCcRunfiles();
-        return ccRunfiles == null ? Runfiles.EMPTY : ccRunfiles.getRunfilesForLinkingStatically();
-      };
-
-  /**
-   * Returns a function that gets the shared C++ runfiles from a {@link TransitiveInfoCollection} or
-   * the empty runfiles instance if it does not contain that provider.
-   */
-  public static final Function<TransitiveInfoCollection, Runfiles>
-      RUNFILES_FOR_LINKING_DYNAMICALLY =
-          input -> {
-            CcLinkingInfo provider = input.get(CcLinkingInfo.PROVIDER);
-            CcRunfiles ccRunfiles = provider == null ? null : provider.getCcRunfiles();
-            return ccRunfiles == null
-                ? Runfiles.EMPTY
-                : ccRunfiles.getRunfilesForLinkingDynamically();
-          };
-
-  /**
-   * Returns a function that gets the C++ runfiles from a {@link TransitiveInfoCollection} or
-   * the empty runfiles instance if it does not contain that provider.
-   */
-  public static final Function<TransitiveInfoCollection, Runfiles> runfilesFunction(
-      boolean linkingStatically) {
-    return linkingStatically ? RUNFILES_FOR_LINKING_STATICALLY : RUNFILES_FOR_LINKING_DYNAMICALLY;
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 63a94cf5f7..e94f3038e7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -37,6 +38,7 @@ import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.StaticallyLinkedMarkerProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
@@ -482,6 +484,40 @@ public class CppHelper {
   }
 
   /**
+   * Returns a function that gets the C++ runfiles from a {@link TransitiveInfoCollection} or the
+   * empty runfiles instance if it does not contain that provider.
+   */
+  public static final Function<TransitiveInfoCollection, Runfiles> runfilesFunction(
+      RuleContext ruleContext, boolean linkingStatically) {
+    final Function<TransitiveInfoCollection, Runfiles> runfilesForLinkingDynamically =
+        input -> {
+          CcLinkingInfo provider = input.get(CcLinkingInfo.PROVIDER);
+          CcLinkParamsStore ccLinkParamsStore =
+              provider == null ? null : provider.getCcLinkParamsStore();
+          return ccLinkParamsStore == null
+              ? Runfiles.EMPTY
+              : new Runfiles.Builder(ruleContext.getWorkspaceName())
+                  .addTransitiveArtifacts(
+                      ccLinkParamsStore.get(false, false).getDynamicLibrariesForRuntime())
+                  .build();
+        };
+
+    final Function<TransitiveInfoCollection, Runfiles> runfilesForLinkingStatically =
+        input -> {
+          CcLinkingInfo provider = input.get(CcLinkingInfo.PROVIDER);
+          CcLinkParamsStore ccLinkParamsStore =
+              provider == null ? null : provider.getCcLinkParamsStore();
+          return ccLinkParamsStore == null
+              ? Runfiles.EMPTY
+              : new Runfiles.Builder(ruleContext.getWorkspaceName())
+                  .addTransitiveArtifacts(
+                      ccLinkParamsStore.get(true, false).getDynamicLibrariesForRuntime())
+                  .build();
+        };
+    return linkingStatically ? runfilesForLinkingStatically : runfilesForLinkingDynamically;
+  }
+
+  /**
    * A header that has been grepped for #include statements. Includes the original header as well as
    * a stripped version of that header that contains only #include statements.
    */