Skip to content

Commit

Permalink
Integrate starlark options with select() via the flag_values attribut…
Browse files Browse the repository at this point in the history
…e of config_setting.

This includes adding an implicit provider to every build setting rule that knows its type and its default value.

PiperOrigin-RevId: 247508938
  • Loading branch information
juliexxia authored and copybara-github committed May 9, 2019
1 parent 85a5a2b commit b677702
Show file tree
Hide file tree
Showing 9 changed files with 337 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2019 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.analysis;

import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.packages.RequiredProviders;
import com.google.devtools.build.lib.syntax.Type;

/**
* A native provider to allow select()s to know the type and default value when selecting on build
* settings
*/
public class BuildSettingProvider implements TransitiveInfoProvider {

public static final RequiredProviders REQUIRE_BUILD_SETTING_PROVIDER =
RequiredProviders.acceptAnyBuilder()
.addNativeSet(ImmutableSet.of(BuildSettingProvider.class))
.build();

private final BuildSetting buildSetting;
private final Object defaultValue;

public BuildSettingProvider(BuildSetting buildSetting, Object defaultValue) {
this.buildSetting = buildSetting;
this.defaultValue = defaultValue;
}

public Type<?> getType() {
return buildSetting.getType();
}

public Object getDefaultValue() {
return defaultValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;

import static com.google.devtools.build.lib.analysis.ExtraActionUtils.createExtraActionProvider;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -44,6 +45,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.packages.InfoInterface;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.packages.Provider;
Expand Down Expand Up @@ -184,6 +186,15 @@ public ConfiguredTarget build() throws ActionConflictException {
}
}

if (ruleContext.getRule().isBuildSetting()) {
BuildSetting buildSetting = ruleContext.getRule().getRuleClassObject().getBuildSetting();
Object defaultValue =
ruleContext
.attributes()
.get(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, buildSetting.getType());
addProvider(BuildSettingProvider.class, new BuildSettingProvider(buildSetting, defaultValue));
}

TransitiveInfoProviderMap providers = providersBuilder.build();

if (ruleContext.getRule().isAnalysisTest()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public List<String> getInputs() {
public List<String> getOutputs() {
return outputs;
}


/**
* Returns the location of the Starlark code responsible for determining the transition's changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class FunctionPatchTransition extends StarlarkTransition implements PatchTransit
FunctionPatchTransition(
StarlarkDefinedConfigTransition starlarkDefinedConfigTransition, Rule rule) {
super(starlarkDefinedConfigTransition);

LinkedHashMap<String, Object> attributes = new LinkedHashMap<>();
RawAttributeMapper attributeMapper = RawAttributeMapper.of(rule);
for (Attribute attribute : rule.getAttributes()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ public boolean hasAnalysisTestTransition() {
return ruleClass.hasAnalysisTestTransition();
}

public boolean isBuildSetting() {
return ruleClass.getBuildSetting() != null;
}

/**
* Returns true iff there were errors while constructing this rule, such as
* attributes with missing values or values of the wrong type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.packages.RequiredProviders;
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigFeatureFlagProviderApi;
import com.google.devtools.build.lib.syntax.Environment;
Expand All @@ -37,6 +39,9 @@ public class ConfigFeatureFlagProvider extends NativeInfo implements ConfigFeatu
/** Skylark constructor and identifier for ConfigFeatureFlagProvider. */
static final NativeProvider<ConfigFeatureFlagProvider> SKYLARK_CONSTRUCTOR = new Constructor();

static final RequiredProviders REQUIRE_CONFIG_FEATURE_FLAG_PROVIDER =
RequiredProviders.acceptAnyBuilder().addSkylarkSet(ImmutableSet.of(id())).build();

private final String value;
private final Predicate<String> validityPredicate;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ <li>Its syntax (<code>--define KEY=VAL</code>) means <code>KEY=VAL</code> is
attr(FLAG_SETTINGS_ATTRIBUTE, LABEL_KEYED_STRING_DICT)
.undocumented("the feature flag feature has not yet been launched")
.allowedFileTypes()
.mandatoryProviders(ImmutableList.of(ConfigFeatureFlagProvider.id()))
.nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
/* <!-- #BLAZE_RULE(config_setting).ATTRIBUTE(constraint_values) -->
The minimum set of <code>constraint_values</code> that the target platform must specify
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.common.collect.Multiset;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.BuildSettingProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
Expand Down Expand Up @@ -57,6 +58,7 @@
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.rules.config.ConfigRuleClasses.ConfigSettingRule;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
Expand Down Expand Up @@ -108,16 +110,17 @@ public ConfiguredTarget create(RuleContext ruleContext)
return null;
}

TransitiveOptionDetails optionDetails =
BuildConfigurationOptionDetails.get(ruleContext.getConfiguration());

boolean nativeFlagsMatch =
matchesConfig(
nativeFlagSettings.entries(),
BuildConfigurationOptionDetails.get(ruleContext.getConfiguration()),
ruleContext);
matchesConfig(nativeFlagSettings.entries(), optionDetails, ruleContext);

ConfigFeatureFlagMatch featureFlags =
ConfigFeatureFlagMatch.fromAttributeValueAndPrerequisites(
UserDefinedFlagMatch userDefinedFlags =
UserDefinedFlagMatch.fromAttributeValueAndPrerequisites(
userDefinedFlagSettings,
ruleContext.getPrerequisites(ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, Mode.TARGET),
optionDetails,
ruleContext);

boolean constraintValuesMatch = constraintValuesMatch(ruleContext);
Expand All @@ -130,8 +133,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
new ConfigMatchingProvider(
ruleContext.getLabel(),
nativeFlagSettings,
featureFlags.getSpecifiedFlagValues(),
nativeFlagsMatch && featureFlags.matches() && constraintValuesMatch);
userDefinedFlags.getSpecifiedFlagValues(),
nativeFlagsMatch && userDefinedFlags.matches() && constraintValuesMatch);

return new RuleConfiguredTargetBuilder(ruleContext)
.addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY)
Expand Down Expand Up @@ -368,14 +371,13 @@ private static boolean optionMatches(
return actualList.contains(expectedSingleValue);
}

private static final class ConfigFeatureFlagMatch {
private static final class UserDefinedFlagMatch {
private final boolean matches;
private final ImmutableMap<Label, String> specifiedFlagValues;

private static final Joiner QUOTED_COMMA_JOINER = Joiner.on("', '");

private ConfigFeatureFlagMatch(
boolean matches, ImmutableMap<Label, String> specifiedFlagValues) {
private UserDefinedFlagMatch(boolean matches, ImmutableMap<Label, String> specifiedFlagValues) {
this.matches = matches;
this.specifiedFlagValues = specifiedFlagValues;
}
Expand All @@ -386,7 +388,7 @@ public boolean matches() {
}

/** Gets the specified flag values, with aliases converted to their original targets' labels. */
public ImmutableMap<Label, String> getSpecifiedFlagValues() {
ImmutableMap<Label, String> getSpecifiedFlagValues() {
return specifiedFlagValues;
}

Expand All @@ -401,20 +403,16 @@ private static ListMultimap<Label, Label> collectAliases(
return targetsToAliases.build();
}

public static ConfigFeatureFlagMatch fromAttributeValueAndPrerequisites(
static UserDefinedFlagMatch fromAttributeValueAndPrerequisites(
Map<Label, String> attributeValue,
Iterable<? extends TransitiveInfoCollection> prerequisites,
TransitiveOptionDetails optionDetails,
RuleErrorConsumer errors) {
Map<Label, String> specifiedFlagValues = new LinkedHashMap<>();
boolean matches = true;
boolean foundDuplicate = false;

for (TransitiveInfoCollection target : prerequisites) {
ConfigFeatureFlagProvider provider = ConfigFeatureFlagProvider.fromTarget(target);
// We know the provider exists because only labels with ConfigFeatureFlagProvider can be
// added to this attribute.
assert provider != null;

Label actualLabel = target.getLabel();
Label specifiedLabel = AliasProvider.getDependencyLabel(target);
String specifiedValue = attributeValue.get(specifiedLabel);
Expand All @@ -423,17 +421,52 @@ public static ConfigFeatureFlagMatch fromAttributeValueAndPrerequisites(
}
specifiedFlagValues.put(actualLabel, specifiedValue);

if (!provider.isValidValue(specifiedValue)) {
if (target.satisfies(ConfigFeatureFlagProvider.REQUIRE_CONFIG_FEATURE_FLAG_PROVIDER)) {
// config_feature_flag
ConfigFeatureFlagProvider provider = ConfigFeatureFlagProvider.fromTarget(target);
if (!provider.isValidValue(specifiedValue)) {
errors.attributeError(
ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
String.format(
"error while parsing user-defined configuration values: "
+ "'%s' is not a valid value for '%s'",
specifiedValue, specifiedLabel));
matches = false;
continue;
}
if (!provider.getFlagValue().equals(specifiedValue)) {
matches = false;
}
} else if (target.satisfies(BuildSettingProvider.REQUIRE_BUILD_SETTING_PROVIDER)) {
// build setting
BuildSettingProvider provider = target.getProvider(BuildSettingProvider.class);
Object configurationValue =
optionDetails.getOptionValue(specifiedLabel) != null
? optionDetails.getOptionValue(specifiedLabel)
: provider.getDefaultValue();
Object convertedSpecifiedValue;
try {
convertedSpecifiedValue = provider.getType().convert(specifiedValue, specifiedLabel);
} catch (ConversionException e) {
errors.attributeError(
ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
String.format(
"error while parsing user-defined configuration values: "
+ "'%s' cannot be converted to %s type %s",
specifiedValue, specifiedLabel, provider.getType()));
matches = false;
continue;
}
if (!configurationValue.equals(convertedSpecifiedValue)) {
matches = false;
}
} else {
errors.attributeError(
ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
String.format(
"error while parsing user-defined configuration values: "
+ "'%s' is not a valid value for '%s'",
specifiedValue, specifiedLabel));
matches = false;
continue;
}
if (!provider.getFlagValue().equals(specifiedValue)) {
+ "%s keys must be build settings or feature flags and %s is not",
ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, specifiedLabel));
matches = false;
}
}
Expand All @@ -454,11 +487,10 @@ public static ConfigFeatureFlagMatch fromAttributeValueAndPrerequisites(
actualLabel, QUOTED_COMMA_JOINER.join(aliasList)));
}
}

matches = false;
}

return new ConfigFeatureFlagMatch(matches, ImmutableMap.copyOf(specifiedFlagValues));
return new UserDefinedFlagMatch(matches, ImmutableMap.copyOf(specifiedFlagValues));
}
}
}
Loading

0 comments on commit b677702

Please sign in to comment.