C++: Remove CcDynamicLibrariesForRuntime.

This was providing runtime libraries. This provider is redundant because the same libraries are in CcLinkParamsStore. Note to sheriff: Shouldn't break anything. If any, breakages would most likely be in Go test rules. I will look at the nightly tomorrow anyway. The breakages in the linked TGP re-run are not caused by this change, later re-runs have unrelated changes. TESTED=[] RELNOTES:none PiperOrigin-RevId: 204738136
bazelbuild · Jul 16, 2018 · 020ac7e · 020ac7e
1 parent 1281a02
commit 020ac7e
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 18 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
@@ -886,10 +886,12 @@ private static void addTransitiveInfoProviders(
     ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext);
 
     CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
-    ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
-        new CcDynamicLibrariesForRuntime(
-            collectDynamicLibrariesForRuntimeArtifacts(
-                ruleContext, linkingOutputs.getDynamicLibrariesForRuntime())));
+    if (cppConfiguration.enableCcDynamicLibrariesForRuntime()) {
+      ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
+          new CcDynamicLibrariesForRuntime(
+              collectDynamicLibrariesForRuntimeArtifacts(
+                  ruleContext, linkingOutputs.getDynamicLibrariesForRuntime())));
+    }
 
     builder
         .setFilesToBuild(filesToBuild)

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
@@ -531,10 +531,11 @@ public LinkingInfo link(CcCompilationOutputs ccOutputs, CcCompilationContext ccC
     }
 
     CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
-    ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
-        collectDynamicLibrariesForRuntimeArtifacts(
-            ccLinkingOutputs.getDynamicLibrariesForRuntime()));
-
+    if (cppConfiguration.enableCcDynamicLibrariesForRuntime()) {
+      ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime(
+          collectDynamicLibrariesForRuntimeArtifacts(
+              ccLinkingOutputs.getDynamicLibrariesForRuntime()));
+    }
     CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
     boolean forcePic = cppConfiguration.forcePic();
     ccLinkingInfoBuilder.setCcLinkParamsStore(

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
@@ -1161,6 +1161,11 @@ public boolean useLLVMCoverageMapFormat() {
     return cppOptions.useLLVMCoverageMapFormat;
   }
 
+  /** Returns true if the deprecated CcDynamicLibrariesForRuntime class should be used */
+  public boolean enableCcDynamicLibrariesForRuntime() {
+    return cppOptions.enableCcDynamicLibrariesForRuntime;
+  }
+
   private void checkForToolchainSkylarkApiAvailability() throws EvalException {
     if (!cppOptions.enableLegacyToolchainSkylarkApi) {
       throw new EvalException(null, "Information about the C++ toolchain API is not accessible "

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
@@ -803,6 +803,17 @@ public Label getFdoPrefetchHintsLabel() {
               + "breaking changes.")
   public boolean enableCcSkylarkApi;
 
+  @Option(
+      name = "experimental_enable_cc_dynlibs_for_runtime",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.EAGERNESS_TO_EXIT},
+      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+      help =
+          "If false, Blaze will not propagate runtime libs through CcDynamicLibrariesForRuntime "
+              + "field of CcLinkingInfo. See b/111289526.")
+  public boolean enableCcDynamicLibrariesForRuntime;
+
   @Override
   public FragmentOptions getHost() {
     CppOptions host = (CppOptions) getDefault();

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
@@ -119,8 +119,9 @@ public void testEmptyLibrary() throws Exception {
     assertThat(
             emptylib
                 .get(CcLinkingInfo.PROVIDER)
-                .getCcDynamicLibrariesForRuntime()
-                .getDynamicLibrariesForRuntimeArtifacts()
+                .getCcLinkParamsStore()
+                .get(/* linkingStatically= */ false, /* linkShared= */ false)
+                .getDynamicLibrariesForRuntime()
                 .isEmpty())
         .isTrue();
   }
@@ -232,8 +233,9 @@ public void testLinkStaticStatically() throws Exception {
     assertThat(
             statically
                 .get(CcLinkingInfo.PROVIDER)
-                .getCcDynamicLibrariesForRuntime()
-                .getDynamicLibrariesForRuntimeArtifacts()
+                .getCcLinkParamsStore()
+                .get(/* linkingStatically= */ false, /* linkShared= */ false)
+                .getDynamicLibrariesForRuntime()
                 .isEmpty())
         .isTrue();
     Artifact staticallyDotA = getOnlyElement(getFilesToBuild(statically));

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
@@ -219,8 +219,9 @@ public void testFilesToBuild() throws Exception {
     assertThat(
             hello
                 .get(CcLinkingInfo.PROVIDER)
-                .getCcDynamicLibrariesForRuntime()
-                .getDynamicLibrariesForRuntimeArtifacts())
+                .getCcLinkParamsStore()
+                .get(/* linkingStatically= */ false, /* linkShared= */ false)
+                .getDynamicLibrariesForRuntime())
         .containsExactly(implSharedObjectLink);
   }
 
@@ -284,8 +285,9 @@ public void testFilesToBuildWithInterfaceSharedObjects() throws Exception {
     assertThat(
             hello
                 .get(CcLinkingInfo.PROVIDER)
-                .getCcDynamicLibrariesForRuntime()
-                .getDynamicLibrariesForRuntimeArtifacts())
+                .getCcLinkParamsStore()
+                .get(/* linkingStatically= */ false, /* linkShared= */ false)
+                .getDynamicLibrariesForRuntime())
         .containsExactly(implSharedObjectLink);
   }
 

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
@@ -111,8 +111,9 @@ public boolean apply(Artifact artifact) {
         Iterables.getOnlyElement(
             ccLib
                 .get(CcLinkingInfo.PROVIDER)
-                .getCcDynamicLibrariesForRuntime()
-                .getDynamicLibrariesForRuntimeArtifacts());
+                .getCcLinkParamsStore()
+                .get(/* linkingStatically= */ false, /* linkShared= */ false)
+                .getDynamicLibrariesForRuntime());
     // This artifact is generated by a SolibSymlinkAction, so we need to go back two levels.
     CppLinkAction solibLink =
         (CppLinkAction) getGeneratingAction(getGeneratingAction(soLib).getPrimaryInput());