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() {