Skip to content

Commit

Permalink
Remove late bound option defaults.
Browse files Browse the repository at this point in the history
This change affects config_settings that select on the "compiler" value.

Users can select on "compiler" value through the "@bazel_tools//tools/cpp:compiler" flag:

config_setting(
  name = "x",
  flag_values = {"@bazel_tools//tools/cpp:compiler": "y"},
)

RELNOTES: config_settings that select on "compiler" value instead of values = {"compiler" : "x"} should use flag_values = {"@bazel_tools//tools/cpp:compiler": "x"}.

Closes #6384.

PiperOrigin-RevId: 220442634
  • Loading branch information
scentini authored and Copybara-Service committed Nov 7, 2018
1 parent bd47bb5 commit c983241
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 245 deletions.
Expand Up @@ -894,31 +894,6 @@ public enum ConfigsMode {
NOTRIM,
}

@Option(
name = "experimental_use_late_bound_option_defaults",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
help =
"Do not use this flag. Use --incompatible_disable_late_bound_option_defaults instead.")
public boolean useLateBoundOptionDefaults;

@Option(
name = "incompatible_disable_late_bound_option_defaults",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"When true, Bazel will not allow late bound values read from the CROSSTOOL file "
+ "to be used in config_settings. The CROSSTOOL field used in this manner is "
+ "'compiler'. Instead of config_setting(values = {'compiler': 'x'}), "
+ "config_setting(flag_values = {'@bazel_tools/tools/cpp:compiler': 'x'}) should "
+ "be used.")
public boolean incompatibleDisableLateBoundOptionDefaults;

/**
* Converter for --experimental_dynamic_configs.
Expand Down Expand Up @@ -1320,11 +1295,7 @@ public BuildConfiguration(
this.testEnv = setupTestEnvironment();

this.transitiveOptionDetails =
computeOptionsMap(
buildOptions,
fragments.values(),
(options.useLateBoundOptionDefaults
&& !options.incompatibleDisableLateBoundOptionDefaults));
TransitiveOptionDetails.forOptionsWithDefaults(buildOptions.getNativeOptions());

ImmutableMap.Builder<String, String> globalMakeEnvBuilder = ImmutableMap.builder();
for (Fragment fragment : fragments.values()) {
Expand Down Expand Up @@ -1424,22 +1395,6 @@ TransitiveOptionDetails getTransitiveOptionDetails() {
return transitiveOptionDetails;
}

/** Computes and returns the {@link TransitiveOptionDetails} for this configuration. */
private static TransitiveOptionDetails computeOptionsMap(
BuildOptions buildOptions, Iterable<Fragment> fragments, boolean useLateBoundOptionDefaults) {
// Collect from our fragments "alternative defaults" for options where the default
// should be something other than what's specified in Option.defaultValue.
Map<String, Object> lateBoundDefaults = Maps.newHashMap();
if (useLateBoundOptionDefaults) {
for (Fragment fragment : fragments) {
lateBoundDefaults.putAll(fragment.lateBoundOptionDefaults());
}
}

return TransitiveOptionDetails.forOptionsWithDefaults(
buildOptions.getNativeOptions(), lateBoundDefaults);
}

private String buildMnemonic() {
// See explanation at declaration for outputRoots.
String platformSuffix = (options.platformSuffix != null) ? options.platformSuffix : "";
Expand Down Expand Up @@ -1974,8 +1929,4 @@ public ImmutableSet<String> getReservedActionMnemonics() {
return reservedActionMnemonics;
}

public boolean disableLateBoundOptionDefaults() {
return options.incompatibleDisableLateBoundOptionDefaults
|| !options.useLateBoundOptionDefaults;
}
}
Expand Up @@ -21,7 +21,6 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import java.io.Serializable;
import java.util.Map;
import javax.annotation.Nullable;

/**
Expand All @@ -37,8 +36,8 @@ public final class TransitiveOptionDetails implements Serializable {
* Computes and returns the transitive optionName -> "option info" map for the given set of
* options sets, using the given map as defaults for options which would otherwise be null.
*/
public static TransitiveOptionDetails forOptionsWithDefaults(
Iterable<? extends OptionsBase> buildOptions, Map<String, Object> lateBoundDefaults) {
static TransitiveOptionDetails forOptionsWithDefaults(
Iterable<? extends OptionsBase> buildOptions) {
ImmutableMap.Builder<String, OptionDetails> map = ImmutableMap.builder();
try {
for (OptionsBase options : buildOptions) {
Expand All @@ -51,14 +50,10 @@ public static TransitiveOptionDetails forOptionsWithDefaults(
continue;
}
Object value = optionDefinition.getField().get(options);
if (value == null) {
if (lateBoundDefaults.containsKey(optionDefinition.getOptionName())) {
value = lateBoundDefaults.get(optionDefinition.getOptionName());
} else if (!optionDefinition.isSpecialNullDefault()) {
if (value == null && !optionDefinition.isSpecialNullDefault()) {
// See {@link Option#defaultValue} for an explanation of default "null" strings.
value = optionDefinition.getUnparsedDefaultValue();
}
}
}
map.put(
optionDefinition.getOptionName(),
new OptionDetails(options.getClass(), value, optionDefinition.allowsMultiple()));
Expand Down
Expand Up @@ -76,6 +76,19 @@ public static class GraveyardOptions extends OptionsBase {
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
help = "noop")
public boolean forceIgnoreDashStatic;

@Option(
name = "incompatible_disable_late_bound_option_defaults",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.NO_OP},
metadataTags = {
OptionMetadataTag.DEPRECATED,
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "This option is deprecated and has no effect.")
public boolean incompatibleDisableLateBoundOptionDefaults;
}

@Override
Expand Down
Expand Up @@ -975,11 +975,9 @@ public CcToolchainConfigInfo ccToolchainConfigInfoFromSkylark(
if (!config.enableCcToolchainConfigInfoFromSkylark()) {
throw new InvalidConfigurationException("Creating a CcToolchainConfigInfo is not enabled.");
}
if (!skylarkRuleContext.getConfiguration().disableLateBoundOptionDefaults()
|| !config.disableMakeVariables()) {
if (!config.disableMakeVariables()) {
throw new InvalidConfigurationException(
"--incompatible_disable_late_bound_option_defaults and "
+ "--incompatible_disable_cc_configuration_make_variables must be set to true in "
"--incompatible_disable_cc_configuration_make_variables must be set to true in "
+ "order to configure the C++ toolchain from Starlark.");
}

Expand Down
Expand Up @@ -41,7 +41,6 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import java.util.Map;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -1119,13 +1118,6 @@ public boolean isStrictSystemIncludes() {
return cppOptions.strictSystemIncludes;
}

@Override
public Map<String, Object> lateBoundOptionDefaults() {
// --compiler initially defaults to null because its *actual* default isn't known
// until it's read from the CROSSTOOL. Feed the CROSSTOOL defaults in here.
return ImmutableMap.of("compiler", cppToolchainInfo.getCompiler());
}

public String getFdoInstrument() {
return cppOptions.fdoInstrumentForBuild;
}
Expand Down

0 comments on commit c983241

Please sign in to comment.