Skip to content

Commit

Permalink
Add the parents attribute to the platform rule.
Browse files Browse the repository at this point in the history 
Fixes bazelbuild#6218.

Change-Id: I7dcf430fbcf745654058113cf68df18a88e44c94
  • Loading branch information
@katre
katre committed Oct 23, 2018
1 parent 37ef973 commit 34c4fc4
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 1 deletion.
15 changes: 15 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package com.google.devtools.build.lib.rules.platform;

import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
Expand All @@ -33,6 +35,7 @@
import com.google.devtools.build.lib.util.CPU;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.Pair;
import java.util.List;

/** Defines a platform for execution contexts. */
public class Platform implements RuleConfiguredTargetFactory {
Expand All @@ -42,6 +45,18 @@ public ConfiguredTarget create(RuleContext ruleContext)

PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel());

List<PlatformInfo> parentPlatforms =
Lists.newArrayList(
PlatformProviderUtils.platforms(
ruleContext.getPrerequisites(PlatformRule.PARENTS_PLATFORM_ATTR, Mode.DONT_CHECK)));

if (parentPlatforms.size() > 1) {
throw ruleContext.throwWithAttributeError(
PlatformRule.PARENTS_PLATFORM_ATTR, "parents attribute must have a single value");
}
PlatformInfo parentPlatform = Iterables.getFirst(parentPlatforms, null);
platformBuilder.setParent(parentPlatform);

Boolean isHostPlatform =
ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN);
Boolean isTargetPlatform =
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.syntax.Type;
Expand All @@ -29,6 +30,7 @@
public class PlatformRule implements RuleDefinition {
public static final String RULE_NAME = "platform";
public static final String CONSTRAINT_VALUES_ATTR = "constraint_values";
public static final String PARENTS_PLATFORM_ATTR = "parents";
public static final String REMOTE_EXECUTION_PROPS_ATTR = "remote_execution_properties";
static final String HOST_PLATFORM_ATTR = "host_platform";
static final String TARGET_PLATFORM_ATTR = "target_platform";
Expand All @@ -52,14 +54,24 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
.add(
attr(CONSTRAINT_VALUES_ATTR, BuildType.LABEL_LIST)
.allowedFileTypes(FileTypeSet.NO_FILE)
.mandatoryProviders(ImmutableList.of(ConstraintValueInfo.PROVIDER.id())))
.mandatoryProviders(ConstraintValueInfo.PROVIDER.id()))

