Skip to content

Commit

Permalink
Remove the unused PlatformInfo starlark constructor.
Browse files Browse the repository at this point in the history 
Also remove the exec property fields from the Starlark API.

Work towards platform-based flags: #19409.

PiperOrigin-RevId: 568265920
Change-Id: I5a4146f93167901455c2f4d3856505840aeb64f1
  • Loading branch information
@katre @copybara-github
katre authored and copybara-github committed Sep 25, 2023
1 parent a0503eb commit e5fe228
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 293 deletions.
52 changes: 2 additions & 50 deletions src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.syntax.Location;

/** Provider for a platform, which is a group of constraints and values. */
Expand Down Expand Up @@ -66,49 +61,8 @@ public class PlatformInfo extends NativeInfo
}

/** Provider singleton constant. */
public static final BuiltinProvider<PlatformInfo> PROVIDER = new Provider();

/** Provider for {@link PlatformInfo} objects. */
private static class Provider extends BuiltinProvider<PlatformInfo>
implements PlatformInfoApi.Provider<
ConstraintSettingInfo, ConstraintValueInfo, PlatformInfo> {
private Provider() {
super(STARLARK_NAME, PlatformInfo.class);
}

@Override
public PlatformInfo platformInfo(
Label label,
Object parentUnchecked,
Sequence<?> constraintValuesUnchecked,
Object execPropertiesUnchecked,
StarlarkThread thread)
throws EvalException {
PlatformInfo.Builder builder = PlatformInfo.builder();
builder.setLabel(label);
if (parentUnchecked != Starlark.NONE) {
builder.setParent((PlatformInfo) parentUnchecked);
}
if (!constraintValuesUnchecked.isEmpty()) {
builder.addConstraints(
Sequence.cast(
constraintValuesUnchecked, ConstraintValueInfo.class, "constraint_values"));
}
if (execPropertiesUnchecked != null) {
Dict<String, String> execProperties =
Dict.noneableCast(
execPropertiesUnchecked, String.class, String.class, "exec_properties");
builder.setExecProperties(ImmutableMap.copyOf(execProperties));
}
builder.setLocation(thread.getCallerLocation());

try {
return builder.build();
} catch (DuplicateConstraintException | ExecPropertiesException e) {
throw new EvalException(e);
}
}
}
public static final BuiltinProvider<PlatformInfo> PROVIDER =
new BuiltinProvider<PlatformInfo>(STARLARK_NAME, PlatformInfo.class) {};

private final Label label;
private final ConstraintCollection constraints;
Expand Down Expand Up @@ -147,12 +101,10 @@ public ConstraintCollection constraints() {
return constraints;
}

@Override
public String remoteExecutionProperties() {
return remoteExecutionProperties;
}

@Override
public ImmutableMap<String, String> execProperties() {
return execProperties;
}
Expand Down
85 changes: 0 additions & 85 deletions src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform/PlatformInfoApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,11 @@
package com.google.devtools.build.lib.starlarkbuildapi.platform;

import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.docgen.annot.StarlarkConstructor;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
import java.util.Map;
import net.starlark.java.annot.Param;
import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.NoneType;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.StarlarkThread;

/** Info object representing data about a specific platform. */
@StarlarkBuiltin(
Expand Down Expand Up @@ -64,79 +54,4 @@ public interface PlatformInfoApi<
structField = true,
enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API)
ConstraintCollectionApi<ConstraintSettingInfoT, ConstraintValueInfoT> constraints();

@StarlarkMethod(
name = "remoteExecutionProperties",
doc = "Properties that are available for the use of remote execution.",
structField = true,
enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API)
String remoteExecutionProperties();

@StarlarkMethod(
name = "exec_properties",
doc = "Properties to configure a remote execution platform.",
structField = true,
enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API)
Map<String, String> execProperties();

