Skip to content

Commit

Permalink
Automated rollback of commit 56f2b2b.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

b/123103413.

*** Original change description ***

C++: Remove more CcLinkParams

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: 230094550
  • Loading branch information
@janakdr @weixiao-huang
janakdr authored and weixiao-huang committed Jan 31, 2019
1 parent ca6164a commit e59a527
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 178 deletions.
181 changes: 67 additions & 114 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,11 +56,9 @@
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 @@ -114,11 +112,12 @@ public static class CcLauncherInfo extends NativeInfo {
public static final Provider PROVIDER = new Provider();

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

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

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

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

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

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

CcLinkingContext depsCcLinkingContext = collectCcLinkingContext(ruleContext);
CcLinkingInfo depsCcLinkingInfo = collectCcLinkingInfo(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 (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());
}
}
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());
}
}

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

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

ExtraLinkTimeLibraries extraLinkTimeLibraries =
depsCcLinkingContext.getExtraLinkTimeLibraries();
depsCcLinkingInfo
.getCcLinkParams(isStaticMode, forDynamicLibrary)
.getExtraLinkTimeLibraries();
if (extraLinkTimeLibraries != null) {
ExtraLinkTimeLibrary.BuildLibraryOutput extraLinkBuildLibraryOutput =
extraLinkTimeLibraries.buildLibraries(
Expand All @@ -432,7 +425,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
ccCompilationContext,
fake,
binary,
depsCcLinkingContext,
depsCcLinkingInfo,
extraLinkTimeLibrariesNestedSet.build(),
linkCompileOutputSeparately,
semantics,
Expand Down Expand Up @@ -508,23 +501,14 @@ 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,
NestedSetBuilder.<Artifact>linkOrder()
.addAll(runtimeLibraries.build())
.build()))
depsCcLinkingInfo
.getCcLinkParams(isStaticMode, forDynamicLibrary)
.getDynamicLibrariesForRuntime()))
.build();
}

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

CcInfo depsCcInfo = CcInfo.builder().setCcLinkingContext(depsCcLinkingContext).build();

CcLinkingContext.Builder currentCcLinkingContextBuilder = CcLinkingContext.builder();
CcLinkParams.Builder ccLinkParamsBuilder = CcLinkParams.builder();
ccLinkParamsBuilder.addTransitiveArgs(
depsCcLinkingInfo.getCcLinkParams(
linkingMode != LinkingMode.DYNAMIC, isLinkShared(ruleContext)));

if (linkCompileOutputSeparately) {
ImmutableList.Builder<LibraryToLinkWrapper> localLibraries = ImmutableList.builder();
for (LibraryToLink library : ccLinkingOutputs.getDynamicLibrariesForLinking()) {
LibraryToLinkWrapper.Builder libraryToLinkWrapperBuilder = LibraryToLinkWrapper.builder();
libraryToLinkWrapperBuilder.setLibraryIdentifier(
LibraryToLinkWrapper.setDynamicArtifactsAndReturnIdentifier(
libraryToLinkWrapperBuilder,
library,
library,
/* runtimeLibraryIterator= */ ImmutableList.<Artifact>of().listIterator()));
localLibraries.add(libraryToLinkWrapperBuilder.build());
ccLinkParamsBuilder.addLibrary(library);
}
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())) {
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setDynamicLibrary(
common.getDynamicLibrarySymlink(library, /* preserveName= */ true))
.setResolvedSymlinkDynamicLibrary(library)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
ccLinkParamsBuilder.addLibrary(
LinkerInputs.solibLibraryToLink(
common.getDynamicLibrarySymlink(library, true),
library,
CcLinkingOutputs.libraryIdentifierOf(library)));
} else if (Link.LINK_LIBRARY_FILETYPES.matches(library.getFilename())) {
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setStaticLibrary(library)
.setAlwayslink(true)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
ccLinkParamsBuilder.addLibrary(
LinkerInputs.precompiledLibraryToLink(
library, ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY));
} else if (Link.ARCHIVE_FILETYPES.matches(library.getFilename())) {
LibraryToLinkWrapper libraryToLinkWrapper =
LibraryToLinkWrapper.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
.setStaticLibrary(library)
.build();
precompiledLibraries.add(libraryToLinkWrapper);
ccLinkParamsBuilder.addLibrary(
LinkerInputs.precompiledLibraryToLink(library, ArtifactCategory.STATIC_LIBRARY));
} 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()) {
userLinkflags.add("-static-libgcc");
ccLinkParamsBuilder.addLinkOpts(ImmutableList.of("-static-libgcc"));
}
}

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())
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())
.build();
CcInfo ccInfo =
CcInfo.merge(
ImmutableList.of(ccInfoWithoutExtraLinkTimeLibraries, extraLinkTimeLibrariesCcInfo));

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);

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

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

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

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

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

private static void addTransitiveInfoProviders(
Expand Down
7 changes: 0 additions & 7 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,13 +211,6 @@ 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: 2 additions & 1 deletion 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,6 +20,7 @@
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 @@ -106,7 +107,7 @@ public final Builder add(ExtraLinkTimeLibrary b) {
public BuildLibraryOutput buildLibraries(
RuleContext ruleContext, boolean staticMode, boolean forDynamicLibrary)
throws InterruptedException, RuleErrorException {
NestedSetBuilder<LibraryToLinkWrapper> librariesToLink = NestedSetBuilder.linkOrder();
NestedSetBuilder<LibraryToLink> librariesToLink = NestedSetBuilder.linkOrder();
NestedSetBuilder<Artifact> runtimeLibraries = NestedSetBuilder.linkOrder();
for (ExtraLinkTimeLibrary extraLibrary : getExtraLibraries()) {
BuildLibraryOutput buildLibraryOutput =
Expand Down
7 changes: 4 additions & 3 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,6 +18,7 @@
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 @@ -33,16 +34,16 @@ public interface ExtraLinkTimeLibrary {

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

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

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

Expand Down
Loading

0 comments on commit e59a527

Please sign in to comment.