Skip to content

Commit

Permalink
C++: Deletes CcLinkingInfo
Browse files Browse the repository at this point in the history
This is the last CL in the CcLinkParams nightmare. Instead of doing the last 3 or 4 incrementally. We have decided to do one big CL where it gets deleted. These CLs where there is back and forth conversion from the old and the new API are error prone and have been rolled back quite frequently. The errors are introduced in the conversion code. By completely removing CcLinkingInfo and CcLinkParams, these errors go away and the refactoring is finished.

All tests pass and there is no memory regression.
Tested with downstream projects:
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/767

RELNOTES:none
PiperOrigin-RevId: 230347403
  • Loading branch information
oquenchil authored and Copybara-Service committed Jan 22, 2019
1 parent e65f3a8 commit 3e727b1
Show file tree
Hide file tree
Showing 42 changed files with 739 additions and 1,764 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs;
import com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.LinkingInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.rules.cpp.CcModule;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
Expand All @@ -46,7 +45,6 @@ public class BazelCcModule extends CcModule
CcCompilationContext,
CcCompilationOutputs,
LinkingInfo,
CcLinkingInfo,
CcLinkingContext,
LibraryToLinkWrapper,
CcToolchainVariables> {
Expand Down Expand Up @@ -87,7 +85,7 @@ public LinkingInfo link(
CcCompilationOutputs ccCompilationOutputs,
Object skylarkLinkopts,
Object dynamicLibrary,
SkylarkList<CcLinkingInfo> skylarkCcLinkingInfos,
SkylarkList<CcLinkingContext> skylarkCcLinkingContexts,
boolean neverLink)
throws InterruptedException, EvalException {
return BazelCcModule.link(
Expand All @@ -99,7 +97,7 @@ public LinkingInfo link(
skylarkLinkopts,
/* shouldCreateStaticLibraries= */ true,
dynamicLibrary,
skylarkCcLinkingInfos,
skylarkCcLinkingContexts,
neverLink);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ public CcInfo buildCcInfoProvider(Iterable<? extends TransitiveInfoCollection> d
.collect(ImmutableList.toImmutableList()))
.build();

// TODO(plf): return empty CcLinkingInfo.
// TODO(plf): return empty CcInfo.
return CcInfo.merge(ccInfos);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public final class AndroidCcLinkParamsProvider extends NativeInfo

public AndroidCcLinkParamsProvider(CcInfo ccInfo) {
super(PROVIDER);
this.ccInfo = CcInfo.builder().setCcLinkingInfo(ccInfo.getCcLinkingInfo()).build();
this.ccInfo = CcInfo.builder().setCcLinkingContext(ccInfo.getCcLinkingContext()).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
import com.google.devtools.build.lib.rules.android.ZipFilterBuilder.CheckHashMismatchMode;
import com.google.devtools.build.lib.rules.android.databinding.DataBindingContext;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams.LinkOptions;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper.CcLinkingContext;
import com.google.devtools.build.lib.rules.java.ClasspathConfiguredFragment;
import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider;
import com.google.devtools.build.lib.rules.java.JavaCommon;
Expand Down Expand Up @@ -825,16 +825,13 @@ public CcInfo getCcInfo() {
public static CcInfo getCcInfo(
final Iterable<? extends TransitiveInfoCollection> deps, final Collection<String> linkOpts) {

CcLinkParams linkOptsParams = CcLinkParams.builder().addLinkOpts(linkOpts).build();
CcLinkingInfo linkOptsCcLinkingInfo =
CcLinkingInfo.Builder.create()
.setStaticModeParamsForDynamicLibrary(linkOptsParams)
.setStaticModeParamsForExecutable(linkOptsParams)
.setDynamicModeParamsForDynamicLibrary(linkOptsParams)
.setDynamicModeParamsForExecutable(linkOptsParams)
CcLinkingContext ccLinkingContext =
CcLinkingContext.builder()
.addUserLinkFlags(
NestedSetBuilder.<LinkOptions>linkOrder().add(LinkOptions.of(linkOpts)).build())
.build();

CcInfo linkoptsCcInfo = CcInfo.builder().setCcLinkingInfo(linkOptsCcLinkingInfo).build();
CcInfo linkoptsCcInfo = CcInfo.builder().setCcLinkingContext(ccLinkingContext).build();

ImmutableList<CcInfo> ccInfos =
ImmutableList.<CcInfo>builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.android;

import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -33,12 +34,11 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.cpp.CppSemantics;
import com.google.devtools.build.lib.rules.cpp.LinkerInput;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper;
import com.google.devtools.build.lib.rules.nativedeps.NativeDepsHelper;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -68,17 +68,15 @@ public static NativeLibs fromLinkedNativeDeps(
String nativeDepsLibraryBasename = null;
for (Map.Entry<String, Collection<TransitiveInfoCollection>> entry :
getSplitDepsByArchitecture(ruleContext, depsAttributes).asMap().entrySet()) {
CcLinkParams linkParams =
CcInfo ccInfo =
AndroidCommon.getCcInfo(
entry.getValue(),
ImmutableList.of("-Wl,-soname=lib" + ruleContext.getLabel().getName()))
.getCcLinkingInfo()
.getStaticModeParamsForDynamicLibrary();
entry.getValue(),
ImmutableList.of("-Wl,-soname=lib" + ruleContext.getLabel().getName()));

Artifact nativeDepsLibrary =
NativeDepsHelper.linkAndroidNativeDepsIfPresent(
ruleContext,
linkParams,
ccInfo,
configurationMap.get(entry.getKey()),
toolchainsByCpu.get(entry.getKey()),
cppSemantics);
Expand All @@ -89,7 +87,8 @@ public static NativeLibs fromLinkedNativeDeps(
nativeDepsLibraryBasename = nativeDepsLibrary.getExecPath().getBaseName();
}
librariesBuilder.addAll(
filterUniqueSharedLibraries(ruleContext, nativeDepsLibrary, linkParams.getLibraries()));
filterUniqueSharedLibraries(
ruleContext, nativeDepsLibrary, ccInfo.getCcLinkingContext().getLibraries()));
NestedSet<Artifact> libraries = librariesBuilder.build();

if (!libraries.isEmpty()) {
Expand Down Expand Up @@ -246,20 +245,33 @@ private static Map<String, CcToolchainProvider> getToolchainsByCpu(RuleContext r
}

private static Iterable<Artifact> filterUniqueSharedLibraries(
RuleContext ruleContext, Artifact linkedLibrary, NestedSet<? extends LinkerInput> libraries) {
RuleContext ruleContext, Artifact linkedLibrary, NestedSet<LibraryToLinkWrapper> libraries) {
Map<String, Artifact> basenames = new HashMap<>();
Set<Artifact> artifacts = new HashSet<>();
if (linkedLibrary != null) {
basenames.put(linkedLibrary.getExecPath().getBaseName(), linkedLibrary);
}
for (LinkerInput linkerInput : libraries) {
String name = linkerInput.getArtifact().getFilename();
if (!(CppFileTypes.SHARED_LIBRARY.matches(name)
|| CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(name))) {
for (LibraryToLinkWrapper linkerInput : libraries) {
if (linkerInput.getPicStaticLibrary() != null || linkerInput.getStaticLibrary() != null) {
// This is not a shared library and will not be loaded by Android, so skip it.
continue;
}
Artifact artifact = linkerInput.getOriginalLibraryArtifact();
Artifact artifact = null;
if (linkerInput.getInterfaceLibrary() != null) {
if (linkerInput.getResolvedSymlinkInterfaceLibrary() != null) {
artifact = linkerInput.getResolvedSymlinkInterfaceLibrary();
} else {
artifact = linkerInput.getInterfaceLibrary();
}
} else {
if (linkerInput.getResolvedSymlinkDynamicLibrary() != null) {
artifact = linkerInput.getResolvedSymlinkDynamicLibrary();
} else {
artifact = linkerInput.getDynamicLibrary();
}
}
Preconditions.checkNotNull(artifact);

if (!artifacts.add(artifact)) {
// We have already reached this library, e.g., through a different solib symlink.
continue;
Expand Down
Loading

0 comments on commit 3e727b1

Please sign in to comment.