diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java index 9bbec8674faf9d..9e06a6cb3fff47 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.python.PyCommon; +import com.google.devtools.build.lib.rules.python.PyProvider; import com.google.devtools.build.lib.rules.python.PyRuleClasses; import com.google.devtools.build.lib.rules.python.PythonVersion; import com.google.devtools.build.lib.util.FileType; @@ -69,7 +70,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .override( builder .copy("deps") - .legacyMandatoryProviders(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME) + .legacyMandatoryProviders(PyProvider.PROVIDER_NAME) .allowedFileTypes()) /* List of import directories to be added to the PYTHONPATH. diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetBuilder.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetBuilder.java index f90efc423da05e..fa2fbd3d13312c 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetBuilder.java @@ -41,6 +41,16 @@ public NestedSetBuilder(Order order) { this.order = order; } + /** + * Returns the order used by this builder. + * + *

This is useful for testing for incompatibilities (via {@link Order#isCompatible}) without + * catching an unchecked exception from {@link #addTransitive}. + */ + public Order getOrder() { + return order; + } + /** Returns whether the set to be built is empty. */ public boolean isEmpty() { return items.isEmpty() && transitiveSets.isEmpty(); @@ -93,8 +103,8 @@ public NestedSetBuilder addAll(NestedSet elements) { * *

The relative left-to-right order of transitive members is preserved from the sequence of * calls to {@link #addTransitive}. Since the traversal {@link Order} controls whether direct - * members appear before or after transitive ones, the interleaving of - * {@link #add}/{@link #addAll} with {@link #addTransitive} does not matter. + * members appear before or after transitive ones, the interleaving of {@link #add}/{@link + * #addAll} with {@link #addTransitive} does not matter. * *

The {@link Order} of the added set must be compatible with the order of this builder (see * {@link Order#isCompatible}). This is true even if the added set is empty. Strictly speaking, it @@ -109,7 +119,7 @@ public NestedSetBuilder addAll(NestedSet elements) { * * @param subset the set to add as a transitive member; must not be null * @return the builder - * @throws IllegalStateException if the order of {@code subset} is not compatible with the + * @throws IllegalArgumentException if the order of {@code subset} is not compatible with the * order of this builder */ public NestedSetBuilder addTransitive(NestedSet subset) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index 5e5301d18190f3..29f5c206a62bd9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -16,7 +16,6 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionOwner; @@ -43,12 +42,9 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.StructImpl; -import com.google.devtools.build.lib.packages.StructProvider; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.syntax.EvalException; -import com.google.devtools.build.lib.syntax.EvalUtils; -import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileType; @@ -66,11 +62,6 @@ */ public final class PyCommon { - public static final String PYTHON_SKYLARK_PROVIDER_NAME = "py"; - public static final String TRANSITIVE_PYTHON_SRCS = "transitive_sources"; - public static final String IS_USING_SHARED_LIBRARY = "uses_shared_libraries"; - public static final String IMPORTS = "imports"; - public static final String DEFAULT_PYTHON_VERSION_ATTRIBUTE = "default_python_version"; public static final String PYTHON_VERSION_ATTRIBUTE = "python_version"; @@ -163,34 +154,14 @@ public void addCommonTransitiveInfoProviders( filesToBuild, /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER))) .addSkylarkTransitiveInfo( - PYTHON_SKYLARK_PROVIDER_NAME, - createSourceProvider(this.transitivePythonSources, usesSharedLibraries(), imports)) + PyProvider.PROVIDER_NAME, + PyProvider.create(this.transitivePythonSources, usesSharedLibraries(), imports)) // Python targets are not really compilable. The best we can do is make sure that all // generated source files are ready. .addOutputGroup(OutputGroupInfo.FILES_TO_COMPILE, transitivePythonSources) .addOutputGroup(OutputGroupInfo.COMPILATION_PREREQUISITES, transitivePythonSources); } - /** - * Returns a Skylark struct for exposing transitive Python sources: - * - *

addSkylarkTransitiveInfo(PYTHON_SKYLARK_PROVIDER_NAME, createSourceProvider(...)) - */ - public static StructImpl createSourceProvider( - NestedSet transitivePythonSources, - boolean isUsingSharedLibrary, - NestedSet imports) { - return StructProvider.STRUCT.create( - ImmutableMap.of( - TRANSITIVE_PYTHON_SRCS, - SkylarkNestedSet.of(Artifact.class, transitivePythonSources), - IS_USING_SHARED_LIBRARY, - isUsingSharedLibrary, - IMPORTS, - SkylarkNestedSet.of(String.class, imports)), - "No such attribute '%s'"); - } - /** Returns the parsed value of the "srcs_version" attribute. */ private static PythonVersion getSrcsVersionAttr(RuleContext ruleContext) { String attrValue = ruleContext.attributes().get("srcs_version", Type.STRING); @@ -331,49 +302,33 @@ private Iterable getTargetDeps() { return ruleContext.getPrerequisites("deps", Mode.TARGET); } - private NestedSet getTransitivePythonSourcesFromSkylarkProvider( - TransitiveInfoCollection dep) { - StructImpl pythonSkylarkProvider = null; - try { - pythonSkylarkProvider = - SkylarkType.cast( - dep.get(PYTHON_SKYLARK_PROVIDER_NAME), - StructImpl.class, - null, - "%s should be a struct", - PYTHON_SKYLARK_PROVIDER_NAME); - - if (pythonSkylarkProvider != null) { - Object sourceFiles = pythonSkylarkProvider.getValue(TRANSITIVE_PYTHON_SRCS); - String errorType; - if (sourceFiles == null) { - errorType = "null"; - } else { - errorType = EvalUtils.getDataTypeNameFromClass(sourceFiles.getClass()); - } - String errorMsg = "Illegal Argument: attribute '%s' in provider '%s' is " - + "of unexpected type. Should be a set, but got a '%s'"; - NestedSet pythonSourceFiles = SkylarkType.cast( - sourceFiles, SkylarkNestedSet.class, Artifact.class, null, - errorMsg, TRANSITIVE_PYTHON_SRCS, PYTHON_SKYLARK_PROVIDER_NAME, errorType) - .getSet(Artifact.class); - return pythonSourceFiles; - } - } catch (EvalException e) { - ruleContext.ruleError(e.getMessage()); - } - return null; + private static String getOrderErrorMessage(String fieldName, Order expected, Order actual) { + return String.format( + "Incompatible order for %s: expected 'default' or '%s', got '%s'", + fieldName, expected.getSkylarkName(), actual.getSkylarkName()); } private void collectTransitivePythonSourcesFrom( Iterable deps, NestedSetBuilder builder) { for (TransitiveInfoCollection dep : deps) { - NestedSet pythonSourceFiles = getTransitivePythonSourcesFromSkylarkProvider(dep); - if (pythonSourceFiles != null) { - builder.addTransitive(pythonSourceFiles); + if (PyProvider.hasProvider(dep)) { + try { + StructImpl info = PyProvider.getProvider(dep); + NestedSet sources = PyProvider.getTransitiveSources(info); + if (!builder.getOrder().isCompatible(sources.getOrder())) { + ruleContext.ruleError( + getOrderErrorMessage( + PyProvider.TRANSITIVE_SOURCES, builder.getOrder(), sources.getOrder())); + } else { + builder.addTransitive(sources); + } + } catch (EvalException e) { + // Either the provider type or field type is bad. + ruleContext.ruleError(e.getMessage()); + } } else { // TODO(bazel-team): We also collect .py source files from deps (e.g. for proto_library - // rules). Rules should implement PythonSourcesProvider instead. + // rules). We should have rules implement a "PythonSourcesProvider" instead. FileProvider provider = dep.getProvider(FileProvider.class); builder.addAll(FileType.filter(provider.getFilesToBuild(), PyRuleClasses.PYTHON_SOURCE)); } @@ -389,6 +344,8 @@ private NestedSet collectTransitivePythonSources() { return builder.build(); } + // TODO(brandjon): Move provider merging logic into PyProvider. Have rule implementations read + // the sources off a merged provider of deps (with/without the local rule included in the merge). public NestedSet collectTransitivePythonSourcesWithoutLocal() { NestedSetBuilder builder = NestedSetBuilder.compileOrder(); collectTransitivePythonSourcesFrom(getTargetDeps(), builder); @@ -406,7 +363,15 @@ private void collectTransitivePythonImports(NestedSetBuilder builder) { for (TransitiveInfoCollection dep : getTargetDeps()) { if (dep.getProvider(PythonImportsProvider.class) != null) { PythonImportsProvider provider = dep.getProvider(PythonImportsProvider.class); - builder.addTransitive(provider.getTransitivePythonImports()); + NestedSet imports = provider.getTransitivePythonImports(); + if (!builder.getOrder().isCompatible(imports.getOrder())) { + // TODO(brandjon): Add test case for this error, once we replace PythonImportsProvider + // with the normal Python provider and once we clean up our provider merge logic. + ruleContext.ruleError( + getOrderErrorMessage(PyProvider.IMPORTS, builder.getOrder(), imports.getOrder())); + } else { + builder.addTransitive(imports); + } } } } @@ -510,11 +475,12 @@ public boolean usesSharedLibraries() { public static boolean checkForSharedLibraries(Iterable deps) throws EvalException{ for (TransitiveInfoCollection dep : deps) { - Object providerObject = dep.get(PYTHON_SKYLARK_PROVIDER_NAME); + Object providerObject = dep.get(PyProvider.PROVIDER_NAME); if (providerObject != null) { SkylarkType.checkType(providerObject, StructImpl.class, null); StructImpl provider = (StructImpl) providerObject; - Boolean isUsingSharedLibrary = provider.getValue(IS_USING_SHARED_LIBRARY, Boolean.class); + Boolean isUsingSharedLibrary = + provider.getValue(PyProvider.USES_SHARED_LIBRARIES, Boolean.class); if (Boolean.TRUE.equals(isUsingSharedLibrary)) { return true; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyProvider.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyProvider.java new file mode 100644 index 00000000000000..75484ff2900b4e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyProvider.java @@ -0,0 +1,157 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.python; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.packages.StructImpl; +import com.google.devtools.build.lib.packages.StructProvider; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.EvalUtils; +import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.SkylarkType; + +/** + * Static helper class for managing the "py" struct provider returned and consumed by Python rules. + */ +// TODO(brandjon): Replace this with a real provider. +public class PyProvider { + + // Disable construction. + private PyProvider() {} + + /** Name of the Python provider in Starlark code (as a field of a {@code Target}. */ + public static final String PROVIDER_NAME = "py"; + + /** + * Name of field holding a depset of transitive sources (i.e., .py files in srcs and in srcs of + * transitive deps). + */ + public static final String TRANSITIVE_SOURCES = "transitive_sources"; + + /** + * Name of field holding a boolean indicating whether any transitive dep uses shared libraries. + */ + public static final String USES_SHARED_LIBRARIES = "uses_shared_libraries"; + + /** + * Name of field holding a depset of import paths added by the transitive deps (including this + * target). + */ + public static final String IMPORTS = "imports"; + + /** Constructs a provider instance with the given field values. */ + public static StructImpl create( + NestedSet transitiveSources, + boolean usesSharedLibraries, + NestedSet imports) { + return StructProvider.STRUCT.create( + ImmutableMap.of( + PyProvider.TRANSITIVE_SOURCES, + SkylarkNestedSet.of(Artifact.class, transitiveSources), + PyProvider.USES_SHARED_LIBRARIES, + usesSharedLibraries, + PyProvider.IMPORTS, + SkylarkNestedSet.of(String.class, imports)), + "No such attribute '%s'"); + } + + /** Returns whether a given dependency has the py provider. */ + public static boolean hasProvider(TransitiveInfoCollection dep) { + return dep.get(PROVIDER_NAME) != null; + } + + /** + * Returns the struct representing the py provider, from the given target info. + * + * @throws EvalException if the provider does not exist or has the wrong type. + */ + public static StructImpl getProvider(TransitiveInfoCollection dep) throws EvalException { + Object info = dep.get(PROVIDER_NAME); + if (info == null) { + throw new EvalException(/*location=*/ null, "Target does not have 'py' provider"); + } + return SkylarkType.cast( + info, StructImpl.class, null, "'%s' provider should be a struct", PROVIDER_NAME); + } + + private static Object getValue(StructImpl info, String fieldName) throws EvalException { + Object fieldValue = info.getValue(fieldName); + if (fieldValue == null) { + throw new EvalException( + /*location=*/ null, String.format("'py' provider missing '%s' field", fieldName)); + } + return fieldValue; + } + + /** + * Casts and returns the transitive sources field. + * + * @throws EvalException if the field does not exist or is not a depset of {@link Artifact} + */ + public static NestedSet getTransitiveSources(StructImpl info) throws EvalException { + Object fieldValue = getValue(info, TRANSITIVE_SOURCES); + SkylarkNestedSet castValue = + SkylarkType.cast( + fieldValue, + SkylarkNestedSet.class, + Artifact.class, + null, + "'%s' provider's '%s' field should be a depset of Files (got a '%s')", + PROVIDER_NAME, + TRANSITIVE_SOURCES, + EvalUtils.getDataTypeNameFromClass(fieldValue.getClass())); + return castValue.getSet(Artifact.class); + } + + /** + * Casts and returns the uses-shared-libraries field. + * + * @throws EvalException if the field does not exist or is not a boolean + */ + public static boolean getUsesSharedLibraries(StructImpl info) throws EvalException { + Object fieldValue = getValue(info, USES_SHARED_LIBRARIES); + return SkylarkType.cast( + fieldValue, + Boolean.class, + null, + "'%s' provider's '%s' field should be a boolean (got a '%s')", + PROVIDER_NAME, + USES_SHARED_LIBRARIES, + EvalUtils.getDataTypeNameFromClass(fieldValue.getClass())); + } + + /** + * Casts and returns the imports field. + * + * @throws EvalException if the field does not exist or is not a depset of strings + */ + public static NestedSet getImports(StructImpl info) throws EvalException { + Object fieldValue = getValue(info, IMPORTS); + SkylarkNestedSet castValue = + SkylarkType.cast( + fieldValue, + SkylarkNestedSet.class, + String.class, + null, + "'%s' provider's '%s' field should be a depset of strings (got a '%s')", + PROVIDER_NAME, + IMPORTS, + EvalUtils.getDataTypeNameFromClass(fieldValue.getClass())); + return castValue.getSet(String.class); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonImportsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonImportsProvider.java index 114acee4362d8a..4e6cd792650cc3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonImportsProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonImportsProvider.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; /** A {@link TransitiveInfoProvider} that supplies import directories for Python dependencies. */ +// TODO(brandjon): Merge this provider into PyProvider. @Immutable @AutoCodec public final class PythonImportsProvider implements TransitiveInfoProvider { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonStarlarkApiTest.java index ecf6befa26cbbd..cd0262903534f7 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonStarlarkApiTest.java @@ -20,15 +20,15 @@ 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.packages.SkylarkInfo; -import com.google.devtools.build.lib.rules.python.PyCommon; +import com.google.devtools.build.lib.packages.StructImpl; +import com.google.devtools.build.lib.rules.python.PyProvider; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import java.io.IOException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Python Starlark APi test */ +/** Python Starlark API test */ @RunWith(JUnit4.class) public class BazelPythonStarlarkApiTest extends BuildViewTestCase { @Test @@ -37,11 +37,11 @@ public void pythonProviderWithFields() throws Exception { assertNoEvents(); ConfiguredTarget helloTarget = getConfiguredTarget("//py:hello"); - SkylarkInfo provider = getPythonSkylarkProvider(helloTarget); + StructImpl provider = PyProvider.getProvider(helloTarget); - assertThat(provider.hasField(PyCommon.TRANSITIVE_PYTHON_SRCS)).isTrue(); - assertThat(provider.hasField(PyCommon.IS_USING_SHARED_LIBRARY)).isTrue(); - assertThat(provider.hasField(PyCommon.IMPORTS)).isTrue(); + assertThat(provider.hasField(PyProvider.TRANSITIVE_SOURCES)).isTrue(); + assertThat(provider.hasField(PyProvider.USES_SHARED_LIBRARIES)).isTrue(); + assertThat(provider.hasField(PyProvider.IMPORTS)).isTrue(); assertThat(provider.hasField("srcs")).isFalse(); } @@ -51,15 +51,14 @@ public void simpleFieldsValues() throws Exception { assertNoEvents(); ConfiguredTarget helloTarget = getConfiguredTarget("//py:hello"); - SkylarkInfo provider = getPythonSkylarkProvider(helloTarget); + StructImpl provider = PyProvider.getProvider(helloTarget); - SkylarkNestedSet sources = - provider.getValue(PyCommon.TRANSITIVE_PYTHON_SRCS, SkylarkNestedSet.class); + SkylarkNestedSet sources = (SkylarkNestedSet) provider.getValue(PyProvider.TRANSITIVE_SOURCES); assertThat(prettyArtifactNames(sources.getSet(Artifact.class))).containsExactly("py/hello.py"); - assertThat(provider.getValue(PyCommon.IS_USING_SHARED_LIBRARY, Boolean.class)).isFalse(); + assertThat((Boolean) provider.getValue(PyProvider.USES_SHARED_LIBRARIES)).isFalse(); - SkylarkNestedSet imports = provider.getValue(PyCommon.IMPORTS, SkylarkNestedSet.class); + SkylarkNestedSet imports = (SkylarkNestedSet) provider.getValue(PyProvider.IMPORTS); assertThat(imports.getSet(String.class)).containsExactly("__main__/py"); } @@ -69,16 +68,15 @@ public void transitiveFieldsValues() throws Exception { assertNoEvents(); ConfiguredTarget helloTarget = getConfiguredTarget("//py:sayHello"); - SkylarkInfo provider = getPythonSkylarkProvider(helloTarget); + StructImpl provider = PyProvider.getProvider(helloTarget); - SkylarkNestedSet sources = - provider.getValue(PyCommon.TRANSITIVE_PYTHON_SRCS, SkylarkNestedSet.class); + SkylarkNestedSet sources = (SkylarkNestedSet) provider.getValue(PyProvider.TRANSITIVE_SOURCES); assertThat(prettyArtifactNames(sources.getSet(Artifact.class))) .containsExactly("py/hello.py", "py/sayHello.py"); - assertThat(provider.getValue(PyCommon.IS_USING_SHARED_LIBRARY, Boolean.class)).isFalse(); + assertThat((Boolean) provider.getValue(PyProvider.USES_SHARED_LIBRARIES)).isFalse(); - SkylarkNestedSet imports = provider.getValue(PyCommon.IMPORTS, SkylarkNestedSet.class); + SkylarkNestedSet imports = (SkylarkNestedSet) provider.getValue(PyProvider.IMPORTS); assertThat(imports.getSet(String.class)).containsExactly("__main__/py"); } @@ -95,13 +93,4 @@ private void simpleSources() throws IOException { "py_binary(name=\"sayHello\", srcs=[\"sayHello.py\"], deps=[\":hello\"])", "py_library(name=\"hello\", srcs=[\"hello.py\"], imports= [\".\"])"); } - - private SkylarkInfo getPythonSkylarkProvider(ConfiguredTarget target) { - Object object = target.get(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME); - assertThat(object).isInstanceOf(SkylarkInfo.class); - SkylarkInfo provider = (SkylarkInfo) object; - - assertThat(provider).isNotNull(); - return provider; - } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD index b3efdc61f514fd..48e28b44a44864 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD @@ -15,6 +15,7 @@ test_suite( ":BazelPythonConfigurationTest", ":PyBinaryConfiguredTargetTest", ":PyLibraryConfiguredTargetTest", + ":PyProviderTest", ":PyTestConfiguredTargetTest", ":PythonConfigurationTest", ":PythonVersionTest", @@ -135,3 +136,21 @@ java_test( "//third_party:truth", ], ) + +java_test( + name = "PyProviderTest", + srcs = ["PyProviderTest.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/build/lib:packages-internal", + "//src/main/java/com/google/devtools/build/lib:python-rules", + "//src/main/java/com/google/devtools/build/lib:syntax", + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + "//src/test/java/com/google/devtools/build/lib:analysis_testutil", + "//src/test/java/com/google/devtools/build/lib:testutil", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderTest.java new file mode 100644 index 00000000000000..7af272b6c1414a --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderTest.java @@ -0,0 +1,262 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.python; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; + +import com.google.common.collect.ImmutableMap; +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.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.packages.StructImpl; +import com.google.devtools.build.lib.packages.StructProvider; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.testutil.MoreAsserts.ThrowingRunnable; +import java.io.IOException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link PyProvider}. */ +@RunWith(JUnit4.class) +public class PyProviderTest extends BuildViewTestCase { + + private static final StructImpl EMPTY_STRUCT = + StructProvider.STRUCT.create(ImmutableMap.of(), "No such attribute '%s'"); + + private static final StructImpl WRONG_TYPES_STRUCT = + StructProvider.STRUCT.create( + ImmutableMap.of( + PyProvider.TRANSITIVE_SOURCES, + 123, + PyProvider.USES_SHARED_LIBRARIES, + 123, + PyProvider.IMPORTS, + 123), + "No such attribute '%s'"); + + private StructImpl getGoodStruct() { + return StructProvider.STRUCT.create( + ImmutableMap.of( + PyProvider.TRANSITIVE_SOURCES, + SkylarkNestedSet.of( + Artifact.class, + NestedSetBuilder.create(Order.STABLE_ORDER, getSourceArtifact("dummy_artifact"))), + PyProvider.USES_SHARED_LIBRARIES, + true, + PyProvider.IMPORTS, + SkylarkNestedSet.of(String.class, NestedSetBuilder.create(Order.STABLE_ORDER, "abc"))), + "No such attribute '%s'"); + } + + /** Defines //pytarget, a target that returns a py provider with some arbitrary field values. */ + private void definePyTarget() throws IOException { + scratch.file("rules/BUILD"); + scratch.file( + "rules/pyrule.bzl", + "def _pyrule_impl(ctx):", + " info = struct(", + " transitive_sources = depset(direct=ctx.files.transitive_sources),", + " uses_shared_libraries = ctx.attr.uses_shared_libraries,", + " imports = depset(direct=ctx.attr.imports),", + " )", + " return struct(py=info)", + "", + "pyrule = rule(", + " implementation = _pyrule_impl,", + " attrs = {", + " 'transitive_sources': attr.label_list(allow_files=True),", + " 'uses_shared_libraries': attr.bool(default=False),", + " 'imports': attr.string_list(),", + " },", + ")"); + scratch.file( + "pytarget/BUILD", + "load('//rules:pyrule.bzl', 'pyrule')", + "", + "pyrule(", + " name = 'pytarget',", + " transitive_sources = ['a.py'],", + " uses_shared_libraries = True,", + " imports = ['b']", + ")"); + scratch.file("pytarget/a.py"); + } + + /** Defines //dummytarget, a target that returns no py provider. */ + private void defineDummyTarget() throws IOException { + scratch.file("rules/BUILD"); + scratch.file( + "rules/dummytarget.bzl", + "def _dummyrule_impl(ctx):", + " pass", + "", + "dummyrule = rule(", + " implementation = _dummyrule_impl,", + ")"); + scratch.file( + "dummytarget/BUILD", + "load('//rules:dummytarget.bzl', 'dummyrule')", + "", + "dummyrule(", + " name = 'dummytarget',", + ")"); + } + + @Test + public void hasProvider_True() throws Exception { + definePyTarget(); + assertThat(PyProvider.hasProvider(getConfiguredTarget("//pytarget"))).isTrue(); + } + + @Test + public void hasProvider_False() throws Exception { + defineDummyTarget(); + assertThat(PyProvider.hasProvider(getConfiguredTarget("//dummytarget"))).isFalse(); + } + + @Test + public void getProvider_Present() throws Exception { + definePyTarget(); + StructImpl info = PyProvider.getProvider(getConfiguredTarget("//pytarget")); + // If we got this far, it's present. getProvider() should never be null, but check just in case. + assertThat(info).isNotNull(); + } + + @Test + public void getProvider_Absent() throws Exception { + defineDummyTarget(); + EvalException ex = + assertThrows( + EvalException.class, + () -> PyProvider.getProvider(getConfiguredTarget("//dummytarget"))); + assertThat(ex).hasMessageThat().contains("Target does not have 'py' provider"); + } + + @Test + public void getProvider_WrongType() throws Exception { + // badtyperule() returns a "py" provider that has the wrong type. + scratch.file("rules/BUILD"); + scratch.file( + "rules/badtyperule.bzl", + "def _badtyperule_impl(ctx):", + " return struct(py='abc')", + "", + "badtyperule = rule(", + " implementation = _badtyperule_impl", + ")"); + scratch.file( + "badtypetarget/BUILD", + "load('//rules:badtyperule.bzl', 'badtyperule')", + "", + "badtyperule(", + " name = 'badtypetarget',", + ")"); + EvalException ex = + assertThrows( + EvalException.class, + () -> PyProvider.getProvider(getConfiguredTarget("//badtypetarget"))); + assertThat(ex).hasMessageThat().contains("'py' provider should be a struct"); + } + + private static void assertThrowsEvalExceptionContaining( + ThrowingRunnable runnable, String message) { + assertThat(assertThrows(EvalException.class, runnable)).hasMessageThat().contains(message); + } + + private static void assertHasMissingFieldMessage(ThrowingRunnable access, String fieldName) { + assertThrowsEvalExceptionContaining( + access, String.format("\'py' provider missing '%s' field", fieldName)); + } + + private static void assertHasWrongTypeMessage( + ThrowingRunnable access, String fieldName, String expectedType) { + assertThrowsEvalExceptionContaining( + access, + String.format( + "\'py' provider's '%s' field should be a %s (got a 'int')", fieldName, expectedType)); + } + + @Test + public void getTransitiveSources() throws Exception { + assertHasMissingFieldMessage( + () -> PyProvider.getTransitiveSources(EMPTY_STRUCT), "transitive_sources"); + assertHasWrongTypeMessage( + () -> PyProvider.getTransitiveSources(WRONG_TYPES_STRUCT), + "transitive_sources", + "depset of Files"); + assertThat(PyProvider.getTransitiveSources(getGoodStruct())) + .containsExactly(getSourceArtifact("dummy_artifact")); + } + + @Test + public void getTransitiveSources_OrderMismatch() throws Exception { + reporter.removeHandler(failFastHandler); + // Depset order mismatches should be caught as rule errors. + scratch.file("rules/BUILD"); + scratch.file( + "rules/badorderrule.bzl", + "def _badorderrule_impl(ctx):", + // Native rules use "compile" / "postorder", so using "preorder" here creates a conflict. + " info = struct(transitive_sources=depset(direct=[], order='preorder'))", + " return struct(py=info)", + "", + "badorderrule = rule(", + " implementation = _badorderrule_impl", + ")"); + scratch.file( + "badordertarget/BUILD", + "load('//rules:badorderrule.bzl', 'badorderrule')", + "", + "badorderrule(", + " name = 'badorderdep',", + ")", + "py_library(", + " name = 'pylib',", + " srcs = ['pylib.py'],", + ")", + "py_binary(", + " name = 'pybin',", + " srcs = ['pybin.py'],", + " deps = [':pylib', ':badorderdep'],", + ")"); + getConfiguredTarget("//badordertarget:pybin"); + assertContainsEvent( + "Incompatible order for transitive_sources: expected 'default' or 'postorder', got " + + "'preorder'"); + } + + @Test + public void getUsesSharedLibraries() throws Exception { + assertHasMissingFieldMessage( + () -> PyProvider.getUsesSharedLibraries(EMPTY_STRUCT), "uses_shared_libraries"); + assertHasWrongTypeMessage( + () -> PyProvider.getUsesSharedLibraries(WRONG_TYPES_STRUCT), + "uses_shared_libraries", + "boolean"); + assertThat(PyProvider.getUsesSharedLibraries(getGoodStruct())).isTrue(); + } + + @Test + public void getImports() throws Exception { + assertHasMissingFieldMessage(() -> PyProvider.getImports(EMPTY_STRUCT), "imports"); + assertHasWrongTypeMessage( + () -> PyProvider.getImports(WRONG_TYPES_STRUCT), "imports", "depset of strings"); + assertThat(PyProvider.getImports(getGoodStruct())).containsExactly("abc"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index 8706d97e83cfe1..8f9e1dc4e56742 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -44,7 +44,7 @@ import com.google.devtools.build.lib.packages.StructImpl; import com.google.devtools.build.lib.rules.java.JavaInfo; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; -import com.google.devtools.build.lib.rules.python.PyCommon; +import com.google.devtools.build.lib.rules.python.PyProvider; import com.google.devtools.build.lib.skylark.util.SkylarkTestCase; import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkDict; @@ -492,7 +492,7 @@ public void shouldGetPrerequisites() throws Exception { TransitiveInfoCollection tic1 = (TransitiveInfoCollection) ((SkylarkList) result).get(0); assertThat(JavaInfo.getProvider(JavaSourceJarsProvider.class, tic1)).isNotNull(); // Check an unimplemented provider too - assertThat(tic1.get(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME)).isNull(); + assertThat(PyProvider.hasProvider(tic1)).isFalse(); } @Test