Skip to content

Commit

Permalink
Improve the name of supportsPlatforms by renaming to useToolchainReso…
Browse files Browse the repository at this point in the history
…lution.

PiperOrigin-RevId: 253632079
  • Loading branch information
katre authored and siberex committed Jul 4, 2019
1 parent ce9b88b commit 7da70da
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
<code>data</code></a> for more information about how to depend on and use data files.
</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
.add(
attr("data", LABEL_LIST)
.allowedFileTypes(FileTypeSet.ANY_FILE)
.dontCheckConstraints())
.add(attr("data", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE).dontCheckConstraints())
.add(attr("output_licenses", LICENSE))
/*<!-- #BLAZE_RULE(filegroup).ATTRIBUTE(path) -->
An optional string to set a path to the files in the group, relative to the package path.
Expand All @@ -81,7 +78,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
attr("path", STRING)
.undocumented(
"only used to expose FilegroupPathProvider, which is not currently used"))
.supportsPlatforms(false)
.useToolchainResolution(false)
.build();
}

Expand Down
24 changes: 12 additions & 12 deletions src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ public enum ThirdPartyLicenseExistencePolicy {

private final Map<String, Attribute> attributes = new LinkedHashMap<>();
private final Set<Label> requiredToolchains = new HashSet<>();
private boolean supportsPlatforms = true;
private boolean useToolchainResolution = true;
private ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed =
ExecutionPlatformConstraintsAllowed.PER_RULE;
private Set<Label> executionPlatformConstraints = new HashSet<>();
Expand Down Expand Up @@ -761,7 +761,7 @@ public Builder(String name, RuleClassType type, boolean skylark, RuleClass... pa
supportsConstraintChecking = parent.supportsConstraintChecking;

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

// Make sure we use the highest priority value from all parents.
executionPlatformConstraintsAllowed(
Expand Down Expand Up @@ -885,7 +885,7 @@ public RuleClass build(String name, String key) {
supportsConstraintChecking,
thirdPartyLicenseExistencePolicy,
requiredToolchains,
supportsPlatforms,
useToolchainResolution,
executionPlatformConstraintsAllowed,
executionPlatformConstraints,
outputFileKind,
Expand Down Expand Up @@ -1387,11 +1387,11 @@ public Builder addRequiredToolchains(Label... toolchainLabels) {
}

/**
* Rules that support platforms can use toolchains and execution platforms. Rules that are part
* of configuring toolchains and platforms should set this to {@code false}.
* Causes rules to use toolchain resolution to determine the execution platform and toolchains.
* Rules that are part of configuring toolchains and platforms should set this to {@code false}.
*/
public Builder supportsPlatforms(boolean flag) {
this.supportsPlatforms = flag;
public Builder useToolchainResolution(boolean flag) {
this.useToolchainResolution = flag;
return this;
}

Expand Down Expand Up @@ -1556,7 +1556,7 @@ public Attribute.Builder<?> copy(String name) {
private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

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

Expand Down Expand Up @@ -1611,7 +1611,7 @@ public Attribute.Builder<?> copy(String name) {
boolean supportsConstraintChecking,
ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy,
Set<Label> requiredToolchains,
boolean supportsPlatforms,
boolean useToolchainResolution,
ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed,
Set<Label> executionPlatformConstraints,
OutputFile.Kind outputFileKind,
Expand Down Expand Up @@ -1650,7 +1650,7 @@ public Attribute.Builder<?> copy(String name) {
this.supportsConstraintChecking = supportsConstraintChecking;
this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy;
this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains);
this.supportsPlatforms = supportsPlatforms;
this.useToolchainResolution = useToolchainResolution;
this.executionPlatformConstraintsAllowed = executionPlatformConstraintsAllowed;
this.executionPlatformConstraints = ImmutableSet.copyOf(executionPlatformConstraints);
this.buildSetting = buildSetting;
Expand Down Expand Up @@ -2559,8 +2559,8 @@ public ImmutableSet<Label> getRequiredToolchains() {
return requiredToolchains;
}

public boolean supportsPlatforms() {
return supportsPlatforms;
public boolean useToolchainResolution() {
return useToolchainResolution;
}

public ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public static ToolchainContext getToolchainContext(
}

Rule rule = ((Rule) target);
if (!rule.getRuleClassObject().supportsPlatforms()) {
if (!rule.getRuleClassObject().useToolchainResolution()) {
return null;
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/google/devtools/build/lib/rules/Alias.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
.mandatory())
.canHaveAnyProvider()
// Aliases themselves do not need toolchains or an execution platform, so this is fine.
// The actual target
// will resolve platforms and toolchains with no issues regardless of this setting.
.supportsPlatforms(false)
// The actual target will resolve platforms and toolchains with no issues regardless of
// this setting.
.useToolchainResolution(false)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private static RuleClass buildRuleClass(RuleClass.Builder builder, boolean flag)
.add(attr(":alias", LABEL).value(ACTUAL))
.setBuildSetting(new BuildSetting(flag, LABEL))
.canHaveAnyProvider()
.supportsPlatforms(false)
.useToolchainResolution(false)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static class ToolchainTypeRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
return builder
.supportsPlatforms(false)
.useToolchainResolution(false)
.removeAttribute("licenses")
.removeAttribute("distribs")
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
.value(ImmutableList.of("manual"))
.nonconfigurable("low-level attribute, used in platform configuration"))
.exemptFromConstraintChecking("this rule helps *define* a constraint")
.supportsPlatforms(false)
.useToolchainResolution(false)
.removeAttribute("deps")
.removeAttribute("data")
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
.removeAttribute("deps")
.removeAttribute("data")
.exemptFromConstraintChecking("this rule *defines* a constraint")
.supportsPlatforms(false)
.useToolchainResolution(false)

/* <!-- #BLAZE_RULE(toolchain).ATTRIBUTE(toolchain_type) -->
The label of a <code>toolchain_type</code> target that represents the role that this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ private UnloadedToolchainContext computeUnloadedToolchainContext(
return null;
}
Rule rule = ((Rule) targetAndConfig.getTarget());
if (!rule.getRuleClassObject().supportsPlatforms()) {
if (!rule.getRuleClassObject().useToolchainResolution()) {
return null;
}
BuildConfiguration configuration = targetAndConfig.getConfiguration();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
try {
if (configuredTargetAndData.getTarget() instanceof Rule) {
Rule rule = ((Rule) configuredTargetAndData.getTarget());
if (rule.getRuleClassObject().supportsPlatforms()) {
if (rule.getRuleClassObject().useToolchainResolution()) {
ImmutableSet<Label> requiredToolchains =
rule.getRuleClassObject().getRequiredToolchains();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,9 @@ public boolean shouldRetain(Target target, boolean explicit) {
return true;
}

// If the rule requires platforms, it can't be used as a platform.
// If the rule requires platforms or toolchain resolution, it can't be used as a platform.
RuleClass ruleClass = target.getAssociatedRule().getRuleClassObject();
if (ruleClass == null) {
return false;
}

if (ruleClass.supportsPlatforms()) {
if (ruleClass == null || ruleClass.useToolchainResolution()) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ private static RuleClass newRuleClass(
supportsConstraintChecking,
ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE,
/*requiredToolchains=*/ ImmutableSet.of(),
/*supportsPlatforms=*/ true,
/*useToolchainResolution=*/ true,
ExecutionPlatformConstraintsAllowed.PER_RULE,
/* executionPlatformConstraints= */ ImmutableSet.of(),
OutputFile.Kind.FILE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public static void installFragmentsAndNativeRules(
.add(
attr("deps", BuildType.LABEL_LIST)
.allowedFileTypes(FileTypeSet.ANY_FILE))
.supportsPlatforms(false)
.useToolchainResolution(false)
.setImplicitOutputsFunction(
ImplicitOutputsFunction.fromTemplates("%{name}.np"));
});
Expand All @@ -359,7 +359,7 @@ public static void installFragmentsAndNativeRules(
.add(
attr("deps", BuildType.LABEL_LIST)
.allowedFileTypes(FileTypeSet.ANY_FILE))
.supportsPlatforms(true)
.useToolchainResolution(true)
.setImplicitOutputsFunction(
ImplicitOutputsFunction.fromTemplates("%{name}.p"));
});
Expand Down Expand Up @@ -391,7 +391,7 @@ public static void installFragmentsAndNativeRules(
.add(
attr("deps", BuildType.LABEL_LIST)
.allowedFileTypes(FileTypeSet.ANY_FILE))
.supportsPlatforms(true)
.useToolchainResolution(true)
.addRequiredToolchains(toolchainTypeLabel)
.setImplicitOutputsFunction(
ImplicitOutputsFunction.fromTemplates("%{name}.u"));
Expand Down

0 comments on commit 7da70da

Please sign in to comment.