Skip to content

Commit

Permalink
Introduce --incompatible_remove_legacy_whole_archive
Browse files Browse the repository at this point in the history 
    This flag disables legacy whole archive behavior, as well as makes the --legacy_whole_archive option a noop. See details in
    bazelbuild/bazel#7362.

    It also introduces a possibility to enable legacy behavior per target by `--features=legacy_whole_archive` option, or `features = [ "legacy_whole_archive" ]` attribute.

    RELNOTES: None.
    PiperOrigin-RevId: 233691404
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 02d8fcf commit 25f2aae
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 82 deletions.
4 changes: 0 additions & 4 deletions ...elbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,4 @@ public boolean enableLegacyCcProvider() {
public boolean disableCrosstool() {
return cppOptions.disableCrosstool;
}

public boolean dontEnableHostNonhost() {
return cppOptions.dontEnableHostNonhost;
}
}
103 changes: 53 additions & 50 deletions ...ild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.rules.cpp.CppLinkAction.LinkArtifactFactory;
import com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.CollectedLibrariesToLink;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext.Linkstamp;
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.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -111,9 +112,9 @@ public Artifact create(
// Keep these in sync with {@link Context}.
private final Set<LinkerInput> objectFiles = new LinkedHashSet<>();
private final Set<Artifact> nonCodeInputs = new LinkedHashSet<>();
private final NestedSetBuilder<LinkerInputs.LibraryToLink> libraries =
private final NestedSetBuilder<LibraryToLink> libraries = NestedSetBuilder.linkOrder();
private final NestedSetBuilder<LibraryToLinkWrapper> libraryToLinkWrappers =
NestedSetBuilder.linkOrder();
private final NestedSetBuilder<LibraryToLink> librariesToLink = NestedSetBuilder.linkOrder();
private NestedSet<Artifact> linkerFiles = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
private Artifact runtimeMiddleman;
private ArtifactCategory toolchainLibrariesType = null;
Expand Down Expand Up @@ -219,8 +220,10 @@ public Set<LinkerInput> getObjectFiles() {
return objectFiles;
}

/** Returns linker inputs that are libraries. */
public NestedSetBuilder<LinkerInputs.LibraryToLink> getLibraries() {
/**
* Returns linker inputs that are libraries.
*/
public NestedSetBuilder<LibraryToLink> getLibraries() {
return libraries;
}

Expand Down Expand Up @@ -280,12 +283,10 @@ private ImmutableSet<LinkerInput> computeLtoIndexingObjectFileInputs() {
* Maps bitcode library files used by the LTO backends to the corresponding minimized bitcode file
* used as input to the LTO indexing step.
*/
private static NestedSet<LinkerInputs.LibraryToLink> computeLtoIndexingUniqueLibraries(
NestedSet<LinkerInputs.LibraryToLink> originalUniqueLibraries,
boolean includeLinkStaticInLtoIndexing) {
NestedSetBuilder<LinkerInputs.LibraryToLink> uniqueLibrariesBuilder =
NestedSetBuilder.linkOrder();
for (LinkerInputs.LibraryToLink lib : originalUniqueLibraries) {
private static NestedSet<LibraryToLink> computeLtoIndexingUniqueLibraries(
NestedSet<LibraryToLink> originalUniqueLibraries, boolean includeLinkStaticInLtoIndexing) {
NestedSetBuilder<LibraryToLink> uniqueLibrariesBuilder = NestedSetBuilder.linkOrder();
for (LibraryToLink lib : originalUniqueLibraries) {
if (!lib.containsObjectFiles()) {
uniqueLibrariesBuilder.add(lib);
continue;
Expand Down Expand Up @@ -331,7 +332,7 @@ public boolean hasLtoBitcodeInputs() {
if (!ltoCompilationContext.isEmpty()) {
return true;
}
for (LinkerInputs.LibraryToLink lib : libraries.build()) {
for (LibraryToLink lib : libraries.build()) {
if (!lib.getLtoCompilationContext().isEmpty()) {
return true;
}
Expand Down Expand Up @@ -409,11 +410,11 @@ private List<String> getLtoBackendCommandLineOptions(

private Iterable<LtoBackendArtifacts> createLtoArtifacts(
PathFragment ltoOutputRootPrefix,
NestedSet<LinkerInputs.LibraryToLink> uniqueLibraries,
NestedSet<LibraryToLink> uniqueLibraries,
boolean allowLtoIndexing,
boolean includeLinkStaticInLtoIndexing) {
Set<Artifact> compiled = new LinkedHashSet<>();
for (LinkerInputs.LibraryToLink lib : uniqueLibraries) {
for (LibraryToLink lib : uniqueLibraries) {
compiled.addAll(lib.getLtoCompilationContext().getBitcodeFiles());
}

Expand All @@ -424,7 +425,7 @@ private Iterable<LtoBackendArtifacts> createLtoArtifacts(
// statically linked, so we need to look at includeLinkStaticInLtoIndexing to decide whether
// to include its objects in the LTO indexing for this target.
if (includeLinkStaticInLtoIndexing) {
for (LinkerInputs.LibraryToLink lib : uniqueLibraries) {
for (LibraryToLink lib : uniqueLibraries) {
if (!lib.containsObjectFiles()) {
continue;
}
Expand All @@ -442,7 +443,7 @@ private Iterable<LtoBackendArtifacts> createLtoArtifacts(
}

ImmutableList.Builder<LtoBackendArtifacts> ltoOutputs = ImmutableList.builder();
for (LinkerInputs.LibraryToLink lib : uniqueLibraries) {
for (LibraryToLink lib : uniqueLibraries) {
if (!lib.containsObjectFiles()) {
continue;
}
Expand Down Expand Up @@ -556,18 +557,19 @@ boolean canSplitCommandLine() {
}
}

private List<LinkerInputs.LibraryToLink> convertLibraryToLinkListToLibraryToLinkList(
NestedSet<LibraryToLink> librariesToLink) {
ImmutableList.Builder<LinkerInputs.LibraryToLink> librariesToLinkBuilder =
ImmutableList.builder();
for (LibraryToLink libraryToLink : librariesToLink) {
LinkerInputs.LibraryToLink staticLibraryToLink =
libraryToLink.getStaticLibrary() == null ? null : libraryToLink.getStaticLibraryToLink();
LinkerInputs.LibraryToLink picStaticLibraryToLink =
libraryToLink.getPicStaticLibrary() == null
private List<LibraryToLink> convertLibraryToLinkWrapperListToLibraryToLinkList(
NestedSet<LibraryToLinkWrapper> libraryToLinkWrappers) {
ImmutableList.Builder<LibraryToLink> librariesToLink = ImmutableList.builder();
for (LibraryToLinkWrapper libraryToLinkWrapper : libraryToLinkWrappers) {
LibraryToLink staticLibraryToLink =
libraryToLinkWrapper.getStaticLibrary() == null
? null
: libraryToLink.getPicStaticLibraryToLink();
LinkerInputs.LibraryToLink libraryToLinkToUse = null;
: libraryToLinkWrapper.getStaticLibraryToLink();
LibraryToLink picStaticLibraryToLink =
libraryToLinkWrapper.getPicStaticLibrary() == null
? null
: libraryToLinkWrapper.getPicStaticLibraryToLink();
LibraryToLink libraryToLinkToUse = null;
if (linkingMode == LinkingMode.STATIC) {
if (linkType.isDynamicLibrary()) {
if (picStaticLibraryToLink != null) {
Expand All @@ -584,30 +586,31 @@ private List<LinkerInputs.LibraryToLink> convertLibraryToLinkListToLibraryToLink
}
}
if (libraryToLinkToUse == null) {
if (libraryToLink.getInterfaceLibrary() != null) {
libraryToLinkToUse = libraryToLink.getInterfaceLibraryToLink();
} else if (libraryToLink.getDynamicLibrary() != null) {
libraryToLinkToUse = libraryToLink.getDynamicLibraryToLink();
if (libraryToLinkWrapper.getInterfaceLibrary() != null) {
libraryToLinkToUse = libraryToLinkWrapper.getInterfaceLibraryToLink();
} else if (libraryToLinkWrapper.getDynamicLibrary() != null) {
libraryToLinkToUse = libraryToLinkWrapper.getDynamicLibraryToLink();
}
}
Preconditions.checkNotNull(libraryToLinkToUse);
checkLibrary(libraryToLinkToUse);
librariesToLinkBuilder.add(libraryToLinkToUse);
librariesToLink.add(libraryToLinkToUse);
}
return librariesToLinkBuilder.build();
return librariesToLink.build();
}

/** Builds the Action as configured and returns it. */
public CppLinkAction build() throws InterruptedException {
NestedSet<LinkerInputs.LibraryToLink> originalUniqueLibraries = null;
NestedSet<LibraryToLink> originalUniqueLibraries = null;

if (librariesToLink.isEmpty()) {
if (libraryToLinkWrappers.isEmpty()) {
originalUniqueLibraries = libraries.build();
} else {
Preconditions.checkState(libraries.isEmpty());
originalUniqueLibraries =
NestedSetBuilder.<LinkerInputs.LibraryToLink>linkOrder()
.addAll(convertLibraryToLinkListToLibraryToLinkList(librariesToLink.build()))
NestedSetBuilder.<LibraryToLink>linkOrder()
.addAll(
convertLibraryToLinkWrapperListToLibraryToLinkList(libraryToLinkWrappers.build()))
.build();
}

Expand Down Expand Up @@ -697,7 +700,7 @@ public CppLinkAction build() throws InterruptedException {
// Get the set of object files and libraries containing the correct
// inputs for this link, depending on whether this is LTO indexing or
// a native link.
NestedSet<LinkerInputs.LibraryToLink> uniqueLibraries;
NestedSet<LibraryToLink> uniqueLibraries;
ImmutableSet<LinkerInput> objectFileInputs;
ImmutableSet<LinkerInput> linkstampObjectFileInputs;
if (isLtoIndexing) {
Expand Down Expand Up @@ -734,7 +737,7 @@ public CppLinkAction build() throws InterruptedException {
.addAll(objectArtifacts)
.addAll(linkstampObjectArtifacts)
.build();
final LinkerInputs.LibraryToLink outputLibrary =
final LibraryToLink outputLibrary =
linkType.isExecutable()
? null
: LinkerInputs.newInputLibrary(
Expand All @@ -749,7 +752,7 @@ public CppLinkAction build() throws InterruptedException {
: LtoCompilationContext.EMPTY,
createSharedNonLtoArtifacts(isLtoIndexing),
/* mustKeepDebug= */ false);
final LinkerInputs.LibraryToLink interfaceOutputLibrary =
final LibraryToLink interfaceOutputLibrary =
(interfaceOutput == null)
? null
: LinkerInputs.newInputLibrary(
Expand Down Expand Up @@ -1352,7 +1355,7 @@ public CppLinkActionBuilder addFakeObjectFiles(Iterable<Artifact> inputs) {
return this;
}

private void checkLibrary(LinkerInputs.LibraryToLink input) {
private void checkLibrary(LibraryToLink input) {
String name = input.getArtifact().getFilename();
Preconditions.checkArgument(
Link.ARCHIVE_LIBRARY_FILETYPES.matches(name) || Link.SHARED_LIBRARY_FILETYPES.matches(name),
Expand All @@ -1365,7 +1368,7 @@ private void checkLibrary(LinkerInputs.LibraryToLink input) {
* library. Note that all directly added libraries are implicitly ordered before all nested sets
* added with {@link #addLibraries}, even if added in the opposite order.
*/
public CppLinkActionBuilder addLibrary(LinkerInputs.LibraryToLink input) {
public CppLinkActionBuilder addLibrary(LibraryToLink input) {
checkLibrary(input);
libraries.add(input);
if (input.isMustKeepDebug()) {
Expand All @@ -1378,9 +1381,9 @@ public CppLinkActionBuilder addLibrary(LinkerInputs.LibraryToLink input) {
* Adds multiple artifact to the set of inputs. The artifacts must be archives or shared
* libraries.
*/
public CppLinkActionBuilder addLibraries(Iterable<LinkerInputs.LibraryToLink> inputs) {
Preconditions.checkState(librariesToLink.isEmpty());
for (LinkerInputs.LibraryToLink input : inputs) {
public CppLinkActionBuilder addLibraries(Iterable<LibraryToLink> inputs) {
Preconditions.checkState(libraryToLinkWrappers.isEmpty());
for (LibraryToLink input : inputs) {
checkLibrary(input);
if (input.isMustKeepDebug()) {
mustKeepDebug = true;
Expand All @@ -1390,14 +1393,14 @@ public CppLinkActionBuilder addLibraries(Iterable<LinkerInputs.LibraryToLink> in
return this;
}

public CppLinkActionBuilder addLibrariesToLink(Iterable<LibraryToLink> inputs) {
public CppLinkActionBuilder addLibraryToLinkWrappers(Iterable<LibraryToLinkWrapper> inputs) {
Preconditions.checkState(libraries.isEmpty());
for (LibraryToLink input : inputs) {
for (LibraryToLinkWrapper input : inputs) {
if (input.getMustKeepDebug()) {
mustKeepDebug = true;
}
}
this.librariesToLink.addAll(inputs);
this.libraryToLinkWrappers.addAll(inputs);
return this;
}

Expand Down Expand Up @@ -1469,7 +1472,7 @@ public CppLinkActionBuilder addLinkopts(Collection<String> linkopts) {
* #addLibraries}, and {@link #addLinkstamps}.
*/
public CppLinkActionBuilder addLinkParams(
List<LinkerInputs.LibraryToLink> libraries,
List<LibraryToLink> libraries,
List<String> userLinkFlags,
List<Linkstamp> linkstamps,
List<Artifact> nonCodeInputs,
Expand Down
15 changes: 0 additions & 15 deletions ...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,20 +685,6 @@ public Label getFdoPrefetchHintsLabel() {
)
public boolean useLLVMCoverageMapFormat;

@Option(
name = "incompatible_dont_enable_host_nonhost_crosstool_features",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If true, Bazel will not enable 'host' and 'nonhost' features in the c++ toolchain "
+ "(see https://github.com/bazelbuild/bazel/issues/7407 for more information).")
public boolean dontEnableHostNonhost;

@Option(
name = "incompatible_disable_legacy_crosstool_fields",
oldName = "experimental_disable_legacy_crosstool_fields",
Expand Down Expand Up @@ -900,7 +886,6 @@ public FragmentOptions getHost() {
host.disableCrosstool = disableCrosstool;
host.enableCcToolchainResolution = enableCcToolchainResolution;
host.removeLegacyWholeArchive = removeLegacyWholeArchive;
host.dontEnableHostNonhost = dontEnableHostNonhost;
return host;
}

Expand Down
3 changes: 3 additions & 0 deletions ...azelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,9 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) {
*/
public static final String SUPPORTS_DYNAMIC_LINKER = "supports_dynamic_linker";

/** A feature marking that the target needs to link its deps in --whole-archive block. */
public static final String LEGACY_WHOLE_ARCHIVE = "legacy_whole_archive";

/** Ancestor for all rules that do include scanning. */
public static final class CcIncludeScanningRule implements RuleDefinition {
@Override
Expand Down
28 changes: 15 additions & 13 deletions ....bazel/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@
import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
import com.google.devtools.build.lib.rules.cpp.CppSemantics;
import com.google.devtools.build.lib.rules.cpp.FdoContext;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext.Linkstamp;
import com.google.devtools.build.lib.rules.cpp.LibraryToLinkWrapper;
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;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
Expand Down Expand Up @@ -148,8 +148,8 @@ public static Artifact linkAndroidNativeDepsIfPresent(
}

/** Determines if there is any code to be linked in the input iterable. */
private static boolean containsCodeToLink(Iterable<LibraryToLink> libraries) {
for (LibraryToLink library : libraries) {
private static boolean containsCodeToLink(Iterable<LibraryToLinkWrapper> libraries) {
for (LibraryToLinkWrapper library : libraries) {
if (containsCodeToLink(library)) {
return true;
}
Expand All @@ -158,7 +158,7 @@ private static boolean containsCodeToLink(Iterable<LibraryToLink> libraries) {
}

/** Determines if the input library is or contains an archive which must be linked. */
private static boolean containsCodeToLink(LibraryToLink library) {
private static boolean containsCodeToLink(LibraryToLinkWrapper library) {
if (library.getStaticLibrary() == null && library.getPicStaticLibrary() == null) {
// this is a shared library so we're going to have to copy it
return false;
Expand Down Expand Up @@ -210,7 +210,7 @@ public static NativeDepsRunfiles createNativeDepsAction(
ruleContext, CppBuildInfo.KEY, configuration);

boolean shareNativeDeps = configuration.getFragment(CppConfiguration.class).shareNativeDeps();
NestedSet<LibraryToLink> linkerInputs = ccLinkingContext.getLibraries();
NestedSet<LibraryToLinkWrapper> linkerInputs = ccLinkingContext.getLibraries();
Artifact sharedLibrary;
if (shareNativeDeps) {
PathFragment sharedPath =
Expand All @@ -230,13 +230,15 @@ public static NativeDepsRunfiles createNativeDepsAction(
sharedLibrary = nativeDeps;
}
FdoContext fdoContext = toolchain.getFdoContext();
ImmutableSet.Builder<String> requestedFeatures =
ImmutableSet.<String>builder().addAll(ruleContext.getFeatures()).add(STATIC_LINKING_MODE);
if (!ruleContext.getDisabledFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) {
requestedFeatures.add(CppRuleClasses.LEGACY_WHOLE_ARCHIVE);
}
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrReportRuleError(
ruleContext,
/* requestedFeatures= */ ImmutableSet.<String>builder()
.addAll(ruleContext.getFeatures())
.add(STATIC_LINKING_MODE)
.build(),
/* requestedFeatures= */ requestedFeatures.build(),
/* unsupportedFeatures= */ ruleContext.getDisabledFeatures(),
toolchain);
CppLinkActionBuilder builder =
Expand All @@ -260,7 +262,7 @@ public static NativeDepsRunfiles createNativeDepsAction(
toolchain.getStaticRuntimeLinkInputs(ruleContext, featureConfiguration));
}
LtoCompilationContext.Builder ltoCompilationContext = new LtoCompilationContext.Builder();
for (LibraryToLink lib : linkerInputs) {
for (LibraryToLinkWrapper lib : linkerInputs) {
if (lib.getPicLtoCompilationContext() != null
&& !lib.getPicLtoCompilationContext().isEmpty()) {
ltoCompilationContext.addAll(lib.getPicLtoCompilationContext());
Expand All @@ -278,7 +280,7 @@ public static NativeDepsRunfiles createNativeDepsAction(
builder
.setLinkArtifactFactory(SHAREABLE_LINK_ARTIFACT_FACTORY)
.setLinkerFiles(toolchain.getLinkerFiles())
.addLibrariesToLink(linkerInputs)
.addLibraryToLinkWrappers(linkerInputs)
.setLinkType(LinkTargetType.DYNAMIC_LIBRARY)
.setLinkingMode(LinkingMode.STATIC)
.setLibraryIdentifier(libraryIdentifier)
Expand Down
Loading

0 comments on commit 25f2aae

Please sign in to comment.