From 4b3725e1a8d71173e0c8d9864d09ece8affac80b Mon Sep 17 00:00:00 2001 From: brandjon Date: Thu, 24 Jan 2019 14:34:04 -0800 Subject: [PATCH] Add helper for abstracting over Python provider representation and defaults This creates PyProviderUtils, a collection of helpers that take in a TransitiveInfoCollection and return fields obtained from the Python provider. Currently this just dispatches to PyStructUtils to retrieve from the legacy "py" struct provider. In a follow-up CL, we'll add a modern PyInfo type, and PyProviderUtils will access either one as appropriate. PyProviderUtils also takes over the default logic from PyCommon, for when no Python provider information is available. Work toward #7010. RELNOTES: None PiperOrigin-RevId: 230791581 --- .../build/lib/rules/python/PyCommon.java | 87 ++++----- .../lib/rules/python/PyProviderUtils.java | 150 ++++++++++++++++ .../build/lib/rules/python/PyStructUtils.java | 28 +-- .../devtools/build/lib/rules/python/BUILD | 20 ++- .../lib/rules/python/PyProviderUtilsTest.java | 165 ++++++++++++++++++ .../lib/rules/python/PyStructUtilsTest.java | 145 ++------------- .../rules/python/PythonStarlarkApiTest.java | 2 +- .../lib/skylark/SkylarkRuleContextTest.java | 4 +- 8 files changed, 388 insertions(+), 213 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/python/PyProviderUtils.java create mode 100644 src/test/java/com/google/devtools/build/lib/rules/python/PyProviderUtilsTest.java 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 f4c58cfa1ce2dd..7fcd79b3e76a36 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 @@ -43,7 +43,6 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.packages.StructImpl; import com.google.devtools.build.lib.rules.cpp.CcInfo; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -230,20 +229,11 @@ private static NestedSet initTransitivePythonSources(RuleContext ruleC private static void collectTransitivePythonSourcesFromDeps( RuleContext ruleContext, NestedSetBuilder builder) { for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { - if (PyStructUtils.hasProvider(dep)) { - try { - StructImpl info = PyStructUtils.getProvider(dep); - NestedSet sources = PyStructUtils.getTransitiveSources(info); - 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). We should have rules implement a "PythonSourcesProvider" instead. - FileProvider provider = dep.getProvider(FileProvider.class); - builder.addAll(FileType.filter(provider.getFilesToBuild(), PyRuleClasses.PYTHON_SOURCE)); + try { + builder.addTransitive(PyProviderUtils.getTransitiveSources(dep)); + } catch (EvalException e) { + // Either the provider type or field type is bad. + ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); } } } @@ -268,48 +258,36 @@ private static boolean initUsesSharedLibraries(RuleContext ruleContext) { } else { targets = ruleContext.getPrerequisites("deps", Mode.TARGET); } - try { - for (TransitiveInfoCollection target : targets) { - if (checkForSharedLibraries(target)) { + for (TransitiveInfoCollection target : targets) { + try { + if (PyProviderUtils.getUsesSharedLibraries(target)) { return true; } + } catch (EvalException e) { + ruleContext.ruleError(String.format("In dep '%s': %s", target.getLabel(), e.getMessage())); } - return false; - } catch (EvalException e) { - ruleContext.ruleError(e.getMessage()); - return false; - } - } - - private static boolean checkForSharedLibraries(TransitiveInfoCollection target) - throws EvalException { - if (PyStructUtils.hasProvider(target)) { - return PyStructUtils.getUsesSharedLibraries(PyStructUtils.getProvider(target)); - } else { - NestedSet files = target.getProvider(FileProvider.class).getFilesToBuild(); - return FileType.contains(files, CppFileTypes.SHARED_LIBRARY); } + return false; } private static NestedSet initImports(RuleContext ruleContext, PythonSemantics semantics) { NestedSetBuilder builder = NestedSetBuilder.compileOrder(); builder.addAll(semantics.getImports(ruleContext)); - for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { - if (dep.getProvider(PythonImportsProvider.class) != null) { - PythonImportsProvider provider = dep.getProvider(PythonImportsProvider.class); - NestedSet imports = provider.getTransitivePythonImports(); + try { + NestedSet imports = PyProviderUtils.getImports(dep); 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. + // TODO(brandjon): We should make order an invariant of the Python provider. Then once we + /// remove PythonImportsProvider we can move this check into PyProvider/PyStructUtils. ruleContext.ruleError( getOrderErrorMessage(PyStructUtils.IMPORTS, builder.getOrder(), imports.getOrder())); } else { builder.addTransitive(imports); } + } catch (EvalException e) { + ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); } } - return builder.build(); } @@ -324,14 +302,12 @@ private static boolean initHasPy2OnlySources( return true; } for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { - if (PyStructUtils.hasProvider(dep)) { - try { - if (PyStructUtils.getHasPy2OnlySources(PyStructUtils.getProvider(dep))) { - return true; - } - } catch (EvalException e) { - ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); + try { + if (PyProviderUtils.getHasPy2OnlySources(dep)) { + return true; } + } catch (EvalException e) { + ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); } } return false; @@ -347,14 +323,12 @@ private static boolean initHasPy3OnlySources( return true; } for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { - if (PyStructUtils.hasProvider(dep)) { - try { - if (PyStructUtils.getHasPy3OnlySources(PyStructUtils.getProvider(dep))) { - return true; - } - } catch (EvalException e) { - ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); + try { + if (PyProviderUtils.getHasPy3OnlySources(dep)) { + return true; } + } catch (EvalException e) { + ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage())); } } return false; @@ -617,10 +591,9 @@ public void addCommonTransitiveInfoProviders( */ public List validateSrcs() { List sourceFiles = new ArrayList<>(); - // TODO(bazel-team): Need to get the transitive deps closure, not just the - // sources of the rule. - for (TransitiveInfoCollection src : ruleContext - .getPrerequisitesIf("srcs", Mode.TARGET, FileProvider.class)) { + // TODO(bazel-team): Need to get the transitive deps closure, not just the sources of the rule. + for (TransitiveInfoCollection src : + ruleContext.getPrerequisitesIf("srcs", Mode.TARGET, FileProvider.class)) { // Make sure that none of the sources contain hyphens. if (Util.containsHyphen(src.getLabel().getPackageFragment())) { ruleContext.attributeError("srcs", diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyProviderUtils.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyProviderUtils.java new file mode 100644 index 00000000000000..9e0dbe20e9c55e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyProviderUtils.java @@ -0,0 +1,150 @@ +// Copyright 2019 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.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +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.StructImpl; +import com.google.devtools.build.lib.rules.cpp.CppFileTypes; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.SkylarkType; +import com.google.devtools.build.lib.util.FileType; + +/** + * Static helper class for creating and accessing Python provider information. + * + *