/* <!-- #BLAZE_RULE(platform).ATTRIBUTE(remote_execution_properties) -->
A string used to configure a remote execution platform. Actual builds make no attempt to
interpret this, it is treated as opaque data that can be used by a specific SpawnRunner.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr(REMOTE_EXECUTION_PROPS_ATTR, Type.STRING))

/* <!-- #BLAZE_RULE(platform).ATTRIBUTE(parents) -->
The label of a <code>platform</code> target that this platform should inherit from. Although
the attribute takes a list, there should be no more than one platform present. Any
constraint_settings not set directly on this platform will be found in the parent platform.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr(PARENTS_PLATFORM_ATTR, BuildType.LABEL_LIST)
.allowedFileTypes(FileTypeSet.NO_FILE)
.mandatoryProviders(PlatformInfo.PROVIDER.id()))

// Undocumented. Indicates that this platform should auto-configure the platform constraints
// based on the current host OS and CPU settings.
.add(
Expand Down
105 changes: 105 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,101 @@ public void testPlatform_remoteExecution() throws Exception {
assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("foo: val1");
}

@Test
public void testPlatform_parent() throws Exception {
constraintBuilder("//foo:setting1").addConstraintValue("value1").write();
constraintBuilder("//foo:setting2").addConstraintValue("value2").write();
platformBuilder("//foo:parent_platform").addConstraint("value1").write();
platformBuilder("//foo:my_platform")
.setParent("//foo:parent_platform")
.addConstraint("value2")
.write();
assertNoEvents();

PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform");
assertThat(platformInfo).isNotNull();
ConstraintSettingInfo constraintSetting1 =
ConstraintSettingInfo.create(makeLabel("//foo:setting1"));
ConstraintValueInfo constraintValue1 =
ConstraintValueInfo.create(constraintSetting1, makeLabel("//foo:value1"));
assertThat(platformInfo.constraints().get(constraintSetting1)).isEqualTo(constraintValue1);
ConstraintSettingInfo constraintSetting2 =
ConstraintSettingInfo.create(makeLabel("//foo:setting2"));
ConstraintValueInfo constraintValue2 =
ConstraintValueInfo.create(constraintSetting2, makeLabel("//foo:value2"));
assertThat(platformInfo.constraints().get(constraintSetting2)).isEqualTo(constraintValue2);
}

@Test
public void testPlatform_parent_override() throws Exception {
constraintBuilder("//foo:setting1")
.addConstraintValue("value1a")
.addConstraintValue("value1b")
.write();
platformBuilder("//foo:parent_platform").addConstraint("value1a").write();
platformBuilder("//foo:my_platform").addConstraint("value1b").write();
assertNoEvents();

PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform");
assertThat(platformInfo).isNotNull();
ConstraintSettingInfo constraintSetting1 =
ConstraintSettingInfo.create(makeLabel("//foo:setting1"));
ConstraintValueInfo constraintValue1 =
ConstraintValueInfo.create(constraintSetting1, makeLabel("//foo:value1b"));
assertThat(platformInfo.constraints().get(constraintSetting1)).isEqualTo(constraintValue1);
}

@Test
public void testPlatform_parent_remoteExecProperties_entireOverride() throws Exception {
platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("parent props").write();
platformBuilder("//foo:my_platform")
.setParent("//foo:parent_platform")
.setRemoteExecutionProperties("child props")
.write();
assertNoEvents();

PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform");
assertThat(platformInfo).isNotNull();
assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("child props");
}

@Test
public void testPlatform_parent_remoteExecProperties_includeParent() throws Exception {
platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("parent props").write();
platformBuilder("//foo:my_platform")
.setParent("//foo:parent_platform")
.setRemoteExecutionProperties("child ({PARENT_REMOTE_EXECUTION_PROPERTIES}) props")
.write();
assertNoEvents();

PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform");
assertThat(platformInfo).isNotNull();
assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("child (parent props) props");
}

@Test
public void testPlatform_parent_tooManyParentsError() throws Exception {
List<String> lines =
new ImmutableList.Builder<String>()
.addAll(platformBuilder("//foo:parent_platform1").lines())
.addAll(platformBuilder("//foo:parent_platform2").lines())
.addAll(
ImmutableList.of(
"platform(name = 'my_platform',\n",
" parents = [\n",
" ':parent_platform1',\n",
" ':parent_platform2',\n",
" ])"))
.build();

checkError(
"foo",
"my_platform",
"in parents attribute of platform rule //foo:my_platform: "
+ "parents attribute must have a single value",
lines.toArray(new String[] {}));
}

ConstraintBuilder constraintBuilder(String name) {
return new ConstraintBuilder(name);
}
Expand Down Expand Up @@ -146,12 +241,18 @@ PlatformBuilder platformBuilder(String name) {
final class PlatformBuilder {
private final Label label;
private final List<String> constraintValues = new ArrayList<>();
private Label parentLabel = null;
private String remoteExecutionProperties = "";

public PlatformBuilder(String name) {
this.label = Label.parseAbsoluteUnchecked(name);
}

public PlatformBuilder setParent(String parentLabel) {
this.parentLabel = Label.parseAbsoluteUnchecked(parentLabel);
return this;
}

public PlatformBuilder addConstraint(String value) {
this.constraintValues.add(value);
return this;
Expand All @@ -166,6 +267,9 @@ public List<String> lines() {
ImmutableList.Builder<String> lines = ImmutableList.builder();

lines.add("platform(", " name = '" + label.getName() + "',");
if (parentLabel != null) {
lines.add(" parents = ['" + parentLabel + "'],");
}
lines.add(" constraint_values = [");
for (String name : constraintValues) {
lines.add(" ':" + name + "',");
Expand All @@ -184,6 +288,7 @@ public void write() throws Exception {
String filename = label.getPackageFragment().getRelative("BUILD").getPathString();
scratch.appendFile(filename, lines.toArray(new String[] {}));
}

}

@Nullable
Expand Down

0 comments on commit 34c4fc4

Please sign in to comment.