Skip to content

Commit

Permalink
Remove legacy crosstool fields
Browse files Browse the repository at this point in the history
Closes #5883.

RELNOTES: None.
PiperOrigin-RevId: 240978300
  • Loading branch information
hlopko authored and copybara-github committed Mar 29, 2019
1 parent ffb65c8 commit 107a43d
Show file tree
Hide file tree
Showing 26 changed files with 78 additions and 2,010 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ public class BazelRulesModule extends BlazeModule {
/** This is where deprecated options go to die. */
public static class GraveyardOptions extends OptionsBase {

@Option(
name = "incompatible_disable_legacy_crosstool_fields",
oldName = "experimental_disable_legacy_crosstool_fields",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "Deprecated no-op.")
public boolean disableLegacyCrosstoolFields;

@Option(
name = "incompatible_require_feature_configuration_for_pic",
defaultValue = "true",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,6 @@ public void collectMetadataArtifacts(Iterable<Artifact> objectFiles,
.addAll(ALL_OTHER_ACTIONS)
.build();

/** Features we request to enable unless a rule explicitly doesn't support them. */
private static final ImmutableSet<String> DEFAULT_FEATURES =
ImmutableSet.of(
CppRuleClasses.DEPENDENCY_FILE,
CppRuleClasses.RANDOM_SEED,
CppRuleClasses.MODULE_MAPS,
CppRuleClasses.MODULE_MAP_HOME_CWD,
CppRuleClasses.HEADER_MODULE_COMPILE,
CppRuleClasses.INCLUDE_PATHS,
CppRuleClasses.PIC,
CppRuleClasses.PREPROCESSOR_DEFINES);

public static final String CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME = ":cc_toolchain";
private static final String SYSROOT_FLAG = "--sysroot=";

Expand Down Expand Up @@ -424,21 +412,6 @@ public void reportInvalidOptions(RuleContext ruleContext) {

public static void reportInvalidOptions(
RuleContext ruleContext, CppConfiguration cppConfiguration, CcToolchainProvider ccToolchain) {
if (cppConfiguration.fissionIsActiveForCurrentCompilationMode()
&& !ccToolchain.supportsFission()) {
ruleContext.ruleWarning(
"Fission is not supported by this crosstool. Please use a "
+ "supporting crosstool to enable fission");
}
if (cppConfiguration.buildTestDwpIsActivated()
&& !(ccToolchain.supportsFission()
&& cppConfiguration.fissionIsActiveForCurrentCompilationMode())) {
ruleContext.ruleWarning(
"Test dwp file requested, but Fission is not enabled. To generate a "
+ "dwp for the test executable, use '--fission=yes' with a toolchain that supports "
+ "Fission to build statically.");
}

if (cppConfiguration.getLibcTopLabel() != null && ccToolchain.getDefaultSysroot() == null) {
ruleContext.ruleError(
"The selected toolchain "
Expand Down Expand Up @@ -762,46 +735,6 @@ public static ImmutableList<String> getCoverageFeatures(CppConfiguration cppConf
return coverageFeatures.build();
}

/**
* Determines whether to statically link the C++ runtimes.
*
* <p>This is complicated because it depends both on a legacy field in the CROSSTOOL
* protobuf--supports_embedded_runtimes--and the newer crosstool
* feature--statically_link_cpp_runtimes. Neither, one, or both could be present or set. Or they
* could be set in to conflicting values.
*
* @return true if we should statically link, false otherwise.
*/
private static boolean enableStaticLinkCppRuntimesFeature(
ImmutableSet<String> requestedFeatures,
ImmutableSet<String> disabledFeatures,
CcToolchainProvider toolchain) {
// All of these cases are encountered in various unit tests,
// integration tests, and obscure CROSSTOOLs.

// A. If the legacy field "supports_embedded_runtimes" is false (or not present):
// dynamically link the cpp runtimes. Done.
if (!toolchain.supportsEmbeddedRuntimes()) {
return false;
}
// From here, the toolchain _can_ statically link the cpp runtime.
//
// B. If the feature static_link_cpp_runtimes is disabled:
// dynamically link the cpp runtimes. Done.
if (disabledFeatures.contains(CppRuleClasses.STATIC_LINK_CPP_RUNTIMES)) {
return false;
}
// C. If the feature is not requested:
// the feature is neither disabled nor requested: statically
// link (for compatibility with the legacy field).
if (!requestedFeatures.contains(CppRuleClasses.STATIC_LINK_CPP_RUNTIMES)) {
return true;
}
// D. The feature is requested:
// statically link the cpp runtimes. Done.
return true;
}

/**
* Creates a feature configuration for a given rule. Assumes strictly cc sources.
*
Expand Down Expand Up @@ -858,20 +791,6 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
unsupportedFeaturesBuilder.add(CppRuleClasses.MODULE_MAPS);
}

if (toolchain.supportsEmbeddedRuntimes()) {
if (!cppConfiguration.disableLegacyCrosstoolFields()) {
// If we're not using legacy crosstool fields, we assume 'static_link_cpp_runtimes' feature
// is enabled by default for toolchains that support it, and can be disabled by the user
// when needed using --feature=-static_link_cpp_runtimes option or
// features = [ '-static_link_cpp_runtimes' ] rule attribute.
if (enableStaticLinkCppRuntimesFeature(requestedFeatures, unsupportedFeatures, toolchain)) {
allRequestedFeaturesBuilder.add(CppRuleClasses.STATIC_LINK_CPP_RUNTIMES);
} else {
unsupportedFeaturesBuilder.add(CppRuleClasses.STATIC_LINK_CPP_RUNTIMES);
}
}
}

if (cppConfiguration.forcePic()) {
if (unsupportedFeatures.contains(CppRuleClasses.SUPPORTS_PIC)) {
throw new EvalException(/* location= */ null, PIC_CONFIGURATION_ERROR);
Expand Down Expand Up @@ -900,10 +819,6 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
ImmutableList.Builder<String> allFeatures =
new ImmutableList.Builder<String>()
.addAll(ImmutableSet.of(cppConfiguration.getCompilationMode().toString()))
.addAll(
cppConfiguration.disableLegacyCrosstoolFields()
? ImmutableList.of()
: DEFAULT_FEATURES)
.addAll(DEFAULT_ACTION_CONFIGS)
.addAll(requestedFeatures)
.addAll(toolchain.getFeatures().getDefaultFeaturesAndActionConfigs());
Expand All @@ -916,12 +831,6 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
}
}

if (cppConfiguration.fissionIsActiveForCurrentCompilationMode()
&& toolchain.supportsFission()
&& !cppConfiguration.disableLegacyCrosstoolFields()) {
allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}

allFeatures.addAll(getCoverageFeatures(cppConfiguration));

String fdoInstrument = cppConfiguration.getFdoInstrument();
Expand Down Expand Up @@ -985,7 +894,7 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
+ "This is most likely a misconfiguration in the C++ toolchain.");
}
}
if ((cppConfiguration.forcePic() || toolchain.toolchainNeedsPic())
if ((cppConfiguration.forcePic())
&& (!featureConfiguration.isEnabled(CppRuleClasses.PIC)
&& !featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_PIC))) {
throw new EvalException(/* location= */ null, PIC_CONFIGURATION_ERROR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,7 @@ public CcToolchainVariables getCompileBuildVariables(
asStringNestedSet(includeDirs),
asStringNestedSet(quoteIncludeDirs),
asStringNestedSet(systemIncludeDirs),
asStringNestedSet(defines),
addLegacyCxxOptions);
asStringNestedSet(defines));
}

@Override
Expand Down Expand Up @@ -281,8 +280,6 @@ public CcToolchainVariables getLinkBuildVariables(
asStringNestedSet(runtimeLibrarySearchDirectories),
/* librariesToLink= */ null,
asStringNestedSet(librarySearchDirectories),
/* isLegacyFullyStaticLinkingMode= */ false,
isStaticLinkingMode,
/* addIfsoRelatedVariables= */ false);
}

Expand Down Expand Up @@ -992,34 +989,10 @@ public CcToolchainConfigInfo ccToolchainConfigInfoFromSkylark(
compiler,
abiVersion,
abiLibcVersion,
/* supportsStartEndLib= */ false,
/* supportsInterfaceSharedLibraries= */ false,
/* supportsEmbeddedRuntimes= */ false,
/* staticRuntimesFilegroup= */ "",
/* dynamicRuntimesFilegroup= */ "",
/* supportsFission */ false,
/* needsPic= */ false,
toolPathList,
/* compilerFlags= */ ImmutableList.of(),
/* cxxFlags= */ ImmutableList.of(),
/* unfilteredCxxFlags= */ ImmutableList.of(),
/* linkerFlags= */ ImmutableList.of(),
/* dynamicLibraryLinkerFlags= */ ImmutableList.of(),
/* testOnlyLinkerFlags= */ ImmutableList.of(),
/* objcopyEmbedFlags= */ ImmutableList.of(),
/* ldEmbedFlags= */ ImmutableList.of(),
/* compilationModeCompilerFlags= */ ImmutableMap.of(),
/* compilationModeCxxFlags= */ ImmutableMap.of(),
/* compilationModeLinkerFlags= */ ImmutableMap.of(),
/* mostlyStaticLinkingModeFlags= */ ImmutableList.of(),
/* dynamicLinkingModeFlags= */ ImmutableList.of(),
/* fullyStaticLinkingModeFlags= */ ImmutableList.of(),
/* mostlyStaticLibrariesLinkingModeFlags= */ ImmutableList.of(),
makeVariablePairs.build(),
convertFromNoneable(builtinSysroot, /* defaultValue= */ ""),
/* defaultLibcTop= */ "",
convertFromNoneable(ccTargetOs, /* defaultValue= */ ""),
/* hasDynamicLinkingModeFlags= */ false,
cToolchain.build().toString());
}

Expand Down
Loading

0 comments on commit 107a43d

Please sign in to comment.