From 020ac7ef02f4de0a85d73737e4316c5fc778f97e Mon Sep 17 00:00:00 2001 From: plf Date: Mon, 16 Jul 2018 07:19:33 -0700 Subject: [PATCH] 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 --- .../google/devtools/build/lib/rules/cpp/CcBinary.java | 10 ++++++---- .../devtools/build/lib/rules/cpp/CcLinkingHelper.java | 9 +++++---- .../build/lib/rules/cpp/CppConfiguration.java | 5 +++++ .../devtools/build/lib/rules/cpp/CppOptions.java | 11 +++++++++++ .../devtools/build/lib/rules/cpp/CcCommonTest.java | 10 ++++++---- .../lib/rules/cpp/CcLibraryConfiguredTargetTest.java | 10 ++++++---- .../build/lib/rules/cpp/LibraryLinkingTest.java | 5 +++-- 7 files changed, 42 insertions(+), 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 index 47bc2b0ed384d4..36aca5f7df33f5 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 @@ -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 index e2448cd425c46f..2c0e64e6386791 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 @@ -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 index bfb6528ad2572e..375f735c7235c1 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 @@ -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 index 0744f005d88734..2ecb33e6d6bc99 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 @@ -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 index 49b14ea565dc9a..580c4e7f7eb013 100644 --- 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 index 6b128f17fb3e3c..b3c23cd3703b59 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 @@ -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 index 6a98bc478c6b0d..130c5ff8db03ec 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 @@ -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());