From edd3fc2cb47f5c7e95e6be1df0a38ac0499bac11 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 13 Apr 2023 11:23:51 -0700 Subject: [PATCH] python: port PyInfo to Starlark and enable it. Because providers are based on identity, and there are Java tests that need to reference the provider, it's hard to split up the changes, so it's all in a single change. A side-effect of this is the name used to access the provider in `cquery` commands changes slightly: from "PyInfo" to "@_builtins//:common/python/providers.bzl%PyInfo". This fully qualified name is considered internal and shouldn't be relied upon; it will change again when the rules are removed from Bazel and part of rules_python. The Java class is kept to make usage by the Java tests easier; it isn't actually used outside of tests. Work towards #15897 PiperOrigin-RevId: 524054712 Change-Id: I1ab7d299c7b0458e0ee5359babb1fa63f9739498 --- .../bazel/rules/BazelRuleClassProvider.java | 6 +- .../build/lib/rules/python/PyInfo.java | 264 ++---------------- .../starlarkbuildapi/python/PyBootstrap.java | 7 +- .../builtins_bzl/common/python/providers.bzl | 10 +- .../build/lib/rules/python/PyInfoTest.java | 172 ++++++------ .../rules/python/PythonStarlarkApiTest.java | 5 +- .../integration/configured_query_test.sh | 41 +-- 7 files changed, 143 insertions(+), 362 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index 44daf05444ac6a..9c7b49a04150c2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -113,7 +113,6 @@ import com.google.devtools.build.lib.rules.proto.BazelProtoLibraryRule; import com.google.devtools.build.lib.rules.proto.ProtoConfiguration; import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainRule; -import com.google.devtools.build.lib.rules.python.PyInfo; import com.google.devtools.build.lib.rules.python.PyRuleClasses.PySymlink; import com.google.devtools.build.lib.rules.python.PyRuntimeRule; import com.google.devtools.build.lib.rules.python.PyStarlarkTransitions; @@ -471,12 +470,9 @@ public void init(ConfiguredRuleClassProvider.Builder builder) { ContextGuardedValue.onlyInAllowedRepos( Starlark.NONE, PyBootstrap.allowedRepositories)); builder.addStarlarkBuiltinsInternal(BazelPyBuiltins.NAME, new BazelPyBuiltins()); - builder.addStarlarkBootstrap( new PyBootstrap( - PyInfo.PROVIDER, - PyStarlarkTransitions.INSTANCE, - new GoogleLegacyStubs.PyWrapCcHelper())); + PyStarlarkTransitions.INSTANCE, new GoogleLegacyStubs.PyWrapCcHelper())); builder.addSymlinkDefinition(PySymlink.PY2); builder.addSymlinkDefinition(PySymlink.PY3); diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyInfo.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyInfo.java index b7b84d9c86bb94..06df261e90f8d7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyInfo.java @@ -14,277 +14,63 @@ package com.google.devtools.build.lib.rules.python; -import com.google.common.base.Preconditions; +import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.Depset; -import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; -import com.google.devtools.build.lib.packages.BuiltinProvider; import com.google.devtools.build.lib.packages.Info; -import com.google.devtools.build.lib.starlarkbuildapi.python.PyInfoApi; -import com.google.errorprone.annotations.CanIgnoreReturnValue; -import java.util.Objects; -import javax.annotation.Nullable; +import com.google.devtools.build.lib.packages.StarlarkInfo; +import com.google.devtools.build.lib.packages.StarlarkProviderWrapper; import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkThread; -import net.starlark.java.syntax.Location; /** Instance of the provider type for the Python rules. */ -public final class PyInfo implements Info, PyInfoApi { - - public static final String STARLARK_NAME = "PyInfo"; - +@VisibleForTesting +public final class PyInfo { public static final PyInfoProvider PROVIDER = new PyInfoProvider(); - /** - * Returns true if the given depset has the given content type and order compatible with the given - * order. - */ - private static boolean depsetHasTypeAndCompatibleOrder( - Depset depset, Depset.ElementType type, Order order) { - // Work around #7266 by special-casing the empty set in the type check. - boolean typeOk = depset.isEmpty() || depset.getElementType().equals(type); - boolean orderOk = depset.getOrder().isCompatible(order); - return typeOk && orderOk; - } + private final StarlarkInfo info; - /** - * Returns the type name of a value and possibly additional description. - * - *

