Skip to content

Commit

Permalink
Rename TransitiveOptionDetails to BuildOptionDetails for better clarity.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 408396064
  • Loading branch information
gregestren authored and copybara-github committed Nov 8, 2021
1 parent bd183ba commit a9206eb
Show file tree
Hide file tree
Showing 15 changed files with 119 additions and 126 deletions.
14 changes: 7 additions & 7 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ java_library(
":common_prerequisite_validator",
":config/auto_cpu_converter",
":config/build_configuration",
":config/build_option_details",
":config/build_options",
":config/build_options_cache",
":config/compilation_mode",
Expand All @@ -78,7 +79,6 @@ java_library(
":config/transitions/patch_transition",
":config/transitions/split_transition",
":config/transitions/transition_factory",
":config/transitive_option_details",
":configurations_collector",
":configured_object_value",
":configured_target",
Expand Down Expand Up @@ -283,6 +283,7 @@ java_library(
":buildinfo/build_info_collection",
":buildinfo/build_info_key",
":config/build_configuration",
":config/build_option_details",
":config/build_options",
":config/config_conditions",
":config/config_matching_provider",
Expand All @@ -307,7 +308,6 @@ java_library(
":config/transitions/split_transition",
":config/transitions/starlark_exposed_rule_transition_factory",
":config/transitions/transition_factory",
":config/transitive_option_details",
":configurations_collector",
":configured_target",
":constraints/constraint_constants",
Expand Down Expand Up @@ -1517,14 +1517,14 @@ java_library(
],
deps = [
":blaze_directories",
":config/build_option_details",
":config/build_options",
":config/compilation_mode",
":config/core_options",
":config/fragment",
":config/fragment_class_set",
":config/invalid_configuration_exception",
":config/run_under",
":config/transitive_option_details",
":platform_options",
":starlark/function_transition_util",
"//src/main/java/com/google/devtools/build/lib/actions",
Expand Down Expand Up @@ -1819,8 +1819,8 @@ java_library(
)

java_library(
name = "config/transitive_option_details",
srcs = ["config/TransitiveOptionDetails.java"],
name = "config/build_option_details",
srcs = ["config/BuildOptionDetails.java"],
deps = [
":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/cmdline",
Expand All @@ -1835,11 +1835,11 @@ java_library(
name = "config/transitions/composing_transition",
srcs = ["config/transitions/ComposingTransition.java"],
deps = [
":config/build_option_details",
":config/build_options",
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
":config/transitive_option_details",
":required_config_fragments_provider",
"//src/main/java/com/google/devtools/build/lib/events",
"//third_party:guava",
Expand Down Expand Up @@ -1867,9 +1867,9 @@ java_library(
"config/transitions/TransitionUtil.java",
],
deps = [
":config/build_option_details",
":config/build_options",
":config/fragment_options",
":config/transitive_option_details",
":required_config_fragments_provider",
"//src/main/java/com/google/devtools/build/lib/events",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public interface ActionEnvironmentProvider {
private final ImmutableMap<String, String> commandLineBuildVariables;

/** Data for introspecting the options used by this configuration. */
private final TransitiveOptionDetails transitiveOptionDetails;
private final BuildOptionDetails buildOptionDetails;

private final Supplier<BuildConfigurationEvent> buildEventSupplier;

Expand Down Expand Up @@ -199,8 +199,8 @@ public BuildConfigurationValue(

this.actionEnv = actionEnvironment;
this.testEnv = setupTestEnvironment();
this.transitiveOptionDetails =
TransitiveOptionDetails.forOptions(
this.buildOptionDetails =
BuildOptionDetails.forOptions(
buildOptions.getNativeOptions(), buildOptions.getStarlarkOptions());

// These should be documented in the build encyclopedia.
Expand Down Expand Up @@ -244,11 +244,9 @@ public BuildConfigurationKey getKey() {
return BuildConfigurationKey.withoutPlatformMapping(fragmentClassSet, buildOptions);
}

/**
* Retrieves the {@link TransitiveOptionDetails} containing data on this configuration's options.
*/
public TransitiveOptionDetails getTransitiveOptionDetails() {
return transitiveOptionDetails;
/** Retrieves the {@link BuildOptionDetails} containing data on this configuration's options. */
public BuildOptionDetails getBuildOptionDetails() {
return buildOptionDetails;
}

/** Returns the output directory for this build configuration. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,33 @@
import javax.annotation.Nullable;

/**
* Introspector for option details, to be used when {@code select()}-ing over the options.
* Maps build option names as they appear to the user (e.g. {@code compilation_mode}) to structured
* metadata.
*
* <p>For native options, this tracks:
* <p>For native options ({@code @Option} defined in a {@link FragmentOptions} implementation), this
* tracks:
*
* <ul>
* <li>what {@link FragmentOptions} class the option is defined in
* <li>what {@link FragmentOptions} class defines the option
* <li>the option's current value
* <li>whether it allows multiple values to be specified ({@link Option#allowMultiple}
* <li>whether it is selectable, i.e., allowed to appear in a {@code config_setting}
* </ul>
*
* <p>For Starlark options, this tracks their value in built-in Starlark-object form (post-parse,
* pre-implementation function form).
*
* <p>This is "transitive" in that it includes *all* options recognizable by a given configuration,
* including those defined in child fragments.
* <p>For Starlark options (defined in a Starlark {@code build_setting)}, this tracks their value
* in built-in Starlark-object form (post-parse, pre-implementation function form).
*/
public final class TransitiveOptionDetails implements Serializable {
public final class BuildOptionDetails {

/** Builds a {@code TransitiveOptionsDetails} for the given set of native options */
/** Builds a {@code BuildOptionDetails} for the given set of native options */
@VisibleForTesting
static TransitiveOptionDetails forOptionsForTesting(
static BuildOptionDetails forOptionsForTesting(
Iterable<? extends FragmentOptions> buildOptions) {
return forOptions(buildOptions, ImmutableMap.of());
}

/** Builds a {@code TransitiveOptionDetails} for the given set of native and Starlark options. */
static TransitiveOptionDetails forOptions(
/** Builds a {@code BuildOptionDetails} for the given set of native and Starlark options. */
static BuildOptionDetails forOptions(
Iterable<? extends FragmentOptions> buildOptions, Map<Label, Object> starlarkOptions) {
ImmutableMap.Builder<String, OptionDetails> map = ImmutableMap.builder();
try {
Expand Down Expand Up @@ -88,7 +87,7 @@ static TransitiveOptionDetails forOptions(
throw new IllegalStateException(
"Unexpected illegal access trying to create this configuration's options map: ", e);
}
return new TransitiveOptionDetails(map.build(), ImmutableMap.copyOf(starlarkOptions));
return new BuildOptionDetails(map.build(), ImmutableMap.copyOf(starlarkOptions));
}

private static final class OptionDetails implements Serializable {
Expand Down Expand Up @@ -130,15 +129,15 @@ private OptionDetails(
* <li>Parse alternative values for the option.
* </ol>
*/
private final ImmutableMap<String, OptionDetails> transitiveNativeOptionsMap;
private final ImmutableMap<String, OptionDetails> nativeOptionsMap;

/** Maps Starlark option labels to values */
private final ImmutableMap<Label, Object> starlarkOptionsMap;

private TransitiveOptionDetails(
ImmutableMap<String, OptionDetails> transitiveNativeOptionsMap,
private BuildOptionDetails(
ImmutableMap<String, OptionDetails> nativeOptionsMap,
ImmutableMap<Label, Object> starlarkOptionsMap) {
this.transitiveNativeOptionsMap = transitiveNativeOptionsMap;
this.nativeOptionsMap = nativeOptionsMap;
this.starlarkOptionsMap = starlarkOptionsMap;
}

Expand All @@ -151,7 +150,7 @@ private TransitiveOptionDetails(
*/
@Nullable
public Class<? extends FragmentOptions> getOptionClass(String optionName) {
OptionDetails optionDetails = transitiveNativeOptionsMap.get(optionName);
OptionDetails optionDetails = nativeOptionsMap.get(optionName);
return optionDetails == null ? null : optionDetails.optionsClass;
}

Expand All @@ -165,7 +164,7 @@ public Class<? extends FragmentOptions> getOptionClass(String optionName) {
*/
@Nullable
public Object getOptionValue(String optionName) {
OptionDetails optionDetails = transitiveNativeOptionsMap.get(optionName);
OptionDetails optionDetails = nativeOptionsMap.get(optionName);
return (optionDetails == null) ? null : optionDetails.value;
}

Expand All @@ -184,7 +183,7 @@ public Object getOptionValue(Label optionName) {
* to be of type {@code List<T>}.
*/
public boolean allowsMultipleValues(String optionName) {
OptionDetails optionDetails = transitiveNativeOptionsMap.get(optionName);
OptionDetails optionDetails = nativeOptionsMap.get(optionName);
return optionDetails != null && optionDetails.allowsMultiple;
}

Expand All @@ -195,7 +194,7 @@ public boolean allowsMultipleValues(String optionName) {
*/
@Nullable
public SelectRestriction getSelectRestriction(String optionName) {
OptionDetails optionDetails = transitiveNativeOptionsMap.get(optionName);
OptionDetails optionDetails = nativeOptionsMap.get(optionName);
return optionDetails == null ? null : optionDetails.selectRestriction;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public static RequiredConfigFragmentsProvider getRuleRequiredFragmentsIfEnabled(
.addRuleImplSpecificRequiredConfigFragments(requiredFragments, attributes, configuration);
}
addRequiredFragmentsFromRuleTransitions(
requiredFragments, target, attributes, configuration.getTransitiveOptionDetails());
requiredFragments, target, attributes, configuration.getBuildOptionDetails());

// We consider build settings (which are both targets and configuration) to require themselves.
if (target.isBuildSetting()) {
Expand Down Expand Up @@ -157,7 +157,7 @@ public static RequiredConfigFragmentsProvider getAspectRequiredFragmentsIfEnable
requiredFragments,
aspect,
ConfiguredAttributeMapper.of(associatedTarget, configConditions, configuration.checksum()),
configuration.getTransitiveOptionDetails());
configuration.getBuildOptionDetails());
return requiredFragments.build();
}

Expand Down Expand Up @@ -212,7 +212,7 @@ private static void addRequiredFragmentsFromRuleTransitions(
RequiredConfigFragmentsProvider.Builder requiredFragments,
Rule target,
ConfiguredAttributeMapper attributeMap,
TransitiveOptionDetails optionDetails) {
BuildOptionDetails optionDetails) {
if (target.getRuleClassObject().getTransitionFactory() != null) {
target
.getRuleClassObject()
Expand Down Expand Up @@ -245,7 +245,7 @@ private static void addRequiredFragmentsFromAspectTransitions(
RequiredConfigFragmentsProvider.Builder requiredFragments,
Aspect aspect,
ConfiguredAttributeMapper attributeMap,
TransitiveOptionDetails optionDetails) {
BuildOptionDetails optionDetails) {
AttributeTransitionData attributeTransitionData =
AttributeTransitionData.builder().attributes(attributeMap).build();
for (Attribute attribute : aspect.getDefinition().getAttributes().values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.config.BuildOptionDetails;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -50,8 +50,7 @@ public final class ComposingTransition implements ConfigurationTransition {

@Override
public void addRequiredFragments(
RequiredConfigFragmentsProvider.Builder requiredFragments,
TransitiveOptionDetails optionDetails) {
RequiredConfigFragmentsProvider.Builder requiredFragments, BuildOptionDetails optionDetails) {
// At first glance this code looks wrong. A composing transition applies transition2 over
// transition1's outputs, not the original options. We don't have to worry about that here
// because the reason we pass the options is so Starlark transitions can map individual flags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@

import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.config.BuildOptionDetails;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Map;

Expand Down Expand Up @@ -47,15 +47,14 @@ default ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments()
* Adds required configuration fragments to the given {@link
* RequiredConfigFragmentsProvider.Builder}.
*
* <p>A {@link TransitiveOptionDetails} instance is provided for Starlark transitions, which need
* to map required options to their {@link FragmentOptions}.
* <p>A {@link BuildOptionDetails} instance is provided for Starlark transitions, which need to
* map required options to their {@link FragmentOptions}.
*
* <p>Non-Starlark transitions should override {@link #requiresOptionFragments} and keep the
* default implementation of this method.
*/
default void addRequiredFragments(
RequiredConfigFragmentsProvider.Builder requiredFragments,
TransitiveOptionDetails optionDetails) {
RequiredConfigFragmentsProvider.Builder requiredFragments, BuildOptionDetails optionDetails) {
requiredFragments.addOptionsClasses(requiresOptionFragments());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ static ImmutableMap<String, BuildOptions> applyAndValidate(
checkForDenylistedOptions(starlarkTransition);

// TODO(waltl): Consider building this once and using it across different split transitions,
// or reusing TransitiveOptionDetails.
// or reusing BuildOptionDetails.
Map<String, OptionInfo> optionInfoMap = buildOptionInfo(buildOptions);
Dict<String, Object> settings =
buildSettings(buildOptions, optionInfoMap, starlarkTransition);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.config.BuildOptionDetails;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.Settings;
import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
Expand Down Expand Up @@ -84,8 +84,7 @@ private ImmutableList<String> getOutputs() {

@Override
public void addRequiredFragments(
RequiredConfigFragmentsProvider.Builder requiredFragments,
TransitiveOptionDetails optionDetails) {
RequiredConfigFragmentsProvider.Builder requiredFragments, BuildOptionDetails optionDetails) {
for (String optionStarlarkName : Iterables.concat(getInputs(), getOutputs())) {
if (!optionStarlarkName.startsWith(COMMAND_LINE_OPTION_PREFIX)) {
requiredFragments.addStarlarkOption(Label.parseAbsoluteUnchecked(optionStarlarkName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:build_setting_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_option_details",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_option_converters",
Expand All @@ -40,7 +41,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/starlark_exposed_rule_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitive_option_details",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
Expand Down
Loading

0 comments on commit a9206eb

Please sign in to comment.