From 8d2036106f68251a14371f478ec6ff886fed9398 Mon Sep 17 00:00:00 2001 From: plf Date: Thu, 11 Jan 2018 08:43:09 -0800 Subject: Automated rollback of commit c166cd99ce9f72eed522e78d63c93ff410b6dc18. *** Reason for rollback *** This was missing adding LTO files in the cc_embed_data rule. Fixed and added test. *** Original change description *** Automated rollback of commit 67330ad52391ad6562d439f77cc5133a0ea4247d. *** Reason for rollback *** Breaks nightly: b/71790513 *** Original change description *** C++ refactoring: Separate compilation and linking calls to CcLibraryHelper RELNOTES:none PiperOrigin-RevId: 181613477 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 101 +++++++++++---------- .../devtools/build/lib/rules/cpp/CcImport.java | 34 ++++--- .../devtools/build/lib/rules/cpp/CcIncLibrary.java | 18 +++- .../devtools/build/lib/rules/cpp/CcLibrary.java | 62 ++++++++----- .../build/lib/rules/cpp/CcLibraryHelper.java | 47 +++++----- .../devtools/build/lib/rules/cpp/CppModel.java | 5 +- .../build/lib/rules/cpp/proto/CcProtoAspect.java | 58 ++++++++---- .../build/lib/rules/objc/CompilationSupport.java | 6 +- 8 files changed, 194 insertions(+), 137 deletions(-) (limited to 'src/main/java/com') 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 69bd049dad..d5fb13f241 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 @@ -93,7 +93,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { RuleContext context, CcToolchainProvider toolchain, CcLinkingOutputs linkingOutputs, - CcLibraryHelper.Info info, + CcLinkingOutputs ccLibraryLinkingOutputs, + CppCompilationContext cppCompilationContext, LinkStaticness linkStaticness, NestedSet filesToBuild, Iterable fakeLinkerInputs, @@ -116,8 +117,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } if (linkCompileOutputSeparately) { builder.addArtifacts( - LinkerInputs.toLibraryArtifacts( - info.getCcLinkingOutputs().getExecutionDynamicLibraries())); + LinkerInputs.toLibraryArtifacts(ccLibraryLinkingOutputs.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. @@ -148,7 +148,6 @@ 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. @@ -195,30 +194,49 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { if (ruleContext.hasErrors()) { return null; } - + + // if cc_binary includes "linkshared=1", then gcc will be invoked with + // linkopt "-shared", which causes the result of linking to be a shared + // library. In this case, the name of the executable target should end + // in ".so" or "dylib" or ".dll". + PathFragment binaryPath = PathFragment.create(ruleContext.getTarget().getName()); + if (!isLinkShared(ruleContext)) { + binaryPath = PathFragment.create(binaryPath.getPathString() + OsUtils.executableExtension()); + } + + Artifact binary = ruleContext.getBinArtifact(binaryPath); + if (isLinkShared(ruleContext) + && !CppFileTypes.SHARED_LIBRARY.matches(binary.getFilename()) + && !CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(binary.getFilename())) { + ruleContext.attributeError("linkshared", "'linkshared' used in non-shared library"); + return null; + } + + CcLibraryHelper compilationHelper = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) + .fromCommon(common) + .addSources(common.getSources()) + .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext))) + .setFake(fake) + .addPrecompiledFiles(precompiledFiles); + Info.CompilationInfo compilationInfo = compilationHelper.compile(); + CppCompilationContext cppCompilationContext = compilationInfo.getCppCompilationContext(); + CcCompilationOutputs ccCompilationOutputs = compilationInfo.getCcCompilationOutputs(); + + CcLibraryHelper linkingHelper = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) + .fromCommon(common) + .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext))) + .setFake(fake) + .enableInterfaceSharedObjects(); List linkopts = common.getLinkopts(); LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration, ccToolchain); - // We currently only want link the dynamic library generated for test code separately. boolean linkCompileOutputSeparately = ruleContext.isTestTarget() && cppConfiguration.getLinkCompileOutputSeparately() && linkStaticness == LinkStaticness.DYNAMIC; - - CcLibraryHelper helper = - new CcLibraryHelper( - ruleContext, - semantics, - featureConfiguration, - ccToolchain, - fdoSupport) - .fromCommon(common) - .addSources(common.getSources()) - .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext))) - .setFake(fake) - .addPrecompiledFiles(precompiledFiles) - .enableInterfaceSharedObjects(); // When linking the object files directly into the resulting binary, we do not need // library-level link outputs; thus, we do not let CcLibraryHelper produce link outputs // (either shared object files or archives) for a non-library link type [*], and add @@ -231,28 +249,9 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // [*] The only library link type is STATIC_LIBRARY. EXECUTABLE specifies a normal // cc_binary output, while DYNAMIC_LIBRARY is a cc_binary rules that produces an // output matching a shared object, for example cc_binary(name="foo.so", ...) on linux. - helper.setLinkType(linkCompileOutputSeparately ? LinkTargetType.STATIC_LIBRARY : linkType); - - CcLibraryHelper.Info info = helper.build(); - CppCompilationContext cppCompilationContext = info.getCppCompilationContext(); - CcCompilationOutputs ccCompilationOutputs = info.getCcCompilationOutputs(); - - // if cc_binary includes "linkshared=1", then gcc will be invoked with - // linkopt "-shared", which causes the result of linking to be a shared - // library. In this case, the name of the executable target should end - // in ".so" or "dylib" or ".dll". - PathFragment binaryPath = PathFragment.create(ruleContext.getTarget().getName()); - if (!isLinkShared(ruleContext)) { - binaryPath = PathFragment.create(binaryPath.getPathString() + OsUtils.executableExtension()); - } - - Artifact binary = ruleContext.getBinArtifact(binaryPath); - if (isLinkShared(ruleContext) - && !CppFileTypes.SHARED_LIBRARY.matches(binary.getFilename()) - && !CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(binary.getFilename())) { - ruleContext.attributeError("linkshared", "'linkshared' used in non-shared library"); - return null; - } + linkingHelper.setLinkType( + linkCompileOutputSeparately ? LinkTargetType.STATIC_LIBRARY : linkType); + Info.LinkingInfo linkingInfo = linkingHelper.link(ccCompilationOutputs, cppCompilationContext); CcLinkParams linkParams = collectCcLinkParams( @@ -260,7 +259,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { linkStaticness != LinkStaticness.DYNAMIC, isLinkShared(ruleContext), linkopts); - CppLinkActionBuilder linkActionBuilder = determineLinkerArguments( ruleContext, @@ -269,7 +267,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { fdoSupport, common, precompiledFiles, - info, + ccCompilationOutputs, + linkingInfo.getCcLinkingOutputs(), cppCompilationContext.getTransitiveCompilationPrerequisites(), fake, binary, @@ -449,12 +448,13 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { ruleContext, ccToolchain, linkingOutputs, - info, + linkingInfo.getCcLinkingOutputs(), + cppCompilationContext, linkStaticness, filesToBuild, fakeLinkerInputs, fake, - helper.getCompilationUnitSources(), + compilationHelper.getCompilationUnitSources(), linkCompileOutputSeparately); RunfilesSupport runfilesSupport = RunfilesSupport.withExecutable( ruleContext, runfiles, executable); @@ -543,7 +543,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { FdoSupportProvider fdoSupport, CcCommon common, PrecompiledFiles precompiledFiles, - Info info, + CcCompilationOutputs compilationOutputs, + CcLinkingOutputs linkingOutputs, ImmutableSet compilationPrerequisites, boolean fake, Artifact binary, @@ -560,12 +561,12 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // Either link in the .o files generated for the sources of this target or link in the // generated dynamic library they are compiled into. if (linkCompileOutputSeparately) { - for (LibraryToLink library : info.getCcLinkingOutputs().getDynamicLibraries()) { + for (LibraryToLink library : linkingOutputs.getDynamicLibraries()) { builder.addLibrary(library); } } else { boolean usePic = CppHelper.usePic(context, toolchain, !isLinkShared(context)); - Iterable objectFiles = info.getCcCompilationOutputs().getObjectFiles(usePic); + Iterable objectFiles = compilationOutputs.getObjectFiles(usePic); if (fake) { builder.addFakeObjectFiles(objectFiles); @@ -574,7 +575,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } } - builder.addLtoBitcodeFiles(info.getCcCompilationOutputs().getLtoBitcodeFiles()); + builder.addLtoBitcodeFiles(compilationOutputs.getLtoBitcodeFiles()); builder.addNonCodeInputs(common.getLinkerScripts()); // Determine the libraries to link in. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java index b9173e71b9..f5e9f9dea5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java @@ -23,6 +23,7 @@ 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.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; @@ -68,13 +69,14 @@ public abstract class CcImport implements RuleConfiguredTargetFactory { CcCommon.configureFeatures(ruleContext, ccToolchain); FdoSupportProvider fdoSupport = CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext); - CcLibraryHelper helper = - new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport); - // Add headers + // Add headers to compilation step. final CcCommon common = new CcCommon(ruleContext); - helper.addPublicHeaders(common.getHeaders()); - helper.setHeadersCheckingMode(HeadersCheckingMode.STRICT); + Info.CompilationInfo compilationInfo = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) + .addPublicHeaders(common.getHeaders()) + .setHeadersCheckingMode(HeadersCheckingMode.STRICT) + .compile(); // Get alwayslink attribute boolean alwayslink = ruleContext.attributes().get("alwayslink", Type.BOOLEAN); @@ -88,14 +90,17 @@ public abstract class CcImport implements RuleConfiguredTargetFactory { .getRelative(labelName.replaceName("lib" + labelName.getBaseName())) .getPathString(); + CcLibraryHelper linkingHelper = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport); + if (staticLibrary != null) { if (CppFileTypes.PIC_ARCHIVE.matches(staticLibrary.getPath())) { - helper.addPicStaticLibraries( + linkingHelper.addPicStaticLibraries( ImmutableList.of( LinkerInputs.opaqueLibraryToLink( staticLibrary, staticLibraryCategory, libraryIdentifier, alwayslink))); } else { - helper.addStaticLibraries( + linkingHelper.addStaticLibraries( ImmutableList.of( LinkerInputs.opaqueLibraryToLink( staticLibrary, staticLibraryCategory, libraryIdentifier, alwayslink))); @@ -121,7 +126,7 @@ public abstract class CcImport implements RuleConfiguredTargetFactory { sharedLibrary, libraryIdentifier)); } - helper.addExecutionDynamicLibraries(executionDynamicLibraryList); + linkingHelper.addExecutionDynamicLibraries(executionDynamicLibraryList); } if (interfaceLibrary != null) { @@ -151,15 +156,20 @@ public abstract class CcImport implements RuleConfiguredTargetFactory { } if (dynamicLibraryList != null) { - helper.addDynamicLibraries(dynamicLibraryList); + linkingHelper.addDynamicLibraries(dynamicLibraryList); } - CcLibraryHelper.Info info = helper.build(); + Info.LinkingInfo linkingInfo = + linkingHelper.link( + compilationInfo.getCcCompilationOutputs(), compilationInfo.getCppCompilationContext()); return new RuleConfiguredTargetBuilder(ruleContext) - .addProviders(info.getProviders()) + .addProviders(compilationInfo.getProviders()) + .addProviders(linkingInfo.getProviders()) .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider()) - .addOutputGroups(info.getOutputGroups()) + .addOutputGroups( + Info.mergeOutputGroups( + compilationInfo.getOutputGroups(), linkingInfo.getOutputGroups())) .addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java index 260a7c655c..c1a40fc579 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider; +import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.Path; @@ -129,12 +130,18 @@ public abstract class CcIncLibrary implements RuleConfiguredTargetFactory { new CreateIncSymlinkAction(ruleContext.getActionOwner(), virtualArtifactMap, includeRoot)); FdoSupportProvider fdoSupport = CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext); - CcLibraryHelper.Info info = + Info.CompilationInfo compilationInfo = new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) .addIncludeDirs(Arrays.asList(includePath)) .addPublicHeaders(virtualArtifactMap.keySet()) .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET)) - .build(); + .compile(); + Info.LinkingInfo linkingInfo = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) + .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET)) + .link( + compilationInfo.getCcCompilationOutputs(), + compilationInfo.getCppCompilationContext()); // cc_inc_library doesn't compile any file - no compilation outputs available. InstrumentedFilesProvider instrumentedFilesProvider = @@ -143,9 +150,12 @@ public abstract class CcIncLibrary implements RuleConfiguredTargetFactory { /*withBaselineCoverage=*/true); return new RuleConfiguredTargetBuilder(ruleContext) - .addProviders(info.getProviders()) + .addProviders(compilationInfo.getProviders()) + .addProviders(linkingInfo.getProviders()) .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider()) - .addOutputGroups(info.getOutputGroups()) + .addOutputGroups( + Info.mergeOutputGroups( + compilationInfo.getOutputGroups(), linkingInfo.getOutputGroups())) .add(InstrumentedFilesProvider.class, instrumentedFilesProvider) .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)) .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 9777523b65..3457fedfd0 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RawAttributeMapper; import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier; +import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; @@ -131,14 +132,19 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { return; } - CcLibraryHelper helper = + CcLibraryHelper compilationHelper = new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) .fromCommon(common) - .addLinkopts(common.getLinkopts()) .addSources(common.getSources()) .addPublicHeaders(common.getHeaders()) - .enableCcNativeLibrariesProvider() .enableCompileProviders() + .addPrecompiledFiles(precompiledFiles); + + CcLibraryHelper linkingHelper = + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) + .fromCommon(common) + .addLinkopts(common.getLinkopts()) + .enableCcNativeLibrariesProvider() .enableInterfaceSharedObjects() // Generate .a and .so outputs even without object files to fulfill the rule class // contract wrt. implicit output files, if the contract says so. Behavior here differs @@ -147,7 +153,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { ruleContext.getRule().getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE) .setLinkType(staticLinkType) .setNeverLink(neverLink) - .addPrecompiledFiles(precompiledFiles) .addLinkstamps(ruleContext.getPrerequisites("linkstamp", Mode.TARGET)); Artifact soImplArtifact = null; @@ -180,7 +185,7 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { ruleContext.checkSrcsSamePackage(true); } if (ruleContext.getRule().isAttrDefined("textual_hdrs", BuildType.LABEL_LIST)) { - helper.addPublicTextualHeaders( + compilationHelper.addPublicTextualHeaders( ruleContext.getPrerequisiteArtifacts("textual_hdrs", Mode.TARGET).list()); } @@ -189,8 +194,8 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { + "Did you mean to use 'linkstatic=1' instead?"); } - helper.setCreateDynamicLibrary(createDynamicLibrary); - helper.setDynamicLibrary(soImplArtifact); + linkingHelper.setCreateDynamicLibrary(createDynamicLibrary); + linkingHelper.setDynamicLibrary(soImplArtifact); // If the reason we're not creating a dynamic library is that the toolchain // doesn't support it, then register an action which complains when triggered, @@ -252,10 +257,10 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { Iterable picAlwayslinkLibrariesFromSrcs = LinkerInputs.opaqueLibrariesToLink( ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY, precompiledFiles.getPicAlwayslinkLibraries()); - helper.addStaticLibraries(staticLibrariesFromSrcs); - helper.addStaticLibraries(alwayslinkLibrariesFromSrcs); - helper.addPicStaticLibraries(picStaticLibrariesFromSrcs); - helper.addPicStaticLibraries(picAlwayslinkLibrariesFromSrcs); + linkingHelper.addStaticLibraries(staticLibrariesFromSrcs); + linkingHelper.addStaticLibraries(alwayslinkLibrariesFromSrcs); + linkingHelper.addPicStaticLibraries(picStaticLibrariesFromSrcs); + linkingHelper.addPicStaticLibraries(picAlwayslinkLibrariesFromSrcs); Iterable dynamicLibraries = Iterables.transform( precompiledFiles.getSharedLibraries(), @@ -264,9 +269,13 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { common.getDynamicLibrarySymlink(library, true), library, CcLinkingOutputs.libraryIdentifierOf(library))); - helper.addDynamicLibraries(dynamicLibraries); - helper.addExecutionDynamicLibraries(dynamicLibraries); - CcLibraryHelper.Info info = helper.build(); + linkingHelper.addDynamicLibraries(dynamicLibraries); + linkingHelper.addExecutionDynamicLibraries(dynamicLibraries); + + Info.CompilationInfo compilationInfo = compilationHelper.compile(); + Info.LinkingInfo linkingInfo = + linkingHelper.link( + compilationInfo.getCcCompilationOutputs(), compilationInfo.getCppCompilationContext()); /* * We always generate a static library, even if there aren't any source files. @@ -276,7 +285,8 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { * However, we only generate a dynamic library if there are source files. */ // For now, we don't add the precompiled libraries to the files to build. - CcLinkingOutputs linkedLibraries = info.getCcLinkingOutputsExcludingPrecompiledLibraries(); + CcLinkingOutputs linkedLibraries = + linkingInfo.getCcLinkingOutputsExcludingPrecompiledLibraries(); NestedSetBuilder filesBuilder = NestedSetBuilder.stableOrder(); filesBuilder.addAll(LinkerInputs.toLibraryArtifacts(linkedLibraries.getStaticLibraries())); @@ -288,10 +298,9 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { LinkerInputs.toNonSolibArtifacts(linkedLibraries.getExecutionDynamicLibraries())); } - CcLinkingOutputs linkingOutputs = info.getCcLinkingOutputs(); + CcLinkingOutputs linkingOutputs = linkingInfo.getCcLinkingOutputs(); if (!featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) { - warnAboutEmptyLibraries( - ruleContext, info.getCcCompilationOutputs(), linkStatic); + warnAboutEmptyLibraries(ruleContext, compilationInfo.getCcCompilationOutputs(), linkStatic); } NestedSet filesToBuild = filesBuilder.build(); @@ -301,16 +310,20 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { neverLink, addDynamicRuntimeInputArtifactsToRunfiles, false); List instrumentedObjectFiles = new ArrayList<>(); - instrumentedObjectFiles.addAll(info.getCcCompilationOutputs().getObjectFiles(false)); - instrumentedObjectFiles.addAll(info.getCcCompilationOutputs().getObjectFiles(true)); + instrumentedObjectFiles.addAll(compilationInfo.getCcCompilationOutputs().getObjectFiles(false)); + instrumentedObjectFiles.addAll(compilationInfo.getCcCompilationOutputs().getObjectFiles(true)); InstrumentedFilesProvider instrumentedFilesProvider = common.getInstrumentedFilesProvider(instrumentedObjectFiles, /*withBaselineCoverage=*/true); CppHelper.maybeAddStaticLinkMarkerProvider(targetBuilder, ruleContext); + targetBuilder .setFilesToBuild(filesToBuild) - .addProviders(info.getProviders()) + .addProviders(compilationInfo.getProviders()) + .addProviders(linkingInfo.getProviders()) .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider()) - .addOutputGroups(info.getOutputGroups()) + .addOutputGroups( + Info.mergeOutputGroups( + compilationInfo.getOutputGroups(), linkingInfo.getOutputGroups())) .addProvider(InstrumentedFilesProvider.class, instrumentedFilesProvider) .addProvider( RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles)) @@ -320,10 +333,11 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { .addOutputGroup( OutputGroupInfo.HIDDEN_TOP_LEVEL, collectHiddenTopLevelArtifacts( - ruleContext, ccToolchain, info.getCcCompilationOutputs())) + ruleContext, ccToolchain, compilationInfo.getCcCompilationOutputs())) .addOutputGroup( CcLibraryHelper.HIDDEN_HEADER_TOKENS, - CcLibraryHelper.collectHeaderTokens(ruleContext, info.getCcCompilationOutputs())); + CcLibraryHelper.collectHeaderTokens( + ruleContext, compilationInfo.getCcCompilationOutputs())); } private static NestedSet collectHiddenTopLevelArtifacts( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 83a36f74a3..b67b6e1330 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -224,6 +224,27 @@ public final class CcLibraryHelper { public CcLinkingOutputs getCcLinkingOutputsExcludingPrecompiledLibraries() { return linkingOutputsExcludingPrecompiledLibraries; } + + /** + * Adds the static, pic-static libraries to the given builder. If addDynamicLibraries + * parameter is true, it also adds dynamic(both compile-time and execution-time) libraries. + */ + public void addLinkingOutputsTo( + NestedSetBuilder filesBuilder, boolean addDynamicLibraries) { + filesBuilder + .addAll(LinkerInputs.toLibraryArtifacts(linkingOutputs.getStaticLibraries())) + .addAll(LinkerInputs.toLibraryArtifacts(linkingOutputs.getPicStaticLibraries())); + if (addDynamicLibraries) { + filesBuilder + .addAll(LinkerInputs.toNonSolibArtifacts(linkingOutputs.getDynamicLibraries())) + .addAll( + LinkerInputs.toNonSolibArtifacts(linkingOutputs.getExecutionDynamicLibraries())); + } + } + + public void addLinkingOutputsTo(NestedSetBuilder filesBuilder) { + addLinkingOutputsTo(filesBuilder, true); + } } public Info(CompilationInfo compilationInfo, LinkingInfo linkingInfo) { @@ -272,36 +293,14 @@ public final class CcLibraryHelper { return context; } - /** - * Adds the static, pic-static libraries to the given builder. - * If addDynamicLibraries parameter is true, it also adds dynamic(both compile-time and - * execution-time) libraries. - */ - public void addLinkingOutputsTo( - NestedSetBuilder filesBuilder, boolean addDynamicLibraries) { - filesBuilder - .addAll(LinkerInputs.toLibraryArtifacts(linkingOutputs.getStaticLibraries())) - .addAll(LinkerInputs.toLibraryArtifacts(linkingOutputs.getPicStaticLibraries())); - if (addDynamicLibraries) { - filesBuilder - .addAll(LinkerInputs.toNonSolibArtifacts(linkingOutputs.getDynamicLibraries())) - .addAll( - LinkerInputs.toNonSolibArtifacts(linkingOutputs.getExecutionDynamicLibraries())); - } - } - - public void addLinkingOutputsTo(NestedSetBuilder filesBuilder) { - addLinkingOutputsTo(filesBuilder, true); - } - /** * Merges a list of output groups into one. The sets for each entry with a given key are merged. */ public static Map> mergeOutputGroups( - List>> outputGroupsList) { + Map>... outputGroups) { Map> mergedOutputGroupsBuilder = new TreeMap<>(); - for (Map> outputGroup : outputGroupsList) { + for (Map> outputGroup : outputGroups) { for (Map.Entry> entryOutputGroup : outputGroup.entrySet()) { String key = entryOutputGroup.getKey(); mergedOutputGroupsBuilder.computeIfAbsent( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index ebc2df87f5..234b8fdf6f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs.Builder; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StringSequenceBuilder; @@ -812,7 +811,7 @@ public final class CppModel { private void createHeaderAction( Label sourceLabel, String outputName, - Builder result, + CcCompilationOutputs.Builder result, AnalysisEnvironment env, CppCompileActionBuilder builder, boolean generateDotd) @@ -933,7 +932,7 @@ public final class CppModel { private void createClifMatchAction( Label sourceLabel, String outputName, - Builder result, + CcCompilationOutputs.Builder result, AnalysisEnvironment env, CppCompileActionBuilder builder) throws RuleErrorException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 8931024b93..aa98d37cac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; +import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -41,6 +42,7 @@ import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.cpp.CcCommon; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper; +import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; import com.google.devtools.build.lib.rules.cpp.CcToolchain; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; @@ -144,13 +146,12 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect FeatureConfiguration featureConfiguration = getFeatureConfiguration(supportData); ProtoConfiguration protoConfiguration = ruleContext.getFragment(ProtoConfiguration.class); - CcLibraryHelper helper = initializeCcLibraryHelper(featureConfiguration); - helper.addDeps(ruleContext.getPrerequisites("deps", TARGET)); + CcLibraryHelper compilationHelper = initializeCompilationHelper(featureConfiguration); // Compute and register files generated by this proto library. Collection outputs = new ArrayList<>(); if (areSrcsBlacklisted()) { - registerBlacklistedSrcs(supportData, helper); + registerBlacklistedSrcs(supportData, compilationHelper); headerProvider = null; } else if (supportData.hasProtoSources()) { Collection headers = @@ -160,8 +161,8 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect outputs.addAll(headers); outputs.addAll(sources); - helper.addSources(sources); - helper.addPublicHeaders(headers); + compilationHelper.addSources(sources); + compilationHelper.addPublicHeaders(headers); NestedSetBuilder publicHeaderPaths = NestedSetBuilder.stableOrder(); publicHeaderPaths.addAll(headers); @@ -176,7 +177,7 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect NestedSetBuilder transitiveHeaders = NestedSetBuilder.stableOrder(); for (ProtoCcHeaderProvider provider : ruleContext.getPrerequisites("deps", TARGET, ProtoCcHeaderProvider.class)) { - helper.addPublicTextualHeaders(provider.getHeaders()); + compilationHelper.addPublicTextualHeaders(provider.getHeaders()); transitiveHeaders.addTransitive(provider.getHeaders()); } headerProvider = new ProtoCcHeaderProvider(transitiveHeaders.build()); @@ -186,11 +187,24 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect filesBuilder.addAll(outputs); createProtoCompileAction(supportData, outputs); - CcLibraryHelper.Info info = helper.build(); - ccLibraryProviders = info.getProviders(); - outputGroups = info.getOutputGroups(); + Info.CompilationInfo compilationInfo = compilationHelper.compile(); + Info.LinkingInfo linkingInfo = + initializeLinkingHelper(featureConfiguration) + .link( + compilationInfo.getCcCompilationOutputs(), + compilationInfo.getCppCompilationContext()); + + ccLibraryProviders = + new TransitiveInfoProviderMapBuilder() + .addAll(compilationInfo.getProviders()) + .addAll(linkingInfo.getProviders()) + .build(); + outputGroups = + ImmutableMap.copyOf( + Info.mergeOutputGroups( + compilationInfo.getOutputGroups(), linkingInfo.getOutputGroups())); // On Windows, dynamic library is not built by default, so don't add them to filesToBuild. - info.addLinkingOutputsTo( + linkingInfo.addLinkingOutputsTo( filesBuilder, !featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)); } @@ -227,17 +241,29 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect featureConfiguration, ccToolchain(ruleContext), CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext)); - helper.enableCcSpecificLinkParamsProvider(); - helper.enableCcNativeLibrariesProvider(); + TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime(); + if (runtime != null) { + helper.addDeps(ImmutableList.of(runtime)); + } + + helper.addDeps(ruleContext.getPrerequisites("deps", TARGET)); + return helper; + } + + private CcLibraryHelper initializeCompilationHelper(FeatureConfiguration featureConfiguration) { + return initializeCcLibraryHelper(featureConfiguration); + } + + private CcLibraryHelper initializeLinkingHelper(FeatureConfiguration featureConfiguration) { + CcLibraryHelper helper = + initializeCcLibraryHelper(featureConfiguration) + .enableCcSpecificLinkParamsProvider() + .enableCcNativeLibrariesProvider(); // TODO(dougk): Configure output artifact with action_config // once proto compile action is configurable from the crosstool. if (!ccToolchain(ruleContext).supportsDynamicLinker()) { helper.setCreateDynamicLibrary(false); } - TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime(); - if (runtime != null) { - helper.addDeps(ImmutableList.of(runtime)); - } return helper; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index 7e4fa5ad73..80c8ddce93 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -479,14 +479,12 @@ public class CompilationSupport { resultLink.link( compilationOutputsBuilder.build(), compilationContextBuilder.build()); - List>> outputGroupsList = - Arrays.asList( + Map> mergedOutputGroups = + Info.mergeOutputGroups( objcArcCompilationInfo.getOutputGroups(), nonObjcArcCompilationInfo.getOutputGroups(), linkingInfo.getOutputGroups()); - Map> mergedOutputGroups = Info.mergeOutputGroups(outputGroupsList); - return new Pair<>(compilationOutputsBuilder.build(), ImmutableMap.copyOf(mergedOutputGroups)); } -- cgit v1.2.3