Skip to content

Commit

Permalink
C++: Remove more CcLinkParams
Browse files Browse the repository at this point in the history 
Tracking: bazelbuild#4570

Fixes conversion bug from old API to new that was already present before the original CL but was only triggered with it.

RELNOTES:none
PiperOrigin-RevId: 229910560
  • Loading branch information
@oquenchil
oquenchil authored and Copybara-Service committed Jan 18, 2019
1 parent ce91f76 commit 56f2b2b
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 98 deletions.
181 changes: 114 additions & 67 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@
import com.google.devtools.build.lib.rules.apple.ApplePlatform;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams.LinkOptions;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
Expand Down Expand Up @@ -112,12 +114,11 @@ public static class CcLauncherInfo extends NativeInfo {
public static final Provider PROVIDER = new Provider();

private final CcCompilationOutputs ccCompilationOutputs;
private final CcLinkingInfo staticModeParamsForExecutable;
private final CcInfo ccInfo;

public CcLauncherInfo(
CcLinkingInfo staticModeParamsForExecutable, CcCompilationOutputs ccCompilationOutputs) {
public CcLauncherInfo(CcInfo ccInfo, CcCompilationOutputs ccCompilationOutputs) {
super(PROVIDER);
this.staticModeParamsForExecutable = staticModeParamsForExecutable;
this.ccInfo = ccInfo;
this.ccCompilationOutputs = ccCompilationOutputs;
}

Expand All @@ -126,9 +127,9 @@ public CcCompilationOutputs getCcCompilationOutputs(RuleContext ruleContext) {
return ccCompilationOutputs;
}

public CcLinkingInfo getStaticModeParamsForExecutable(RuleContext ruleContext) {
public CcInfo getCcInfo(RuleContext ruleContext) {
checkRestrictedUsage(ruleContext);
return staticModeParamsForExecutable;
return ccInfo;
}

private void checkRestrictedUsage(RuleContext ruleContext) {
Expand Down Expand Up @@ -358,24 +359,31 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
}

boolean isStaticMode = linkingMode != LinkingMode.DYNAMIC;
boolean forDynamicLibrary = isLinkShared(ruleContext);

CcLinkingInfo depsCcLinkingInfo = collectCcLinkingInfo(ruleContext);
CcLinkingContext depsCcLinkingContext = collectCcLinkingContext(ruleContext);

Artifact generatedDefFile = null;
Artifact customDefFile = null;
if (isLinkShared(ruleContext)) {
if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
ImmutableList.Builder<Artifact> objectFiles = ImmutableList.builder();
objectFiles.addAll(ccCompilationOutputs.getObjectFiles(false));
for (LibraryToLink library :
depsCcLinkingInfo.getCcLinkParams(isStaticMode, forDynamicLibrary).getLibraries()) {
if (library.containsObjectFiles()
&& library.getArtifactCategory() != ArtifactCategory.DYNAMIC_LIBRARY
&& library.getArtifactCategory() != ArtifactCategory.INTERFACE_LIBRARY) {
objectFiles.addAll(library.getObjectFiles());

for (LibraryToLinkWrapper library : depsCcLinkingContext.getLibraries()) {
if (isStaticMode
|| (library.getDynamicLibrary() == null && library.getInterfaceLibrary() == null)) {
if (library.getPicStaticLibrary() != null) {
if (library.getPicObjectFiles() != null) {
objectFiles.addAll(library.getPicObjectFiles());
}
} else if (library.getStaticLibrary() != null) {
if (library.getObjectFiles() != null) {
objectFiles.addAll(library.getObjectFiles());
}
}
}
}

generatedDefFile =
CppHelper.createDefFileActions(
ruleContext,
Expand All @@ -395,13 +403,12 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
pdbFile = ruleContext.getRelatedArtifact(binary.getRootRelativePath(), ".pdb");
}

NestedSetBuilder<LibraryToLink> extraLinkTimeLibrariesNestedSet = NestedSetBuilder.linkOrder();
NestedSetBuilder<LibraryToLinkWrapper> extraLinkTimeLibrariesNestedSet =
NestedSetBuilder.linkOrder();
NestedSetBuilder<Artifact> extraLinkTimeRuntimeLibraries = NestedSetBuilder.linkOrder();

ExtraLinkTimeLibraries extraLinkTimeLibraries =
depsCcLinkingInfo
.getCcLinkParams(isStaticMode, forDynamicLibrary)
.getExtraLinkTimeLibraries();
depsCcLinkingContext.getExtraLinkTimeLibraries();
if (extraLinkTimeLibraries != null) {
ExtraLinkTimeLibrary.BuildLibraryOutput extraLinkBuildLibraryOutput =
extraLinkTimeLibraries.buildLibraries(
Expand All @@ -425,7 +432,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
ccCompilationContext,
fake,
binary,
depsCcLinkingInfo,
depsCcLinkingContext,
extraLinkTimeLibrariesNestedSet.build(),
linkCompileOutputSeparately,
semantics,
Expand Down Expand Up @@ -501,14 +508,23 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
// If the binary is linked dynamically and COPY_DYNAMIC_LIBRARIES_TO_BINARY is enabled, collect
// all the dynamic libraries we need at runtime. Then copy these libraries next to the binary.
if (featureConfiguration.isEnabled(CppRuleClasses.COPY_DYNAMIC_LIBRARIES_TO_BINARY)) {
ImmutableList.Builder<Artifact> runtimeLibraries = ImmutableList.builder();
for (LibraryToLinkWrapper libraryToLinkWrapper : depsCcLinkingContext.getLibraries()) {
Artifact library =
libraryToLinkWrapper.getDynamicLibraryForRuntimeOrNull(
/* linkingStatically= */ isStaticMode);
if (library != null) {
runtimeLibraries.add(library);
}
}
filesToBuild =
NestedSetBuilder.fromNestedSet(filesToBuild)
.addAll(
createDynamicLibrariesCopyActions(
ruleContext,
depsCcLinkingInfo
.getCcLinkParams(isStaticMode, forDynamicLibrary)
.getDynamicLibrariesForRuntime()))
NestedSetBuilder.<Artifact>linkOrder()
.addAll(runtimeLibraries.build())
.build()))
.build();
}

Expand Down Expand Up @@ -598,8 +614,8 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi
CcCompilationContext ccCompilationContext,
boolean fake,
Artifact binary,
CcLinkingInfo depsCcLinkingInfo,
NestedSet<LibraryToLink> extraLinkTimeLibraries,
CcLinkingContext depsCcLinkingContext,
NestedSet<LibraryToLinkWrapper> extraLinkTimeLibraries,
boolean linkCompileOutputSeparately,
CppSemantics cppSemantics,
LinkingMode linkingMode,
Expand All @@ -624,78 +640,108 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi
fdoContext,
ruleContext.getConfiguration());

CcLinkParams.Builder ccLinkParamsBuilder = CcLinkParams.builder();
ccLinkParamsBuilder.addTransitiveArgs(
depsCcLinkingInfo.getCcLinkParams(
linkingMode != LinkingMode.DYNAMIC, isLinkShared(ruleContext)));
CcInfo depsCcInfo = CcInfo.builder().setCcLinkingContext(depsCcLinkingContext).build();

CcLinkingContext.Builder currentCcLinkingContextBuilder = CcLinkingContext.builder();

if (linkCompileOutputSeparately) {
ImmutableList.Builder<LibraryToLinkWrapper> localLibraries = ImmutableList.builder();
for (LibraryToLink library : ccLinkingOutputs.getDynamicLibrariesForLinking()) {
ccLinkParamsBuilder.addLibrary(library);
LibraryToLinkWrapper.Builder libraryToLinkWrapperBuilder = LibraryToLinkWrapper.builder();
libraryToLinkWrapperBuilder.setLibraryIdentifier(
LibraryToLinkWrapper.setDynamicArtifactsAndReturnIdentifier(
libraryToLinkWrapperBuilder,
library,
library,
/* runtimeLibraryIterator= */ ImmutableList.<Artifact>of().listIterator()));
localLibraries.add(libraryToLinkWrapperBuilder.build());
}
currentCcLinkingContextBuilder.addLibraries(
NestedSetBuilder.<LibraryToLinkWrapper>linkOrder()
.addAll(localLibraries.build())
.build());
ccCompilationOutputsWithOnlyObjects = new CcCompilationOutputs.Builder().build();
}

// Determine the libraries to link in.
// First libraries from srcs. Shared library artifacts here are substituted with mangled symlink
// artifacts generated by getDynamicLibraryLink(). This is done to minimize number of -rpath
// entries during linking process.
ImmutableList.Builder<LibraryToLinkWrapper> precompiledLibraries = ImmutableList.builder();
for (Artifact library : precompiledFiles.getLibraries()) {
if (Link.SHARED_LIBRARY_FILETYPES.matches(library.getFilename())) {
ccLinkParamsBuilder.addLibrary(
LinkerInputs.solibLibraryToLink(
common.getDynamicLibrarySymlink(library, true),
library,
CcLinkingOutputs.libraryIdentifierOf(library)));
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setDynamicLibrary(
common.getDynamicLibrarySymlink(library, /* preserveName= */ true))
.setResolvedSymlinkDynamicLibrary(library)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
} else if (Link.LINK_LIBRARY_FILETYPES.matches(library.getFilename())) {
ccLinkParamsBuilder.addLibrary(
LinkerInputs.precompiledLibraryToLink(
library, ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY));
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setStaticLibrary(library)
.setAlwayslink(true)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
} else if (Link.ARCHIVE_FILETYPES.matches(library.getFilename())) {
ccLinkParamsBuilder.addLibrary(
LinkerInputs.precompiledLibraryToLink(library, ArtifactCategory.STATIC_LIBRARY));
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setStaticLibrary(library)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
} else {
throw new IllegalStateException();
}
}
currentCcLinkingContextBuilder.addLibraries(
NestedSetBuilder.wrap(Order.LINK_ORDER, precompiledLibraries.build()));

ImmutableList.Builder<String> userLinkflags = ImmutableList.builder();
if (linkingMode != Link.LinkingMode.DYNAMIC
&& !cppConfiguration.disableEmittingStaticLibgcc()) {
// Only force a static link of libgcc if static runtime linking is enabled (which
// can't be true if runtimeInputs is empty).
// TODO(bazel-team): Move this to CcToolchain.
if (!ccToolchain.getStaticRuntimeLinkInputs(ruleContext, featureConfiguration).isEmpty()) {
ccLinkParamsBuilder.addLinkOpts(ImmutableList.of("-static-libgcc"));
userLinkflags.add("-static-libgcc");
}
}

ccLinkParamsBuilder
.addNonCodeInputs(ccCompilationContext.getTransitiveCompilationPrerequisites())
.addNonCodeInputs(common.getLinkerScripts())
.addLinkOpts(common.getLinkopts());

CcLinkParams ccLinkParamsForLauncher = ccLinkParamsBuilder.build();
CcLinkParams ccLinkParams =
CcLinkParams.builder()
.addTransitiveArgs(ccLinkParamsForLauncher)
.addLibraries(extraLinkTimeLibraries)
.addTransitiveArgs(CcLinkParams.builder().addLibraries(extraLinkTimeLibraries).build())
userLinkflags.addAll(common.getLinkopts());
currentCcLinkingContextBuilder
.addNonCodeInputs(
NestedSetBuilder.<Artifact>linkOrder()
.addAll(ccCompilationContext.getTransitiveCompilationPrerequisites())
.addAll(common.getLinkerScripts())
.build())
.addUserLinkFlags(
NestedSetBuilder.<LinkOptions>linkOrder()
.add(new LinkOptions(userLinkflags.build()))
.build());

CcInfo ccInfoWithoutExtraLinkTimeLibraries =
CcInfo.merge(
ImmutableList.of(
CcInfo.builder()
.setCcLinkingContext(currentCcLinkingContextBuilder.build())
.build(),
depsCcInfo));

CcInfo extraLinkTimeLibrariesCcInfo =
CcInfo.builder()
.setCcLinkingContext(
CcLinkingContext.builder().addLibraries(extraLinkTimeLibraries).build())
.build();

CcLinkingInfo.Builder ccLinkingInfo = CcLinkingInfo.Builder.create();
ccLinkingInfo.setStaticModeParamsForDynamicLibrary(ccLinkParams);
ccLinkingInfo.setStaticModeParamsForExecutable(ccLinkParams);
ccLinkingInfo.setDynamicModeParamsForDynamicLibrary(ccLinkParams);
ccLinkingInfo.setDynamicModeParamsForExecutable(ccLinkParams);

CcLinkingInfo.Builder ccLinkingInfoForLauncher = CcLinkingInfo.Builder.create();
ccLinkingInfoForLauncher.setStaticModeParamsForDynamicLibrary(ccLinkParamsForLauncher);
ccLinkingInfoForLauncher.setStaticModeParamsForExecutable(ccLinkParamsForLauncher);
ccLinkingInfoForLauncher.setDynamicModeParamsForDynamicLibrary(ccLinkParamsForLauncher);
ccLinkingInfoForLauncher.setDynamicModeParamsForExecutable(ccLinkParamsForLauncher);
CcInfo ccInfo =
CcInfo.merge(
ImmutableList.of(ccInfoWithoutExtraLinkTimeLibraries, extraLinkTimeLibrariesCcInfo));

ccLinkingHelper
.addCcLinkingInfos(ImmutableList.of(ccLinkingInfo.build()))
.addCcLinkingContexts(ImmutableList.of(ccInfo.getCcLinkingContext()))
.setUseTestOnlyFlags(ruleContext.isTestTarget())
.setShouldCreateStaticLibraries(false)
.setLinkingMode(linkingMode)
Expand All @@ -718,7 +764,8 @@ public static Pair<CcLinkingOutputs, CcLauncherInfo> createTransitiveLinkingActi

return Pair.of(
ccLinkingHelper.link(ccCompilationOutputsWithOnlyObjects),
new CcLauncherInfo(ccLinkingInfoForLauncher.build(), ccCompilationOutputsWithOnlyObjects));
new CcLauncherInfo(
ccInfoWithoutExtraLinkTimeLibraries, ccCompilationOutputsWithOnlyObjects));
}

/**
Expand Down Expand Up @@ -938,7 +985,7 @@ private static Artifact getIntermediateDwpFile(RuleContext ruleContext, Artifact
}

/** Collect link parameters from the transitive closure. */
private static CcLinkingInfo collectCcLinkingInfo(RuleContext context) {
private static CcLinkingContext collectCcLinkingContext(RuleContext context) {
ImmutableList.Builder<CcInfo> ccInfoListBuilder = ImmutableList.builder();

ccInfoListBuilder.addAll(context.getPrerequisites("deps", Mode.TARGET, CcInfo.PROVIDER));
Expand All @@ -948,7 +995,7 @@ private static CcLinkingInfo collectCcLinkingInfo(RuleContext context) {
ccInfoListBuilder.add(ccInfo);
}
}
return CcInfo.merge(ccInfoListBuilder.build()).getCcLinkingInfo();
return CcInfo.merge(ccInfoListBuilder.build()).getCcLinkingContext();
}

private static void addTransitiveInfoProviders(
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,13 @@ public CcLinkingHelper addCcLinkingInfos(Iterable<CcLinkingInfo> ccLinkingInfos)
return this;
}

public CcLinkingHelper addCcLinkingContexts(Iterable<CcLinkingContext> ccLinkingContexts) {
for (CcLinkingContext ccLinkingContext : ccLinkingContexts) {
ccLinkingInfos.add(ccLinkingContext.toCcLinkingInfo());
}
return this;
}

/**
* Adds the given linkstamps. Note that linkstamps are usually not compiled at the library level,
* but only in the dependent binary rules.
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/ExtraLinkTimeLibraries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.ExtraLinkTimeLibrary.BuildLibraryOutput;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import java.util.Collection;
Expand Down Expand Up @@ -107,7 +106,7 @@ public final Builder add(ExtraLinkTimeLibrary b) {
public BuildLibraryOutput buildLibraries(
RuleContext ruleContext, boolean staticMode, boolean forDynamicLibrary)
throws InterruptedException, RuleErrorException {
NestedSetBuilder<LibraryToLink> librariesToLink = NestedSetBuilder.linkOrder();
NestedSetBuilder<LibraryToLinkWrapper> librariesToLink = NestedSetBuilder.linkOrder();
NestedSetBuilder<Artifact> runtimeLibraries = NestedSetBuilder.linkOrder();
for (ExtraLinkTimeLibrary extraLibrary : getExtraLibraries()) {
BuildLibraryOutput buildLibraryOutput =
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/ExtraLinkTimeLibrary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;

/**
* An extra library to include in a link. The actual library is built at link time.
Expand All @@ -34,16 +33,16 @@ public interface ExtraLinkTimeLibrary {

/** Output of {@link #buildLibraries}. Pair of libraries to link and runtime libraries. */
class BuildLibraryOutput {
public NestedSet<LibraryToLink> librariesToLink;
public NestedSet<LibraryToLinkWrapper> librariesToLink;
public NestedSet<Artifact> runtimeLibraries;

public BuildLibraryOutput(
NestedSet<LibraryToLink> librariesToLink, NestedSet<Artifact> runtimeLibraries) {
NestedSet<LibraryToLinkWrapper> librariesToLink, NestedSet<Artifact> runtimeLibraries) {
this.librariesToLink = librariesToLink;
this.runtimeLibraries = runtimeLibraries;
}

public NestedSet<LibraryToLink> getLibrariesToLink() {
public NestedSet<LibraryToLinkWrapper> getLibrariesToLink() {
return librariesToLink;
}

Expand Down
Loading

0 comments on commit 56f2b2b

Please sign in to comment.