Skip to content

Commit

Permalink
Automated rollback of commit 56f2b2bbae39d432609afa210db97d6c5669319a.
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/bazel#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
Luca Di Grazia committed Sep 4, 2022
1 parent 4de0386 commit de778af
Show file tree
Hide file tree
Showing 5 changed files with 670 additions and 297 deletions.
11 changes: 6 additions & 5 deletions ...Java/bazelbuild.bazel/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 @@ -322,11 +322,12 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
// can also be enabled specifically for tests with an experimental flag.
// TODO(meikeb): Retire the experimental flag by end of 2018.
boolean linkCompileOutputSeparately =
ruleContext.isTestTarget()
&& linkingMode == LinkingMode.DYNAMIC
&& cppConfiguration.getDynamicModeFlag() == DynamicMode.DEFAULT
&& (cppConfiguration.getLinkCompileOutputSeparately()
|| ruleContext.getDisabledFeatures().contains(STATIC_LINK_TEST_SRCS));
(ruleContext.isTestTarget()
&& cppConfiguration.getLinkCompileOutputSeparately()
&& linkingMode == LinkingMode.DYNAMIC)
|| (ruleContext.isTestTarget()
&& cppConfiguration.getDynamicModeFlag() == DynamicMode.DEFAULT
&& ruleContext.getDisabledFeatures().contains(STATIC_LINK_TEST_SRCS));
// When linking the object files directly into the resulting binary, we do not need
// library-level link outputs; thus, we do not let CcCompilationHelper produce link outputs
// (either shared object files or archives) for a non-library link type [*], and add
Expand Down
186 changes: 84 additions & 102 deletions ...zelbuild.bazel/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 @@ -32,17 +32,19 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams.LinkOptions;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp;
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.CcToolchainVariables.VariablesExtension;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper.CcLinkingContext.Linkstamp;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.Link.LinkerOrArchiver;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.rules.cpp.Link.Picness;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.skylarkbuildapi.cpp.LinkingInfoApi;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
Expand All @@ -64,17 +66,17 @@ public final class CcLinkingHelper {
// TODO(plf): Only used by Skylark API. Remove after migrating.
@Deprecated
public static final class LinkingInfo implements LinkingInfoApi {
private final CcLinkingContext ccLinkingContext;
private final CcLinkingInfo ccLinkingInfo;
private final CcLinkingOutputs linkingOutputs;

public LinkingInfo(CcLinkingContext ccLinkingContext, CcLinkingOutputs linkingOutputs) {
this.ccLinkingContext = ccLinkingContext;
public LinkingInfo(CcLinkingInfo ccLinkingInfo, CcLinkingOutputs linkingOutputs) {
this.ccLinkingInfo = ccLinkingInfo;
this.linkingOutputs = linkingOutputs;
}

@Override
public CcLinkingContext getCcLinkingContext() {
return ccLinkingContext;
public CcLinkingInfo getCcLinkingInfo() {
return ccLinkingInfo;
}

@Override
Expand All @@ -91,7 +93,7 @@ public CcLinkingOutputs getCcLinkingOutputs() {
private final List<Artifact> nonCodeLinkerInputs = new ArrayList<>();
private final List<String> linkopts = new ArrayList<>();
private final List<TransitiveInfoCollection> deps = new ArrayList<>();
private final List<CcLinkingContext> ccLinkingContexts = new ArrayList<>();
private final List<CcLinkingInfo> ccLinkingInfos = new ArrayList<>();
private final NestedSetBuilder<Artifact> linkstamps = NestedSetBuilder.stableOrder();
private final List<Artifact> linkActionInputs = new ArrayList<>();

Expand Down Expand Up @@ -192,20 +194,20 @@ public CcLinkingHelper addLinkopts(Iterable<String> linkopts) {
* (like from a "deps" attribute) and also implicit dependencies on runtime libraries.
*/
public CcLinkingHelper addDeps(Iterable<? extends TransitiveInfoCollection> deps) {
this.ccLinkingContexts.addAll(
this.ccLinkingInfos.addAll(
Streams.stream(AnalysisUtils.getProviders(deps, CcInfo.PROVIDER))
.map(CcInfo::getCcLinkingContext)
.map(CcInfo::getCcLinkingInfo)
.collect(ImmutableList.toImmutableList()));
Iterables.addAll(this.deps, deps);
return this;
}

/**
* Adds additional {@link CcLinkingContext} that will be used everywhere where CcLinkingInfos were
* Adds additional {@link CcLinkingInfo} that will be used everywhere where CcLinkingInfos were
* obtained from deps.
*/
public CcLinkingHelper addCcLinkingContexts(Iterable<CcLinkingContext> ccLinkingContexts) {
Iterables.addAll(this.ccLinkingContexts, ccLinkingContexts);
public CcLinkingHelper addCcLinkingInfos(Iterable<CcLinkingInfo> ccLinkingInfos) {
Iterables.addAll(this.ccLinkingInfos, ccLinkingInfos);
return this;
}

Expand Down Expand Up @@ -357,37 +359,32 @@ public CcLinkingOutputs link(CcCompilationOutputs ccOutputs)
return ccLinkingOutputs;
}

public CcLinkingContext buildCcLinkingContextFromLibraryToLinkWrappers(
public CcLinkingInfo buildCcLinkingInfoFromLibraryToLinkWrappers(
ImmutableCollection<LibraryToLinkWrapper> libraryToLinkWrappers,
CcCompilationContext ccCompilationContext) {
NestedSetBuilder<Linkstamp> linkstampBuilder = NestedSetBuilder.stableOrder();
for (Artifact linkstamp : linkstamps.build()) {
linkstampBuilder.add(new Linkstamp(linkstamp, ccCompilationContext.getDeclaredIncludeSrcs()));
}
CcLinkingContext ccLinkingContext = CcLinkingContext.EMPTY;
CcLinkingInfo ccLinkingInfo = CcLinkingInfo.EMPTY;
if (!neverlink) {
// We want an empty set if there are no link options. We have to make sure we don't
// create a LinkOptions instance that contains an empty list.
ccLinkingContext =
CcLinkingContext.builder()
.addUserLinkFlags(
linkopts.isEmpty()
? NestedSetBuilder.emptySet(Order.LINK_ORDER)
: NestedSetBuilder.create(
Order.LINK_ORDER, CcLinkingContext.LinkOptions.of(linkopts)))
.addLibraries(
NestedSetBuilder.<LibraryToLinkWrapper>linkOrder()
.addAll(libraryToLinkWrappers)
.build())
.addNonCodeInputs(
NestedSetBuilder.<Artifact>linkOrder().addAll(nonCodeLinkerInputs).build())
.addLinkstamps(linkstampBuilder.build())
.build();
ccLinkingInfo =
LibraryToLinkWrapper.toCcLinkingInfo(
cppConfiguration.forcePic(),
libraryToLinkWrappers,
// We want an empty set if there are no link options. We have to make sure we don't
// create a LinkOptions instance that contains an empty list.
linkopts.isEmpty()
? NestedSetBuilder.emptySet(Order.LINK_ORDER)
: NestedSetBuilder.create(Order.LINK_ORDER, LinkOptions.of(linkopts)),
linkstampBuilder.build(),
nonCodeLinkerInputs,
/* extraLinkTimeLibraries= */ null);
}
ImmutableList.Builder<CcLinkingContext> mergedCcLinkingContexts = ImmutableList.builder();
mergedCcLinkingContexts.add(ccLinkingContext);
mergedCcLinkingContexts.addAll(ccLinkingContexts);
return CcLinkingContext.merge(mergedCcLinkingContexts.build());
ImmutableList.Builder<CcLinkingInfo> mergedCcLinkingInfos = ImmutableList.builder();
mergedCcLinkingInfos.add(ccLinkingInfo);
mergedCcLinkingInfos.addAll(ccLinkingInfos);
return CcLinkingInfo.merge(mergedCcLinkingInfos.build());
}

/**
Expand All @@ -412,7 +409,7 @@ private CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs)
staticLinkType.linkerOrArchiver() == LinkerOrArchiver.ARCHIVER,
"can only handle static links");

LibraryToLinkWrapper.Builder libraryToLinkBuilder = LibraryToLinkWrapper.builder();
CcLinkingOutputs.Builder result = new CcLinkingOutputs.Builder();
AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
boolean usePicForBinaries =
CppHelper.usePicForBinaries(ruleContext, ccToolchain, featureConfiguration);
Expand All @@ -426,36 +423,30 @@ private CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs)
.getPathString();

if (shouldCreateStaticLibraries) {
createNoPicAndPicStaticLibraries(
libraryToLinkBuilder,
env,
usePicForBinaries,
usePicForDynamicLibs,
libraryIdentifier,
ccOutputs);
Pair<LibraryToLink, LibraryToLink> staticLibrariesToLink =
createNoPicAndPicStaticLibraries(
env, usePicForBinaries, usePicForDynamicLibs, libraryIdentifier, ccOutputs);
if (staticLibrariesToLink.first != null) {
result.addStaticLibrary(staticLibrariesToLink.first);
}
if (staticLibrariesToLink.second != null) {
result.addPicStaticLibrary(staticLibrariesToLink.second);
}
}

boolean hasBuiltDynamicLibrary = false;
CcLinkingOutputs.Builder ccLinkingOutputsBuilder = CcLinkingOutputs.builder();
if (shouldCreateDynamicLibrary) {
boolean usePic =
(!dynamicLinkType.isExecutable() && usePicForDynamicLibs)
|| (dynamicLinkType.isExecutable() && usePicForBinaries);
hasBuiltDynamicLibrary =
createDynamicLibrary(
ccLinkingOutputsBuilder,
libraryToLinkBuilder,
env,
usePic,
libraryIdentifier,
ccOutputs);
}

if (hasBuiltDynamicLibrary || shouldCreateStaticLibraries) {
ccLinkingOutputsBuilder.setLibraryToLink(libraryToLinkBuilder.build());
createDynamicLibrary(result, env, usePic, libraryIdentifier, ccOutputs);
}

return ccLinkingOutputsBuilder.build();
CcLinkingOutputs ccLinkingOutputs = result.build();
Preconditions.checkState(ccLinkingOutputs.getStaticLibraries().size() <= 1);
Preconditions.checkState(ccLinkingOutputs.getPicStaticLibraries().size() <= 1);
Preconditions.checkState(ccLinkingOutputs.getDynamicLibrariesForLinking().size() <= 1);
Preconditions.checkState(ccLinkingOutputs.getDynamicLibrariesForRuntime().size() <= 1);
return ccLinkingOutputs;
}

public CcLinkingHelper setWillOnlyBeLinkedIntoDynamicLibraries(
Expand Down Expand Up @@ -494,14 +485,15 @@ public CcLinkingHelper setDefFile(Artifact defFile) {
return this;
}

private void createNoPicAndPicStaticLibraries(
LibraryToLinkWrapper.Builder libraryToLinkBuilder,
private Pair<LibraryToLink, LibraryToLink> createNoPicAndPicStaticLibraries(
AnalysisEnvironment env,
boolean usePicForBinaries,
boolean usePicForDynamicLibs,
String libraryIdentifier,
CcCompilationOutputs ccOutputs)
throws RuleErrorException, InterruptedException {
LibraryToLink staticLibrary = null;
LibraryToLink picStaticLibrary = null;
// Create static library (.a). The staticLinkType only reflects whether the library is
// alwayslink or not. The PIC-ness is determined by whether we need to use PIC or not. There
// are four cases:
Expand Down Expand Up @@ -533,18 +525,10 @@ private void createNoPicAndPicStaticLibraries(
}

if (createNoPicAction) {
LibraryToLink staticLibrary =
staticLibrary =
registerActionForStaticLibrary(
staticLinkType, ccOutputs, /* usePic= */ false, libraryIdentifier, env)
.getOutputLibrary();
libraryToLinkBuilder
.setLibraryIdentifier(staticLibrary.getLibraryIdentifier())
.setStaticLibrary(staticLibrary.getArtifact())
.setObjectFiles(ImmutableList.copyOf(staticLibrary.getObjectFiles()))
.setLtoCompilationContext(staticLibrary.getLtoCompilationContext())
.setSharedNonLtoBackends(staticLibrary.getSharedNonLtoBackends())
.setAlwayslink(
staticLibrary.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY);
}

if (createPicAction) {
Expand All @@ -558,23 +542,16 @@ private void createNoPicAndPicStaticLibraries(
? LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY
: LinkTargetType.PIC_STATIC_LIBRARY;
}
LibraryToLink picStaticLibrary =
picStaticLibrary =
registerActionForStaticLibrary(
linkTargetTypeUsedForNaming,
ccOutputs,
/* usePic= */ true,
libraryIdentifier,
env)
.getOutputLibrary();
libraryToLinkBuilder
.setLibraryIdentifier(picStaticLibrary.getLibraryIdentifier())
.setPicStaticLibrary(picStaticLibrary.getArtifact())
.setPicObjectFiles(ImmutableList.copyOf(picStaticLibrary.getObjectFiles()))
.setPicLtoCompilationContext(picStaticLibrary.getLtoCompilationContext())
.setPicSharedNonLtoBackends(picStaticLibrary.getSharedNonLtoBackends())
.setAlwayslink(
picStaticLibrary.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY);
}
return new Pair<>(staticLibrary, picStaticLibrary);
}

private CppLinkAction registerActionForStaticLibrary(
Expand All @@ -601,15 +578,13 @@ private CppLinkAction registerActionForStaticLibrary(
return action;
}

private boolean createDynamicLibrary(
CcLinkingOutputs.Builder ccLinkingOutputs,
LibraryToLinkWrapper.Builder libraryToLinkBuilder,
private void createDynamicLibrary(
CcLinkingOutputs.Builder result,
AnalysisEnvironment env,
boolean usePic,
String libraryIdentifier,
CcCompilationOutputs ccOutputs)
throws RuleErrorException, InterruptedException {
boolean hasBuiltDynamicLibrary = false;
// Create dynamic library.
Artifact soImpl;
String mainLibraryIdentifier;
Expand Down Expand Up @@ -689,12 +664,16 @@ private boolean createDynamicLibrary(
|| dynamicLinkType != LinkTargetType.NODEPS_DYNAMIC_LIBRARY;

if (shouldLinkTransitively) {
CcLinkingContext ccLinkingContext = CcLinkingContext.merge(ccLinkingContexts);
CcLinkingInfo mergedCcLinkingInfo = CcLinkingInfo.merge(ccLinkingInfos);
CcLinkingContext ccLinkingContext =
LibraryToLinkWrapper.fromCcLinkingInfo(mergedCcLinkingInfo);
List<LibraryToLink> libraries =
LibraryToLinkWrapper.convertLibraryToLinkWrapperListToLibraryToLinkList(
ccLinkingContext.getLibraries(),
linkingMode != LinkingMode.DYNAMIC,
dynamicLinkType.isDynamicLibrary());
ccLinkingContext
.toCcLinkingInfo()
.getCcLinkParams(
linkingMode != LinkingMode.DYNAMIC, dynamicLinkType.isDynamicLibrary())
.getLibraries()
.toList();
dynamicLinkActionBuilder.addLinkParams(
libraries,
ccLinkingContext.getFlattenedUserLinkFlags(),
Expand Down Expand Up @@ -724,10 +703,10 @@ private boolean createDynamicLibrary(
}

if (dynamicLinkActionBuilder.getAllLtoBackendArtifacts() != null) {
ccLinkingOutputs.addAllLtoArtifacts(dynamicLinkActionBuilder.getAllLtoBackendArtifacts());
result.addAllLtoArtifacts(dynamicLinkActionBuilder.getAllLtoBackendArtifacts());
}
CppLinkAction dynamicLinkAction = dynamicLinkActionBuilder.build();
ccLinkingOutputs.addLinkActionInputs(dynamicLinkAction.getInputs());
result.addLinkActionInputs(dynamicLinkAction.getInputs());
env.registerAction(dynamicLinkAction);

LibraryToLink dynamicLibrary = dynamicLinkAction.getOutputLibrary();
Expand All @@ -740,14 +719,11 @@ private boolean createDynamicLibrary(
// When COPY_DYNAMIC_LIBRARIES_TO_BINARY is enabled, we don't need to create the special
// solibDir, instead we use the original interface library and dynamic library.
if (dynamicLibrary != null) {
hasBuiltDynamicLibrary = true;
libraryToLinkBuilder.setLibraryIdentifier(dynamicLibrary.getLibraryIdentifier());
if (neverlink
|| featureConfiguration.isEnabled(CppRuleClasses.COPY_DYNAMIC_LIBRARIES_TO_BINARY)) {
if (interfaceLibrary != null) {
libraryToLinkBuilder.setInterfaceLibrary(interfaceLibrary.getArtifact());
}
libraryToLinkBuilder.setDynamicLibrary(dynamicLibrary.getArtifact());
result.addDynamicLibraryForLinking(
interfaceLibrary == null ? dynamicLibrary : interfaceLibrary);
result.addDynamicLibraryForRuntime(dynamicLibrary);
} else {
Artifact implLibraryLinkArtifact =
SolibSymlinkAction.getDynamicLibrarySymlink(
Expand All @@ -758,10 +734,15 @@ private boolean createDynamicLibrary(
/* preserveName= */ false,
/* prefixConsumer= */ false,
ruleContext.getConfiguration());
libraryToLinkBuilder.setDynamicLibrary(implLibraryLinkArtifact);
libraryToLinkBuilder.setResolvedSymlinkDynamicLibrary(dynamicLibrary.getArtifact());

if (interfaceLibrary != null) {
LibraryToLink implLibraryLink =
LinkerInputs.solibLibraryToLink(
implLibraryLinkArtifact, dynamicLibrary.getArtifact(), libraryIdentifier);
result.addDynamicLibraryForRuntime(implLibraryLink);

LibraryToLink libraryLink;
if (interfaceLibrary == null) {
libraryLink = implLibraryLink;
} else {
Artifact libraryLinkArtifact =
SolibSymlinkAction.getDynamicLibrarySymlink(
/* actionRegistry= */ ruleContext,
Expand All @@ -771,12 +752,13 @@ private boolean createDynamicLibrary(
/* preserveName= */ false,
/* prefixConsumer= */ false,
ruleContext.getConfiguration());
libraryToLinkBuilder.setInterfaceLibrary(libraryLinkArtifact);
libraryToLinkBuilder.setResolvedSymlinkInterfaceLibrary(interfaceLibrary.getArtifact());
libraryLink =
LinkerInputs.solibLibraryToLink(
libraryLinkArtifact, interfaceLibrary.getArtifact(), libraryIdentifier);
}
result.addDynamicLibraryForLinking(libraryLink);
}
}
return hasBuiltDynamicLibrary;
}

private CppLinkActionBuilder newLinkActionBuilder(Artifact outputArtifact) {
Expand Down
1 change: 1 addition & 0 deletions ...d.bazel/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
Loading

0 comments on commit de778af

Please sign in to comment.