Skip to content

Commit

Permalink
Flip --incompatible_linkopts_in_user_link_flags
Browse files Browse the repository at this point in the history
    Closes #6826.

    This is a roll-forward of bazelbuild/bazel@4866f33 after Bazel with the flag was cut.

    RELNOTES: Incompatible flag `--incompatible_linkopts_in_user_link_flags` has been flipped (bazelbuild/bazel#6826)
    PiperOrigin-RevId: 233584729
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 99d7d52 commit 95d2325
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,6 @@
public class BazelRulesModule extends BlazeModule {
/** This is where deprecated options go to die. */
public static class GraveyardOptions extends OptionsBase {
@Option(
name = "incompatible_dont_emit_static_libgcc",
oldName = "experimental_dont_emit_static_libgcc",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.DEPRECATED,
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "Deprecated no-op.")
public boolean disableEmittingStaticLibgcc;

@Option(
name = "incompatible_linkopts_in_user_link_flags",
defaultValue = "true",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,10 @@ public static String getLegacyCrosstoolFieldErrorMessage(String field) {
+ "migration instructions).";
}

public boolean disableEmittingStaticLibgcc() {
return cppOptions.disableEmittingStaticLibgcc;
}

public boolean disableDepsetInUserFlags() {
return cppOptions.disableDepsetInUserFlags;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ public CppLinkAction build() throws InterruptedException {

boolean needWholeArchive =
wholeArchive
|| needWholeArchive(featureConfiguration, linkType, linkopts, cppConfiguration);
|| needWholeArchive(linkingMode, linkType, linkopts, isNativeDeps, cppConfiguration);
// Disallow LTO indexing for test targets that link statically, and optionally for any
// linkstatic target (which can be used to disable LTO indexing for non-testonly cc_binary
// built due to data dependences for a blaze test invocation). Otherwise this will provoke
Expand Down Expand Up @@ -1120,37 +1120,15 @@ private boolean shouldUseLinkDynamicLibraryTool() {

/** The default heuristic on whether we need to use whole-archive for the link. */
private static boolean needWholeArchive(
FeatureConfiguration featureConfiguration,
Link.LinkingMode staticness,
LinkTargetType type,
Collection<String> linkopts,
boolean isNativeDeps,
CppConfiguration cppConfig) {
boolean mostlyStatic = (staticness == Link.LinkingMode.STATIC);
boolean sharedLinkopts =
type.isDynamicLibrary() || linkopts.contains("-shared") || cppConfig.hasSharedLinkOption();
// Fasten your seat belt, the logic below doesn't make perfect sense and it's full of obviously
// missed corner cases. The world still stands and depends on this behavior, so ¯\_(ツ)_/¯.
if (!sharedLinkopts) {
// We are not producing shared library, there is no reason to use --whole-archive with
// executable (if the executable doesn't use the symbols, nobody else will, so --whole-archive
// is not needed).
return false;
}
if (cppConfig.removeLegacyWholeArchive()) {
// --incompatible_remove_legacy_whole_archive has been flipped, no --whole-archive for the
// entire build.
return false;
}
if (featureConfiguration.getRequestedFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) {
// --incompatible_remove_legacy_whole_archive has not been flipped, and this target requested
// --whole-archive using features.
return true;
}
if (cppConfig.legacyWholeArchive()) {
// --incompatible_remove_legacy_whole_archive has not been flipped, so whether to
// use --whole-archive depends on --legacy_whole_archive.
return true;
}
// Hopefully future default.
return false;
return (isNativeDeps || cppConfig.legacyWholeArchive()) && mostlyStatic && sharedLinkopts;
}

private static ImmutableSet<Artifact> constructOutputs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,21 @@ public Label getFdoPrefetchHintsLabel() {
+ "(see https://github.com/bazelbuild/bazel/issues/7008 for migration instructions).")
public boolean disableExpandIfAllAvailableInFlagSet;

@Option(
name = "incompatible_dont_emit_static_libgcc",
oldName = "experimental_dont_emit_static_libgcc",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If true, bazel will not add --static-libgcc to the linking command line, it will be "
+ "the responsibility of the C++ toolchain to append this flag.")
public boolean disableEmittingStaticLibgcc;

@Option(
name = "incompatible_disable_crosstool_file",
defaultValue = "false",
Expand Down Expand Up @@ -862,6 +877,7 @@ public FragmentOptions getHost() {

host.doNotUseCpuTransformer = doNotUseCpuTransformer;
host.disableGenruleCcToolchainDependency = disableGenruleCcToolchainDependency;
host.disableEmittingStaticLibgcc = disableEmittingStaticLibgcc;
host.disableDepsetInUserFlags = disableDepsetInUserFlags;
host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet;
host.disableLegacyCcProvider = disableLegacyCcProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ public static CcToolchainVariables setupVariables(
buildVariables.addStringVariable(
THINLTO_OBJECT_SUFFIX_REPLACE.getVariableName(),
Iterables.getOnlyElement(CppFileTypes.LTO_INDEXING_OBJECT_FILE.getExtensions())
+ ";"
+ objectFileExtension);
+ ";" + objectFileExtension);
if (thinltoMergedObjectFile != null) {
buildVariables.addStringVariable(
THINLTO_MERGED_OBJECT_FILE.getVariableName(), thinltoMergedObjectFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ private RuleContext getRuleContext() throws Exception {
return getRuleContext(getConfiguredTarget("//foo:foo"));
}

@Test
public void testIsUsingFissionIsIdenticalForCompileAndLink() {
assertThat(LinkBuildVariables.IS_USING_FISSION.getVariableName())
.isEqualTo(CompileBuildVariables.IS_USING_FISSION.getVariableName());
}

@Test
public void testForcePicBuildVariable() throws Exception {
useConfiguration("--force_pic");
Expand Down Expand Up @@ -147,7 +141,9 @@ public void testInterfaceLibraryBuildingVariablesWhenLegacyGenerationPossible()
public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws Exception {
// Make sure the interface shared object generation is enabled in the configuration
// (which it is not by default for some windows toolchains)
AnalysisMock.get().ccSupport().setupCrosstool(mockToolsConfig);
AnalysisMock.get()
.ccSupport()
.setupCrosstool(mockToolsConfig, MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES);
useConfiguration();

verifyIfsoVariables();
Expand Down

0 comments on commit 95d2325

Please sign in to comment.