For depsets, this includes its content type and order. - */ - private static String describeType(Object value) { - if (value instanceof Depset) { - Depset depset = (Depset) value; - return depset.getOrder().getStarlarkName() - + "-ordered depset of " - + depset.getElementType() - + "s"; - } - return Starlark.type(value); + private PyInfo(StarlarkInfo info) { + this.info = info; } - private final Location location; - // Verified on initialization to contain Artifact. - private final Depset transitiveSources; - private final boolean usesSharedLibraries; - // Verified on initialization to contain String. - private final Depset imports; - private final boolean hasPy2OnlySources; - private final boolean hasPy3OnlySources; - - private PyInfo( - @Nullable Location location, - Depset transitiveSources, - boolean usesSharedLibraries, - Depset imports, - boolean hasPy2OnlySources, - boolean hasPy3OnlySources) { - Preconditions.checkArgument( - depsetHasTypeAndCompatibleOrder(transitiveSources, Artifact.TYPE, Order.COMPILE_ORDER)); - // TODO(brandjon): PyCommon currently requires COMPILE_ORDER, but we'll probably want to change - // that to NAIVE_LINK (preorder). In the meantime, order isn't an invariant of the provider - // itself, so we use STABLE here to accept any order. - Preconditions.checkArgument( - depsetHasTypeAndCompatibleOrder(imports, Depset.ElementType.STRING, Order.STABLE_ORDER)); - this.location = location != null ? location : Location.BUILTIN; - this.transitiveSources = transitiveSources; - this.usesSharedLibraries = usesSharedLibraries; - this.imports = imports; - this.hasPy2OnlySources = hasPy2OnlySources; - this.hasPy3OnlySources = hasPy3OnlySources; - } - - @Override public PyInfoProvider getProvider() { return PROVIDER; } - @Override - public Location getCreationLocation() { - return location; + public NestedSet getTransitiveSourcesSet() throws EvalException { + Object value = info.getValue("transitive_sources"); + return Depset.cast(value, Artifact.class, "transitive_sources"); } - @Override - public boolean equals(Object other) { - // PyInfo implements value equality, but note that it contains identity-equality fields - // (depsets), so you generally shouldn't rely on equality comparisons. - if (!(other instanceof PyInfo)) { - return false; - } - PyInfo otherInfo = (PyInfo) other; - return (this.transitiveSources.equals(otherInfo.transitiveSources) - && this.usesSharedLibraries == otherInfo.usesSharedLibraries - && this.imports.equals(otherInfo.imports) - && this.hasPy2OnlySources == otherInfo.hasPy2OnlySources - && this.hasPy3OnlySources == otherInfo.hasPy3OnlySources); + public boolean getUsesSharedLibraries() throws EvalException { + return info.getValue("uses_shared_libraries", Boolean.class); } - @Override - public int hashCode() { - return Objects.hash( - PyInfo.class, - transitiveSources, - usesSharedLibraries, - imports, - hasPy2OnlySources, - hasPy3OnlySources); + public NestedSet getImportsSet() throws EvalException { + Object value = info.getValue("imports"); + return Depset.cast(value, String.class, "imports"); } - @Override - public Depset getTransitiveSources() { - return transitiveSources; - } - - public NestedSet getTransitiveSourcesSet() { - try { - return transitiveSources.getSet(Artifact.class); - } catch (TypeException e) { - throw new IllegalStateException( - "'transitiveSources' depset was found to be invalid type " + imports.getElementType(), e); - } + public boolean getHasPy2OnlySources() throws EvalException { + return info.getValue("has_py2_only_sources", Boolean.class); } - @Override - public boolean getUsesSharedLibraries() { - return usesSharedLibraries; - } - - @Override - public Depset getImports() { - return imports; - } - - public NestedSet getImportsSet() { - try { - return imports.getSet(String.class); - } catch (TypeException e) { - throw new IllegalStateException( - "'imports' depset was found to be invalid type " + imports.getElementType(), e); - } - } - - @Override - public boolean getHasPy2OnlySources() { - return hasPy2OnlySources; - } - - @Override - public boolean getHasPy3OnlySources() { - return hasPy3OnlySources; + public boolean getHasPy3OnlySources() throws EvalException { + return info.getValue("has_py3_only_sources", Boolean.class); } /** The singular PyInfo provider type object. */ - public static class PyInfoProvider extends BuiltinProvider - implements PyInfoApi.PyInfoProviderApi { + public static class PyInfoProvider extends StarlarkProviderWrapper { private PyInfoProvider() { - super(STARLARK_NAME, PyInfo.class); + super(Label.parseCanonicalUnchecked("@_builtins//:common/python/providers.bzl"), "PyInfo"); } @Override - public PyInfo constructor( - Depset transitiveSources, - boolean usesSharedLibraries, - Object importsUncast, - boolean hasPy2OnlySources, - boolean hasPy3OnlySources, - StarlarkThread thread) - throws EvalException { - Depset imports = - importsUncast.equals(Starlark.UNBOUND) - ? Depset.of(String.class, NestedSetBuilder.emptySet(Order.COMPILE_ORDER)) - : (Depset) importsUncast; - - if (!depsetHasTypeAndCompatibleOrder(transitiveSources, Artifact.TYPE, Order.COMPILE_ORDER)) { - throw Starlark.errorf( - "'transitive_sources' field should be a postorder-compatible depset of Files (got a" - + " '%s')", - describeType(transitiveSources)); - } - if (!depsetHasTypeAndCompatibleOrder( - imports, Depset.ElementType.STRING, Order.STABLE_ORDER)) { - throw Starlark.errorf( - "'imports' field should be a depset of strings (got a '%s')", describeType(imports)); - } - // Validate depset parameters - Depset.cast(transitiveSources, Artifact.class, "transitive_sources"); - Depset.cast(imports, String.class, "imports"); - - return new PyInfo( - thread.getCallerLocation(), - transitiveSources, - usesSharedLibraries, - imports, - hasPy2OnlySources, - hasPy3OnlySources); - } - } - - public static Builder builder() { - return new Builder(); - } - - /** Builder for {@link PyInfo}. */ - public static class Builder { - Location location = null; - NestedSet transitiveSources = NestedSetBuilder.emptySet(Order.COMPILE_ORDER); - boolean usesSharedLibraries = false; - NestedSet imports = NestedSetBuilder.emptySet(Order.COMPILE_ORDER); - boolean hasPy2OnlySources = false; - boolean hasPy3OnlySources = false; - - // Use the static builder() method instead. - private Builder() {} - - @CanIgnoreReturnValue - public Builder setLocation(Location location) { - this.location = location; - return this; - } - - @CanIgnoreReturnValue - public Builder setTransitiveSources(NestedSet transitiveSources) { - this.transitiveSources = transitiveSources; - return this; - } - - @CanIgnoreReturnValue - public Builder setUsesSharedLibraries(boolean usesSharedLibraries) { - this.usesSharedLibraries = usesSharedLibraries; - return this; - } - - @CanIgnoreReturnValue - public Builder setImports(NestedSet imports) { - this.imports = imports; - return this; - } - - @CanIgnoreReturnValue - public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) { - this.hasPy2OnlySources = hasPy2OnlySources; - return this; - } - - @CanIgnoreReturnValue - public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) { - this.hasPy3OnlySources = hasPy3OnlySources; - return this; - } - - public PyInfo build() { - Preconditions.checkNotNull(transitiveSources); - return new PyInfo( - location, - Depset.of(Artifact.class, transitiveSources), - usesSharedLibraries, - Depset.of(String.class, imports), - hasPy2OnlySources, - hasPy3OnlySources); + public PyInfo wrap(Info value) { + return new PyInfo((StarlarkInfo) value); } } } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyBootstrap.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyBootstrap.java index fdc1e6bc0f537b..d6e16616600ef9 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyBootstrap.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyBootstrap.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.starlarkbuildapi.core.Bootstrap; import com.google.devtools.build.lib.starlarkbuildapi.core.ContextAndFlagGuardedValue; import com.google.devtools.build.lib.starlarkbuildapi.cpp.PyWrapCcHelperApi; -import com.google.devtools.build.lib.starlarkbuildapi.python.PyInfoApi.PyInfoProviderApi; import com.google.devtools.build.lib.starlarkbuildapi.stubs.ProviderStub; import net.starlark.java.eval.FlagGuardedValue; @@ -34,15 +33,12 @@ public class PyBootstrap implements Bootstrap { PackageIdentifier.createUnchecked("rules_python", ""), PackageIdentifier.createUnchecked("", "tools/build_defs/python")); - private final PyInfoProviderApi pyInfoProviderApi; private final PyStarlarkTransitionsApi pyStarlarkTransitionsApi; private final PyWrapCcHelperApi pyWrapCcHelper; public PyBootstrap( - PyInfoProviderApi pyInfoProviderApi, PyStarlarkTransitionsApi pyStarlarkTransitionsApi, PyWrapCcHelperApi pyWrapCcHelper) { - this.pyInfoProviderApi = pyInfoProviderApi; this.pyStarlarkTransitionsApi = pyStarlarkTransitionsApi; this.pyWrapCcHelper = pyWrapCcHelper; } @@ -53,7 +49,8 @@ public void addBindingsToBuilder(ImmutableMap.Builder builder) { "PyInfo", ContextAndFlagGuardedValue.onlyInAllowedReposOrWhenIncompatibleFlagIsFalse( BuildLanguageOptions.INCOMPATIBLE_STOP_EXPORTING_LANGUAGE_MODULES, - pyInfoProviderApi, + // Workaround for https://github.com/bazelbuild/bazel/issues/17713 + new ProviderStub(), allowedRepositories)); builder.put( "PyRuntimeInfo", diff --git a/src/main/starlark/builtins_bzl/common/python/providers.bzl b/src/main/starlark/builtins_bzl/common/python/providers.bzl index 317a9711e43ee1..fb512b23a40a4f 100644 --- a/src/main/starlark/builtins_bzl/common/python/providers.bzl +++ b/src/main/starlark/builtins_bzl/common/python/providers.bzl @@ -154,10 +154,14 @@ def _PyInfo_init( has_py2_only_sources = False, has_py3_only_sources = False): _check_arg_type("transitive_sources", "depset", transitive_sources) + + # Verify it's postorder compatible, but retain is original ordering. + depset(transitive = [transitive_sources], order = "postorder") + _check_arg_type("uses_shared_libraries", "bool", uses_shared_libraries) _check_arg_type("imports", "depset", imports) _check_arg_type("has_py2_only_sources", "bool", has_py2_only_sources) - _check_arg_type("has_py2_only_sources", "bool", has_py3_only_sources) + _check_arg_type("has_py3_only_sources", "bool", has_py3_only_sources) return { "transitive_sources": transitive_sources, "imports": imports, @@ -166,7 +170,7 @@ def _PyInfo_init( "has_py3_only_sources": has_py2_only_sources, } -StarlarkPyInfo, _unused_raw_py_info_ctor = provider( +PyInfo, _unused_raw_py_info_ctor = provider( "Encapsulates information provided by the Python rules.", init = _PyInfo_init, fields = { @@ -191,8 +195,6 @@ is recommended to use `default` order (the default). }, ) -PyInfo = _builtins.toplevel.PyInfo - def _PyCcLinkParamsProvider_init(cc_info): return { "cc_info": _CcInfo(linking_context = cc_info.linking_context), diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java index 55fdcadc02163c..40fc0388c4a1cf 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java @@ -19,10 +19,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; -import com.google.devtools.build.lib.starlark.util.BazelEvaluationTestCase; -import net.starlark.java.syntax.Location; +import java.util.regex.Pattern; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,15 +30,36 @@ @RunWith(JUnit4.class) public class PyInfoTest extends BuildViewTestCase { - private final BazelEvaluationTestCase ev = new BazelEvaluationTestCase(); - private Artifact dummyArtifact; @Before public void setUp() throws Exception { dummyArtifact = getSourceArtifact("dummy"); - ev.update("PyInfo", PyInfo.PROVIDER); - ev.update("dummy_file", dummyArtifact); + } + + private void writeCreatePyInfo(String... lines) throws Exception { + var builder = new StringBuilder(); + for (var line : lines) { + builder.append(" ").append(line).append(",\n"); + } + scratch.overwriteFile( + "defs.bzl", + "def _impl(ctx):", + " dummy_file = ctx.file.dummy_file", + " info = PyInfo(", + builder.toString(), + " )", + " return [info]", + "create_py_info = rule(implementation=_impl, attrs={", + " 'dummy_file': attr.label(default='dummy', allow_single_file=True),", + "})", + ""); + scratch.overwriteFile( + "BUILD", "load(':defs.bzl', 'create_py_info')", "create_py_info(name='subject')"); + } + + private PyInfo getPyInfo() throws Exception { + return getConfiguredTarget("//:subject").get(PyInfo.PROVIDER); } /** We need this because {@code NestedSet}s don't have value equality. */ @@ -50,123 +69,96 @@ private static void assertHasOrderAndContainsExactly( assertThat(set.toList()).containsExactly(values); } - /** Checks values set by the builder. */ - @Test - public void builderExplicit() throws Exception { - NestedSet sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact); - NestedSet imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc"); - Location loc = Location.fromFileLineColumn("foo", 1, 2); - PyInfo info = - PyInfo.builder() - .setLocation(loc) - .setTransitiveSources(sources) - .setUsesSharedLibraries(true) - .setImports(imports) - .setHasPy2OnlySources(true) - .setHasPy3OnlySources(true) - .build(); - assertThat(info.getCreationLocation()).isEqualTo(loc); - assertHasOrderAndContainsExactly( - info.getTransitiveSources().getSet(Artifact.class), Order.COMPILE_ORDER, dummyArtifact); - assertThat(info.getUsesSharedLibraries()).isTrue(); - assertHasOrderAndContainsExactly( - info.getImports().getSet(String.class), Order.COMPILE_ORDER, "abc"); - assertThat(info.getHasPy2OnlySources()).isTrue(); - assertThat(info.getHasPy3OnlySources()).isTrue(); - } + private void assertContainsError(String pattern) throws Exception { + reporter.removeHandler(failFastHandler); // expect errors - /** Checks the defaults set by the builder. */ - @Test - public void builderDefaults() throws Exception { - // transitive_sources is mandatory, so create a dummy value but no need to assert on it. - NestedSet sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact); - PyInfo info = PyInfo.builder().setTransitiveSources(sources).build(); - assertThat(info.getCreationLocation()).isEqualTo(Location.BUILTIN); - assertThat(info.getUsesSharedLibraries()).isFalse(); - assertHasOrderAndContainsExactly(info.getImports().getSet(String.class), Order.COMPILE_ORDER); - assertThat(info.getHasPy2OnlySources()).isFalse(); - assertThat(info.getHasPy3OnlySources()).isFalse(); + getConfiguredTarget("//:subject"); + + // The Starlark messages are within a long multi-line traceback string, so + // add the implicit .* for convenience. + // NOTE: failures and events are accumulated between getConfiguredTarget() calls. + assertContainsEvent(Pattern.compile(".*" + pattern)); } @Test public void starlarkConstructor() throws Exception { - ev.exec( - "info = PyInfo(", - " transitive_sources = depset(direct=[dummy_file]),", - " uses_shared_libraries = True,", - " imports = depset(direct=['abc']),", - " has_py2_only_sources = True,", - " has_py3_only_sources = True,", - ")"); - PyInfo info = (PyInfo) ev.lookup("info"); - assertThat(info.getCreationLocation().toString()).isEqualTo(":1:14"); + writeCreatePyInfo( + " transitive_sources = depset(direct=[dummy_file])", + " uses_shared_libraries = True", + " imports = depset(direct=['abc'])", + " has_py2_only_sources = True", + " has_py3_only_sources = True"); + + PyInfo info = getPyInfo(); + assertHasOrderAndContainsExactly( - info.getTransitiveSources().getSet(Artifact.class), Order.STABLE_ORDER, dummyArtifact); + info.getTransitiveSourcesSet(), Order.STABLE_ORDER, dummyArtifact); assertThat(info.getUsesSharedLibraries()).isTrue(); - assertHasOrderAndContainsExactly( - info.getImports().getSet(String.class), Order.STABLE_ORDER, "abc"); + assertHasOrderAndContainsExactly(info.getImportsSet(), Order.STABLE_ORDER, "abc"); assertThat(info.getHasPy2OnlySources()).isTrue(); assertThat(info.getHasPy3OnlySources()).isTrue(); } @Test public void starlarkConstructorDefaults() throws Exception { - ev.exec("info = PyInfo(transitive_sources = depset(direct=[dummy_file]))"); - PyInfo info = (PyInfo) ev.lookup("info"); - assertThat(info.getCreationLocation().toString()).isEqualTo(":1:14"); + writeCreatePyInfo("transitive_sources = depset(direct=[dummy_file])"); + + PyInfo info = getPyInfo(); + assertHasOrderAndContainsExactly( - info.getTransitiveSources().getSet(Artifact.class), Order.STABLE_ORDER, dummyArtifact); + info.getTransitiveSourcesSet(), Order.STABLE_ORDER, dummyArtifact); assertThat(info.getUsesSharedLibraries()).isFalse(); - assertHasOrderAndContainsExactly(info.getImports().getSet(String.class), Order.COMPILE_ORDER); + assertHasOrderAndContainsExactly(info.getImportsSet(), Order.STABLE_ORDER); assertThat(info.getHasPy2OnlySources()).isFalse(); assertThat(info.getHasPy3OnlySources()).isFalse(); } @Test - public void starlarkConstructorErrors_TransitiveSources() throws Exception { - ev.checkEvalErrorContains( - "missing 1 required named argument: transitive_sources", // - "PyInfo()"); - ev.checkEvalErrorContains( - "got value of type 'string', want 'depset'", // - "PyInfo(transitive_sources = 'abc')"); - ev.checkEvalErrorContains( - "should be a postorder-compatible depset of Files (got a 'default-ordered depset of" - + " strings')", // - "PyInfo(transitive_sources = depset(direct=['abc']))"); - ev.checkEvalErrorContains( - "'transitive_sources' field should be a postorder-compatible depset of Files", - "PyInfo(transitive_sources = depset(direct=[dummy_file], order='preorder'))"); + public void starlarkConstructorErrors_transitiveSources_missing() throws Exception { + writeCreatePyInfo(); + + assertContainsError("missing.*argument.*transitive_sources"); + } + + @Test + public void starlarkConstructorErrors_transitiveSources_badType() throws Exception { + writeCreatePyInfo("transitive_sources = 'abc'"); + + assertContainsError("transitive_sources.*got.*string.*want.*depset"); + } + + @Test + public void starlarkConstructorErrors_transitiveSources_rejectsPreOrder() throws Exception { + writeCreatePyInfo("transitive_sources = depset(direct=[dummy_file], order='preorder')"); + + assertContainsError("Order.*postorder.*incompatible.*preorder"); } @Test public void starlarkConstructorErrors_UsesSharedLibraries() throws Exception { - ev.checkEvalErrorContains( - "got value of type 'string', want 'bool'", - "PyInfo(transitive_sources = depset([]), uses_shared_libraries = 'abc')"); + writeCreatePyInfo("transitive_sources = depset()", "uses_shared_libraries = 'abc'"); + + assertContainsError("uses_shared_libraries.*got.*string.*want.*bool"); } @Test - public void starlarkConstructorErrors_Imports() throws Exception { - ev.checkEvalErrorContains( - "got value of type 'string', want 'depset'", - "PyInfo(transitive_sources = depset([]), imports = 'abc')"); - ev.checkEvalErrorContains( - "should be a depset of strings (got a 'default-ordered depset of ints')", - "PyInfo(transitive_sources = depset([]), imports = depset(direct=[123]))"); + public void starlarkConstructorErrors_imports_badType() throws Exception { + writeCreatePyInfo("transitive_sources = depset()", "imports = 'abc'"); + + assertContainsError("imports.*got.*string.*want.*depset"); } @Test public void starlarkConstructorErrors_HasPy2OnlySources() throws Exception { - ev.checkEvalErrorContains( - "got value of type 'string', want 'bool'", - "PyInfo(transitive_sources = depset([]), has_py2_only_sources = 'abc')"); + writeCreatePyInfo("transitive_sources = depset()", "has_py2_only_sources = 'abc'"); + + assertContainsError("has_py2_only_sources.*got.*string.*want.*bool"); } @Test public void starlarkConstructorErrors_HasPy3OnlySources() throws Exception { - ev.checkEvalErrorContains( - "got value of type 'string', want 'bool'", - "PyInfo(transitive_sources = depset([]), has_py3_only_sources = 'abc')"); + writeCreatePyInfo("transitive_sources = depset()", "has_py3_only_sources = 'abc'"); + + assertContainsError("has_py3_only_sources.*got.*string.*want.*bool"); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java index 08ed13b7d78084..c6b4d68affb3ff 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.testutil.TestConstants; @@ -100,13 +99,13 @@ public void librarySandwich() throws Exception { ConfiguredTarget target = getConfiguredTarget("//pkg:upperuserlib"); PyInfo info = target.get(PyInfo.PROVIDER); - assertThat(info.getTransitiveSources().toList(Artifact.class)) + assertThat(info.getTransitiveSourcesSet().toList()) .containsExactly( getSourceArtifact("pkg/loweruserlib.py"), getSourceArtifact("pkg/pylib.py"), getSourceArtifact("pkg/upperuserlib.py")); assertThat(info.getUsesSharedLibraries()).isTrue(); - assertThat(info.getImports().toList(String.class)) + assertThat(info.getImportsSet().toList()) .containsExactly("loweruserlib_path", "upperuserlib_path"); assertThat(info.getHasPy2OnlySources()).isTrue(); assertThat(info.getHasPy3OnlySources()).isTrue(); diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh index 8cf12718bb32ef..250cf4fa74599b 100755 --- a/src/test/shell/integration/configured_query_test.sh +++ b/src/test/shell/integration/configured_query_test.sh @@ -1200,15 +1200,24 @@ EOF function test_starlark_output_providers_function() { local -r pkg=$FUNCNAME mkdir -p $pkg + cat > $pkg/defs.bzl <<'EOF' +def foo_impl(ctx): + return [DefaultInfo()] + +foo_library = rule( + implementation = foo_impl, +) +EOF cat > $pkg/BUILD <<'EOF' -py_library( - name = "pylib", - srcs = ["pylib.py"], - srcs_version = "PY3", +load(":defs.bzl", "foo_library") + +foo_library( + name = "lib", ) +exports_files(["srcfile.txt"]) EOF - cat > $pkg/pylib.py <<'EOF' -pylib=1 + cat > $pkg/srcfile.txt <<'EOF' +hello, world EOF cat > $pkg/outfunc.bzl <<'EOF' def format(target): @@ -1218,24 +1227,24 @@ def format(target): ret = str(target.label) + ':providers=' + str(sorted(p.keys())) vis_info = p.get('VisibilityProvider') if vis_info: - ret += '\n\tVisbilityProvider.label:' + str(vis_info.label) - py_info = p.get('PyInfo') - if py_info: - ret += '\n\tPyInfo found' + ret += '\n\tVisibilityProvider.label:' + str(vis_info.label) + output_group_info = p.get('OutputGroupInfo') + if output_group_info: + ret += '\n\tOutputGroupInfo found' return ret EOF - bazel cquery "//$pkg:pylib" --output=starlark --starlark:file="$pkg/outfunc.bzl" >output \ + bazel cquery "//$pkg:lib" --output=starlark --starlark:file="$pkg/outfunc.bzl" >output \ 2>"$TEST_log" || fail "Expected success" - assert_contains "//$pkg:pylib:providers=.*PyInfo" output - assert_contains "PyInfo found" output + assert_contains "//$pkg:lib:providers=.*OutputGroupInfo" output + assert_contains "OutputGroupInfo found" output # A file - bazel cquery "//$pkg:pylib.py" --output=starlark --starlark:file="$pkg/outfunc.bzl" >output \ + bazel cquery "//$pkg:srcfile.txt" --output=starlark --starlark:file="$pkg/outfunc.bzl" >output \ 2>"$TEST_log" || fail "Expected success" - assert_contains "//$pkg:pylib.py:providers=.*FileProvider.*FilesToRunProvider.*LicensesProvider.*VisibilityProvider" \ + assert_contains "//$pkg:srcfile.txt:providers=.*FileProvider.*FilesToRunProvider.*LicensesProvider.*VisibilityProvider" \ output - assert_contains "VisbilityProvider.label:@//$pkg:pylib.py" output + assert_contains "VisibilityProvider.label:@//$pkg:srcfile.txt" output } function test_starlark_output_providers_starlark_provider() {