/** Provider for {@link PlatformInfoApi} objects. */
@StarlarkBuiltin(name = "Provider", documented = false, doc = "")
interface Provider<
ConstraintSettingInfoT extends ConstraintSettingInfoApi,
ConstraintValueInfoT extends ConstraintValueInfoApi,
PlatformInfoT extends PlatformInfoApi<ConstraintSettingInfoT, ConstraintValueInfoT>>
extends ProviderApi {

@StarlarkMethod(
name = "PlatformInfo",
doc = "The <code>PlatformInfo</code> constructor.",
documented = false,
parameters = {
@Param(
name = "label",
named = true,
positional = false,
doc = "The label for this platform."),
@Param(
name = "parent",
allowedTypes = {
@ParamType(type = PlatformInfoApi.class),
@ParamType(type = NoneType.class),
},
defaultValue = "None",
named = true,
positional = false,
doc = "The parent of this platform."),
@Param(
name = "constraint_values",
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = ConstraintValueInfoApi.class),
},
defaultValue = "[]",
named = true,
positional = false,
doc = "The constraint values for the platform"),
@Param(
name = "exec_properties",
allowedTypes = {
@ParamType(type = Dict.class),
@ParamType(type = NoneType.class),
},
defaultValue = "None",
named = true,
positional = false,
doc = "The exec properties for the platform.")
},
selfCall = true,
useStarlarkThread = true,
enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API)
@StarlarkConstructor
PlatformInfoT platformInfo(
Label label,
Object parent,
Sequence<?> constraintValues,
Object execProperties,
StarlarkThread thread)
throws EvalException;
}
}
151 changes: 0 additions & 151 deletions src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import com.google.common.collect.ImmutableMap;
import com.google.common.testing.EqualsTester;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import org.junit.Test;
Expand Down Expand Up @@ -291,156 +290,6 @@ public void execProperties_parentPlatform_inheritance() throws Exception {
assertThat(platformInfo.execProperties()).containsExactly("p1", "keep", "p3", "child");
}

@Test
public void starlark_constructor() throws Exception {
scratch.file(
"test/platform/my_platform.bzl",
"def _impl(ctx):",
" constraints = [val[platform_common.ConstraintValueInfo] "
+ "for val in ctx.attr.constraints]",
" platform = platform_common.PlatformInfo(",
" label = ctx.label, constraint_values = constraints)",
" return [platform]",
"my_platform = rule(",
" implementation = _impl,",
" attrs = {",
" 'constraints': attr.label_list(providers = [platform_common.ConstraintValueInfo])",
" }",
")");
scratch.file(
"test/constraint/BUILD",
"constraint_setting(name = 'basic')",
"constraint_value(name = 'foo',",
" constraint_setting = ':basic',",
")");
scratch.file(
"test/platform/BUILD",
"load('//test/platform:my_platform.bzl', 'my_platform')",
"my_platform(name = 'custom',",
" constraints = [",
" '//test/constraint:foo',",
" ],",
")");

setBuildLanguageOptions("--experimental_platforms_api");
ConfiguredTarget platform = getConfiguredTarget("//test/platform:custom");
assertThat(platform).isNotNull();

PlatformInfo provider = PlatformProviderUtils.platform(platform);
assertThat(provider).isNotNull();
assertThat(provider.label()).isEqualTo(Label.parseCanonicalUnchecked("//test/platform:custom"));
ConstraintSettingInfo constraintSetting =
ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//test/constraint:basic"));
ConstraintValueInfo constraintValue =
ConstraintValueInfo.create(
constraintSetting, Label.parseCanonicalUnchecked("//test/constraint:foo"));
assertThat(provider.constraints().get(constraintSetting)).isEqualTo(constraintValue);
}

