Skip to content

Commit

Permalink
Remove the ExecutionPlatformConstraintsAllowed enum entirely.
Browse files Browse the repository at this point in the history
  • Loading branch information
katre committed Nov 6, 2019
1 parent e4eadec commit aa213e3
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 213 deletions.
Expand Up @@ -40,9 +40,9 @@
import com.google.devtools.build.lib.packages.Attribute.LabelListLateBoundDefault;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault.Resolver;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed;
import com.google.devtools.build.lib.packages.TestSize;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
Expand Down Expand Up @@ -423,8 +423,12 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
attr("data", LABEL_LIST)
.allowedFileTypes(FileTypeSet.ANY_FILE)
.dontCheckConstraints())
.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET)
.add(attr(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT).value(ImmutableMap.of()))
.add(
attr(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)
.allowedFileTypes()
.nonconfigurable("Used in toolchain resolution")
.value(ImmutableList.of()))
.build();
}

Expand Down
Expand Up @@ -63,7 +63,6 @@
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed;
import com.google.devtools.build.lib.packages.RuleFactory;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
Expand Down Expand Up @@ -139,6 +138,11 @@ public Label load(String from) throws Exception {
.mandatoryProviders(ImmutableList.of(TemplateVariableInfo.PROVIDER.id()))
.dontCheckConstraints())
.add(attr(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT).value(ImmutableMap.of()))
.add(
attr(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)
.allowedFileTypes()
.nonconfigurable("Used in toolchain resolution")
.value(ImmutableList.of()))
.build();

/** Parent rule class for executable non-test Skylark rules. */
Expand Down Expand Up @@ -227,7 +231,6 @@ public static final RuleClass getTestBaseRule(RuleDefinitionContext env) {
toolsRepository
+ BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))))
.add(attr(":run_under", LABEL).value(RUN_UNDER))
.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET)
.build();
}

Expand Down Expand Up @@ -404,8 +407,6 @@ public BaseFunction rule(
bazelContext.getRepoMapping()));
}

builder.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET);

return new SkylarkRuleFunction(builder, type, attributes, ast.getLocation());
}

Expand Down
Expand Up @@ -220,44 +220,6 @@ public RuleErrorException(String message) {
}
}

/**
* Describes in which way a rule implementation allows additional execution platform constraints.
*/
public enum ExecutionPlatformConstraintsAllowed {
/**
* Allows additional execution platform constraints to be added in the rule definition, which
* apply to all targets of that rule.
*/
PER_RULE(1),
/**
* Users are allowed to specify additional execution platform constraints for each target, using
* the 'exec_compatible_with' attribute. This also allows setting constraints in the rule
* definition, like PER_RULE.
*/
PER_TARGET(2);

private final int priority;

ExecutionPlatformConstraintsAllowed(int priority) {
this.priority = priority;
}

public int priority() {
return priority;
}

public static ExecutionPlatformConstraintsAllowed highestPriority(
ExecutionPlatformConstraintsAllowed first, ExecutionPlatformConstraintsAllowed... rest) {
ExecutionPlatformConstraintsAllowed result = first;
for (ExecutionPlatformConstraintsAllowed value : rest) {
if (result == null || result.priority() < value.priority()) {
result = value;
}
}
return result;
}
}