This class exposes a unified view over both the legacy and modern Python providers. + */ +public class PyProviderUtils { + + // Disable construction. + private PyProviderUtils() {} + + /** Returns whether a given target has the py provider. */ + public static boolean hasProvider(TransitiveInfoCollection target) { + return target.get(PyStructUtils.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 target) throws EvalException { + Object info = target.get(PyStructUtils.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", + PyStructUtils.PROVIDER_NAME); + } + + /** + * Returns the transitive sources of a given target. + * + *

If the target has a py provider, the value from that provider is used. Otherwise, we fall + * back on collecting .py source files from the target's filesToBuild. + * + * @throws EvalException if the legacy struct provider is present but malformed + */ + // TODO(bazel-team): Eliminate the fallback behavior by returning an appropriate py provider from + // the relevant rules. + public static NestedSet getTransitiveSources(TransitiveInfoCollection target) + throws EvalException { + if (hasProvider(target)) { + return PyStructUtils.getTransitiveSources(getProvider(target)); + } else { + NestedSet files = target.getProvider(FileProvider.class).getFilesToBuild(); + return NestedSetBuilder.compileOrder() + .addAll(FileType.filter(files, PyRuleClasses.PYTHON_SOURCE)) + .build(); + } + } + + /** + * Returns whether a target uses shared libraries. + * + *

If the target has a py provider, the value from that provider is used. Otherwise, we fall + * back on checking whether the target's filesToBuild contains a shared library file type (e.g., a + * .so file). + * + * @throws EvalException if the legacy struct provider is present but malformed + */ + public static boolean getUsesSharedLibraries(TransitiveInfoCollection target) + throws EvalException { + if (hasProvider(target)) { + return PyStructUtils.getUsesSharedLibraries(getProvider(target)); + } else { + NestedSet files = target.getProvider(FileProvider.class).getFilesToBuild(); + return FileType.contains(files, CppFileTypes.SHARED_LIBRARY); + } + } + + /** + * Returns the transitive import paths of a target. + * + *

Imports are not currently propagated correctly (#7054). Currently the behavior is to return + * the imports contained in the target's {@link PythonImportsProvider}, ignoring the py provider, + * or no imports if there is no {@code PythonImportsProvider}. When #7054 is fixed, we'll instead + * return the imports specified by the py provider, or those from {@code PythonImportsProvider} if + * the py provider is not present, with an eventual goal of removing {@code + * PythonImportsProvider}. + */ + // TODO(#7054) Implement the above change. + public static NestedSet getImports(TransitiveInfoCollection target) throws EvalException { + PythonImportsProvider importsProvider = target.getProvider(PythonImportsProvider.class); + if (importsProvider != null) { + return importsProvider.getTransitivePythonImports(); + } else { + return NestedSetBuilder.emptySet(Order.COMPILE_ORDER); + } + } + + /** + * Returns whether the target has transitive sources requiring Python 2. + * + *

If the target has a py provider, the value from that provider is used. Otherwise, we default + * to false. + */ + public static boolean getHasPy2OnlySources(TransitiveInfoCollection target) throws EvalException { + if (hasProvider(target)) { + return PyStructUtils.getHasPy2OnlySources(getProvider(target)); + } else { + return false; + } + } + + /** + * Returns whether the target has transitive sources requiring Python 3. + * + *

If the target has a py provider, the value from that provider is used. Otherwise, we default + * to false. + */ + public static boolean getHasPy3OnlySources(TransitiveInfoCollection target) throws EvalException { + if (hasProvider(target)) { + return PyStructUtils.getHasPy3OnlySources(getProvider(target)); + } else { + return false; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyStructUtils.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyStructUtils.java index 9adcfec359747e..5e3ae8c52e45a4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyStructUtils.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyStructUtils.java @@ -17,7 +17,6 @@ import com.google.common.base.Preconditions; 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.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -28,7 +27,7 @@ import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.SkylarkType; -/** Static helper class for creating and accessing instances of the "py" legacy struct provider. */ +/** Static helper class for creating and accessing instances of the legacy "py" struct provider. */ // TODO(#7010): Replace this with a real provider. public class PyStructUtils { @@ -54,9 +53,7 @@ private PyStructUtils() {} * target). */ // TODO(brandjon): Make this a pre-order depset, since higher-level targets should get precedence - // on PYTHONPATH. - // TODO(brandjon): Add assertions that this depset and transitive_sources have an order compatible - // with the one expected by the rules. + // on PYTHONPATH. Add assertions on its order compatibility. public static final String IMPORTS = "imports"; /** @@ -85,25 +82,6 @@ private PyStructUtils() {} DEFAULTS = builder.build(); } - /** 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) { @@ -220,7 +198,7 @@ public static Builder builder() { return new Builder(); } - /** Builder for a py provider struct. */ + /** Builder for a legacy py provider struct. */ public static class Builder { SkylarkNestedSet transitiveSources = null; Boolean usesSharedLibraries = null; 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 39a4d5d5da2584..73d619aea6a0bb 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", + ":PyProviderUtilsTest", ":PyStructUtilsTest", ":PyTestConfiguredTargetTest", ":PythonConfigurationTest", @@ -155,17 +156,32 @@ java_test( ], ) +java_test( + name = "PyProviderUtilsTest", + srcs = ["PyProviderUtilsTest.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/build/lib:python-rules", + "//src/main/java/com/google/devtools/build/lib:syntax", + "//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", + ], +) + java_test( name = "PyStructUtilsTest", srcs = ["PyStructUtilsTest.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/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/test/java/com/google/devtools/build/lib:testutil", "//third_party:guava", "//third_party:junit4", diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderUtilsTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderUtilsTest.java new file mode 100644 index 00000000000000..5edc536fe8c798 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderUtilsTest.java @@ -0,0 +1,165 @@ +// Copyright 2019 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.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.testutil.MoreAsserts.ThrowingRunnable; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link PyProviderUtils}. */ +@RunWith(JUnit4.class) +public class PyProviderUtilsTest extends BuildViewTestCase { + + private static void assertThrowsEvalExceptionContaining( + ThrowingRunnable runnable, String message) { + assertThat(assertThrows(EvalException.class, runnable)).hasMessageThat().contains(message); + } + + /** + * Creates {@code //pkg:target} as an instance of a trivial rule having the given implementation + * function body. + */ + private void declareTargetWithImplementation(String... lines) throws Exception { + String body; + if (lines.length == 0) { + body = " pass"; + } else { + body = " " + String.join("\n ", lines); + } + scratch.file( + "pkg/rules.bzl", + "def _myrule_impl(ctx):", + body, + "myrule = rule(", + " implementation = _myrule_impl,", + ")"); + scratch.file( + "pkg/BUILD", // + "load(':rules.bzl', 'myrule')", // + "myrule(", // + " name = 'target',", // + ")"); + } + + /** Retrieves the target defined by {@link #declareTargetWithImplementation}. */ + private ConfiguredTarget getTarget() throws Exception { + ConfiguredTarget target = getConfiguredTarget("//pkg:target"); + assertThat(target).isNotNull(); + return target; + } + + @Test + public void getAndHasProvider_Present() throws Exception { + declareTargetWithImplementation( // + "return struct(py=struct())"); // Accessor doesn't actually check fields + assertThat(PyProviderUtils.hasProvider(getTarget())).isTrue(); + assertThat(PyProviderUtils.getProvider(getTarget())).isNotNull(); + } + + @Test + public void getAndHasProvider_Absent() throws Exception { + declareTargetWithImplementation( // + "return []"); + assertThat(PyProviderUtils.hasProvider(getTarget())).isFalse(); + assertThrowsEvalExceptionContaining( + () -> PyProviderUtils.getProvider(getTarget()), "Target does not have 'py' provider"); + } + + @Test + public void getProvider_NotAStruct() throws Exception { + declareTargetWithImplementation( // + "return struct(py=123)"); + assertThrowsEvalExceptionContaining( + () -> PyProviderUtils.getProvider(getTarget()), "'py' provider should be a struct"); + } + + @Test + public void getTransitiveSources_Provider() throws Exception { + declareTargetWithImplementation( + "afile = ctx.actions.declare_file('a.py')", + "ctx.actions.write(output=afile, content='a')", + "info = struct(transitive_sources = depset(direct=[afile]))", + "return struct(py=info)"); + assertThat(PyProviderUtils.getTransitiveSources(getTarget())) + .containsExactly(getBinArtifact("a.py", getTarget())); + } + + @Test + public void getTransitiveSources_NoProvider() throws Exception { + declareTargetWithImplementation( + "afile = ctx.actions.declare_file('a.py')", + "ctx.actions.write(output=afile, content='a')", + "return [DefaultInfo(files=depset(direct=[afile]))]"); + assertThat(PyProviderUtils.getTransitiveSources(getTarget())) + .containsExactly(getBinArtifact("a.py", getTarget())); + } + + @Test + public void getUsesSharedLibraries_Provider() throws Exception { + declareTargetWithImplementation( + "info = struct(transitive_sources = depset([]), uses_shared_libraries=True)", + "return struct(py=info)"); + assertThat(PyProviderUtils.getUsesSharedLibraries(getTarget())).isTrue(); + } + + @Test + public void getUsesSharedLibraries_NoProvider() throws Exception { + declareTargetWithImplementation( + "afile = ctx.actions.declare_file('a.so')", + "ctx.actions.write(output=afile, content='a')", + "return [DefaultInfo(files=depset(direct=[afile]))]"); + assertThat(PyProviderUtils.getUsesSharedLibraries(getTarget())).isTrue(); + } + + // TODO(#7054): Add tests for getImports once we actually read the Python provider instead of the + // specialized PythonImportsProvider. + + @Test + public void getHasPy2OnlySources_Provider() throws Exception { + declareTargetWithImplementation( + "info = struct(transitive_sources = depset([]), has_py2_only_sources=True)", + "return struct(py=info)"); + assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isTrue(); + } + + @Test + public void getHasPy2OnlySources_NoProvider() throws Exception { + declareTargetWithImplementation( // + "return []"); + assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isFalse(); + } + + @Test + public void getHasPy3OnlySources_Provider() throws Exception { + declareTargetWithImplementation( + "info = struct(transitive_sources = depset([]), has_py3_only_sources=True)", + "return struct(py=info)"); + assertThat(PyProviderUtils.getHasPy3OnlySources(getTarget())).isTrue(); + } + + @Test + public void getHasPy3OnlySources_NoProvider() throws Exception { + declareTargetWithImplementation( // + "return []"); + assertThat(PyProviderUtils.getHasPy3OnlySources(getTarget())).isFalse(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyStructUtilsTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyStructUtilsTest.java index a4e0f88cf72e30..c555ed7b211f90 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PyStructUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyStructUtilsTest.java @@ -19,7 +19,7 @@ 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.actions.ArtifactRoot; 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; @@ -27,17 +27,29 @@ 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.FoundationTestCase; import com.google.devtools.build.lib.testutil.MoreAsserts.ThrowingRunnable; -import java.io.IOException; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Root; import java.util.LinkedHashMap; import java.util.Map; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; /** Tests for {@link PyStructUtils}. */ @RunWith(JUnit4.class) -public class PyStructUtilsTest extends BuildViewTestCase { +public class PyStructUtilsTest extends FoundationTestCase { + + private Artifact dummyArtifact; + + @Before + public void setUp() { + this.dummyArtifact = + new Artifact( + PathFragment.create("dummy"), ArtifactRoot.asSourceRoot(Root.fromPath(rootDirectory))); + } /** * Constructs a py provider struct with the given field values and with default values for any @@ -63,122 +75,6 @@ private StructImpl makeStruct(Map overrides) { return StructProvider.STRUCT.create(fields, "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),", - " has_py2_only_sources = ctx.attr.has_py2_only_sources,", - " has_py3_only_sources = ctx.attr.has_py3_only_sources,", - " )", - " 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(),", - " 'has_py2_only_sources': attr.bool(),", - " 'has_py3_only_sources': attr.bool(),", - " },", - ")"); - scratch.file( - "pytarget/BUILD", - "load('//rules:pyrule.bzl', 'pyrule')", - "", - "pyrule(", - " name = 'pytarget',", - " transitive_sources = ['a.py'],", - " uses_shared_libraries = True,", - " imports = ['b'],", - " has_py2_only_sources = True,", - " has_py3_only_sources = True,", - ")"); - 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(PyStructUtils.hasProvider(getConfiguredTarget("//pytarget"))).isTrue(); - } - - @Test - public void hasProvider_False() throws Exception { - defineDummyTarget(); - assertThat(PyStructUtils.hasProvider(getConfiguredTarget("//dummytarget"))).isFalse(); - } - - @Test - public void getProvider_Present() throws Exception { - definePyTarget(); - StructImpl info = PyStructUtils.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, - () -> PyStructUtils.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, - () -> PyStructUtils.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); @@ -206,8 +102,7 @@ private static void assertHasOrderAndContainsExactly( @Test public void getTransitiveSources_Good() throws Exception { - NestedSet sources = - NestedSetBuilder.create(Order.COMPILE_ORDER, getSourceArtifact("dummy")); + NestedSet sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact); StructImpl info = makeStruct( ImmutableMap.of( @@ -323,8 +218,7 @@ public void getHasPy3OnlySources_WrongType() { /** Checks values set by the builder. */ @Test public void builder() throws Exception { - NestedSet sources = - NestedSetBuilder.create(Order.COMPILE_ORDER, getSourceArtifact("dummy")); + NestedSet sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact); NestedSet imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc"); StructImpl info = PyStructUtils.builder() @@ -339,7 +233,7 @@ public void builder() throws Exception { assertHasOrderAndContainsExactly( ((SkylarkNestedSet) info.getValue(PyStructUtils.TRANSITIVE_SOURCES)).getSet(Artifact.class), Order.COMPILE_ORDER, - getSourceArtifact("dummy")); + dummyArtifact); assertThat((Boolean) info.getValue(PyStructUtils.USES_SHARED_LIBRARIES)).isTrue(); assertHasOrderAndContainsExactly( ((SkylarkNestedSet) info.getValue(PyStructUtils.IMPORTS)).getSet(String.class), @@ -353,8 +247,7 @@ public void builder() throws Exception { @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, getSourceArtifact("dummy")); + NestedSet sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact); StructImpl info = PyStructUtils.builder().setTransitiveSources(sources).build(); // Assert using struct operations, not PyStructUtils accessors, which aren't necessarily trusted // to be correct. 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 d8892fe44673dc..71ed741fb2b646 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 @@ -92,7 +92,7 @@ public void librarySandwich() throws Exception { " srcs = ['upperuserlib.py'],", " deps = [':pylib'],", ")"); - StructImpl info = PyStructUtils.getProvider(getConfiguredTarget("//pkg:upperuserlib")); + StructImpl info = PyProviderUtils.getProvider(getConfiguredTarget("//pkg:upperuserlib")); assertThat(PyStructUtils.getTransitiveSources(info)) .containsExactly( getSourceArtifact("pkg/loweruserlib.py"), 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 c9fb551b63815b..c38ad4b9e229f4 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.PyStructUtils; +import com.google.devtools.build.lib.rules.python.PyProviderUtils; 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(PyStructUtils.hasProvider(tic1)).isFalse(); + assertThat(PyProviderUtils.hasProvider(tic1)).isFalse(); } @Test