From a1d0aecc2ae8d67bf7b1112d4572906f98a215b4 Mon Sep 17 00:00:00 2001 From: plf Date: Mon, 25 Jun 2018 09:50:56 -0700 Subject: C++: Remove unnecessary runfiles providers. PiperOrigin-RevId: 201969238 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 31 ++++++--- .../devtools/build/lib/rules/cpp/CcLibrary.java | 78 +++++----------------- .../build/lib/rules/cpp/CcLinkingHelper.java | 19 ------ .../devtools/build/lib/rules/cpp/CcRunfiles.java | 49 +------------- .../devtools/build/lib/rules/cpp/CppHelper.java | 36 ++++++++++ 5 files changed, 78 insertions(+), 135 deletions(-) (limited to 'src/main/java/com/google/devtools') 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 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 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 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 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 - 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 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; @@ -481,6 +483,40 @@ public class CppHelper { return getObjDirectory(ruleLabel, false); } + /** + * 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 runfilesFunction( + RuleContext ruleContext, boolean linkingStatically) { + final Function 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 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. -- cgit v1.2.3