From 34c4fc420e9391a0eff225e9a13f18149d12403b Mon Sep 17 00:00:00 2001 From: John Cater Date: Mon, 8 Oct 2018 16:17:00 -0400 Subject: [PATCH] Add the parents attribute to the platform rule. Fixes #6218. Change-Id: I7dcf430fbcf745654058113cf68df18a88e44c94 --- .../build/lib/rules/platform/Platform.java | 15 +++ .../lib/rules/platform/PlatformRule.java | 14 ++- .../rules/platform/PlatformInfoApiTest.java | 105 ++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java index 39c491fef88949..62b5734b40f660 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java @@ -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; @@ -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 { @@ -42,6 +45,18 @@ public ConfiguredTarget create(RuleContext ruleContext) PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel()); + List 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 = diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java b/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java index 032b10aac3f541..63980e87de3048 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/PlatformRule.java @@ -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; @@ -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"; @@ -52,7 +54,7 @@ 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())) /* A string used to configure a remote execution platform. Actual builds make no attempt to @@ -60,6 +62,16 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) */ .add(attr(REMOTE_EXECUTION_PROPS_ATTR, Type.STRING)) + /* + The label of a platform 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. + */ + .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( diff --git a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java index 2e8b2498af8a58..be309a6bbd2a3f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java @@ -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 lines = + new ImmutableList.Builder() + .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); } @@ -146,12 +241,18 @@ PlatformBuilder platformBuilder(String name) { final class PlatformBuilder { private final Label label; private final List 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; @@ -166,6 +267,9 @@ public List lines() { ImmutableList.Builder 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 + "',"); @@ -184,6 +288,7 @@ public void write() throws Exception { String filename = label.getPackageFragment().getRelative("BUILD").getPathString(); scratch.appendFile(filename, lines.toArray(new String[] {})); } + } @Nullable