@Test
public void starlark_constructor_parent() throws Exception {
scratch.file(
"test/platform/my_platform.bzl",
"def _impl(ctx):",
" constraints = [val[platform_common.ConstraintValueInfo] "
+ "for val in ctx.attr.constraints]",
" platform = platform_common.PlatformInfo(",
" label = ctx.label, constraint_values = constraints)",
" return [platform]",
"my_platform = rule(",
" implementation = _impl,",
" attrs = {",
" 'constraints': attr.label_list(providers = [platform_common.ConstraintValueInfo])",
" }",
")");
scratch.file(
"test/constraint/BUILD",
"constraint_setting(name = 'basic')",
"constraint_setting(name = 'complex')",
"constraint_value(name = 'foo',",
" constraint_setting = ':basic',",
")",
"constraint_value(name = 'bar',",
" constraint_setting = ':basic',",
")",
"constraint_value(name = 'baz',",
" constraint_setting = ':complex',",
")");
scratch.file(
"test/platform/BUILD",
"load('//test/platform:my_platform.bzl', 'my_platform')",
"platform(",
" name='parent',",
" constraint_values = [",
" '//test/constraint:foo',",
" ],",
")",
"my_platform(name = 'custom',",
" constraints = [",
" '//test/constraint:bar',",
" '//test/constraint:baz',",
" ],",
")");

setBuildLanguageOptions("--experimental_platforms_api");
ConfiguredTarget platform = getConfiguredTarget("//test/platform:custom");
assertThat(platform).isNotNull();

PlatformInfo provider = PlatformProviderUtils.platform(platform);
assertThat(provider).isNotNull();
assertThat(provider.label()).isEqualTo(Label.parseCanonicalUnchecked("//test/platform:custom"));

// Check that overrides work.
ConstraintSettingInfo constraintSetting =
ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//test/constraint:basic"));
ConstraintValueInfo constraintValue =
ConstraintValueInfo.create(
constraintSetting, Label.parseCanonicalUnchecked("//test/constraint:bar"));
assertThat(provider.constraints().get(constraintSetting)).isEqualTo(constraintValue);

// Check that inheritance works.
ConstraintSettingInfo constraintSetting2 =
ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//test/constraint:complex"));
ConstraintValueInfo constraintValue2 =
ConstraintValueInfo.create(
constraintSetting2, Label.parseCanonicalUnchecked("//test/constraint:baz"));
assertThat(provider.constraints().get(constraintSetting2)).isEqualTo(constraintValue2);
}

@Test
public void starlark_constructor_error_duplicateConstraints() throws Exception {
scratch.file(
"test/platform/my_platform.bzl",
"def _impl(ctx):",
" platform = platform_common.PlatformInfo()",
" return [platform]",
"my_platform = rule(",
" implementation = _impl,",
" attrs = {",
" 'constraints': attr.label_list(providers = [platform_common.ConstraintValueInfo])",
" }",
")");
scratch.file(
"test/constraint/BUILD",
"constraint_setting(name = 'basic')",
"constraint_value(name = 'foo',",
" constraint_setting = ':basic',",
")");
setBuildLanguageOptions("--experimental_platforms_api");
checkError(
"test/platform",
"custom",
"Label '//test/constraint:foo' is duplicated in the 'constraints' attribute of rule"
+ " 'custom'",
"load('//test/platform:my_platform.bzl', 'my_platform')",
"my_platform(name = 'custom',",
" constraints = [",
" '//test/constraint:foo',",
" '//test/constraint:foo',",
" ],",
")");
}

@Test
public void equalsTester() throws Exception {
ConstraintSettingInfo setting1 =
Expand Down
10 changes: 3 additions & 7 deletions src/test/shell/bazel/platforms_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ function test_platform_accessor() {
cat > rules.bzl <<'EOF'
def _impl(ctx):
platform = ctx.attr.platform[platform_common.PlatformInfo]
properties = platform.exec_properties
print("The properties are:", properties)
label = platform.label
print("The label is:", label)
return []
print_props = rule(
Expand All @@ -98,15 +98,11 @@ print_props(
platform(
name = "my_platform",
exec_properties = {
"key": "value",
"key2": "value2",
}
)
EOF

bazel build --experimental_platforms_api=true :a &> $TEST_log || fail "Build failed"
grep 'The properties are: {"key2": "value2", "key": "value"}' $TEST_log || fail "Did not find expected properties"
expect_log 'The label is: @//:my_platform'
}

run_suite "platform repo test"
Expand Down

0 comments on commit e5fe228

Please sign in to comment.