/**
* For Bazel's constraint system: the attribute that declares the set of environments a rule
* supports, overriding the defaults for their respective groups.
Expand Down Expand Up @@ -724,8 +686,6 @@ public enum ThirdPartyLicenseExistencePolicy {
private final Map<String, Attribute> attributes = new LinkedHashMap<>();
private final Set<Label> requiredToolchains = new HashSet<>();
private boolean useToolchainResolution = true;
private ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed =
ExecutionPlatformConstraintsAllowed.PER_RULE;
private Set<Label> executionPlatformConstraints = new HashSet<>();
private OutputFile.Kind outputFileKind = OutputFile.Kind.FILE;

Expand Down Expand Up @@ -760,11 +720,6 @@ public Builder(String name, RuleClassType type, boolean skylark, RuleClass... pa

addRequiredToolchains(parent.getRequiredToolchains());
useToolchainResolution = parent.useToolchainResolution;

// Make sure we use the highest priority value from all parents.
executionPlatformConstraintsAllowed(
ExecutionPlatformConstraintsAllowed.highestPriority(
executionPlatformConstraintsAllowed, parent.executionPlatformConstraintsAllowed()));
addExecutionPlatformConstraints(parent.getExecutionPlatformConstraints());

for (Attribute attribute : parent.getAttributes()) {
Expand Down Expand Up @@ -832,14 +787,7 @@ public RuleClass build(String name, String key) {
if (type == RuleClassType.PLACEHOLDER) {
Preconditions.checkNotNull(ruleDefinitionEnvironmentHashCode, this.name);
}
if (executionPlatformConstraintsAllowed == ExecutionPlatformConstraintsAllowed.PER_TARGET
&& !this.contains(EXEC_COMPATIBLE_WITH_ATTR)) {
this.add(
attr(EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)
.allowedFileTypes()
.nonconfigurable("Used in toolchain resolution")
.value(ImmutableList.of()));
}

if (buildSetting != null) {
Type<?> type = buildSetting.getType();
Attribute.Builder<?> attrBuilder =
Expand Down Expand Up @@ -888,7 +836,6 @@ public RuleClass build(String name, String key) {
thirdPartyLicenseExistencePolicy,
requiredToolchains,
useToolchainResolution,
executionPlatformConstraintsAllowed,
executionPlatformConstraints,
outputFileKind,
attributes.values(),
Expand Down Expand Up @@ -1397,20 +1344,6 @@ public Builder useToolchainResolution(boolean flag) {
return this;
}

/**
* Specifies whether targets of this rule can add additional constraints on the execution
* platform selected. If this is {@link ExecutionPlatformConstraintsAllowed#PER_TARGET}, there
* will be an attribute named {@code exec_compatible_with} that can be used to add these
* constraints.
*
* <p>Please note that this value is not inherited by child rules, and must be re-set on them if
* the same behavior is required.
*/
public Builder executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed value) {
this.executionPlatformConstraintsAllowed = value;
return this;
}

/**
* Adds additional execution platform constraints that apply for all targets from this rule.
*
Expand Down Expand Up @@ -1559,7 +1492,6 @@ public Attribute.Builder<?> copy(String name) {

private final ImmutableSet<Label> requiredToolchains;
private final boolean useToolchainResolution;
private final ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed;
private final ImmutableSet<Label> executionPlatformConstraints;

/**
Expand Down Expand Up @@ -1614,7 +1546,6 @@ public Attribute.Builder<?> copy(String name) {
ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy,
Set<Label> requiredToolchains,
boolean useToolchainResolution,
ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed,
Set<Label> executionPlatformConstraints,
OutputFile.Kind outputFileKind,
Collection<Attribute> attributes,
Expand Down Expand Up @@ -1653,7 +1584,6 @@ public Attribute.Builder<?> copy(String name) {
this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy;
this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains);
this.useToolchainResolution = useToolchainResolution;
this.executionPlatformConstraintsAllowed = executionPlatformConstraintsAllowed;
this.executionPlatformConstraints = ImmutableSet.copyOf(executionPlatformConstraints);
this.buildSetting = buildSetting;

Expand Down Expand Up @@ -2548,10 +2478,6 @@ public boolean useToolchainResolution() {
return useToolchainResolution;
}

public ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed() {
return executionPlatformConstraintsAllowed;
}

public ImmutableSet<Label> getExecutionPlatformConstraints() {
return executionPlatformConstraints;
}
Expand Down
Expand Up @@ -67,7 +67,6 @@
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
Expand Down Expand Up @@ -506,9 +505,8 @@ public static ImmutableSet<Label> getExecutionPlatformConstraints(
ImmutableSet.Builder<Label> execConstraintLabels = new ImmutableSet.Builder<>();

execConstraintLabels.addAll(rule.getRuleClassObject().getExecutionPlatformConstraints());

if (rule.getRuleClassObject().executionPlatformConstraintsAllowed()
== ExecutionPlatformConstraintsAllowed.PER_TARGET) {
if (rule.getRuleClassObject()
.hasAttr(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)) {
execConstraintLabels.addAll(
mapper.get(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST));
}
Expand Down

0 comments on commit aa213e3

Please sign in to comment.