Make `--incompatible_disallow_legacy_py_provider` a no-op
The legacy `py` provider struct is now silently ignored instead of causing
an error.
Cleanup for #7741
PiperOrigin-RevId: 449568980
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 435914a..fedc65c 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
@@ -32,12 +32,10 @@
import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
-import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
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.PyInfo;
import com.google.devtools.build.lib.rules.python.PyRuleClasses;
-import com.google.devtools.build.lib.rules.python.PyStructUtils;
import com.google.devtools.build.lib.rules.python.PythonVersion;
/**
@@ -69,17 +67,7 @@
<a href="${link py_library}"><code>py_library</code></a> rules.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.override(
- builder
- .copy("deps")
- .mandatoryProvidersList(
- ImmutableList.of(
- // Legacy provider.
- // TODO(b/153363654): Remove this legacy set.
- ImmutableList.of(
- StarlarkProviderIdentifier.forLegacy(PyStructUtils.PROVIDER_NAME)),
- // Modern provider.
- ImmutableList.of(PyInfo.PROVIDER.id())))
- .allowedFileTypes())
+ builder.copy("deps").mandatoryProviders(PyInfo.PROVIDER.id()).allowedFileTypes())
/* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->
List of import directories to be added to the <code>PYTHONPATH</code>.
<p>
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 8a76a4a..a1b5ce1 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
@@ -207,7 +207,6 @@
this.hasPy3OnlySources = initHasPy3OnlySources(ruleContext, this.sourcesVersion);
this.runtimeFromToolchain = initRuntimeFromToolchain(ruleContext, this.version);
validatePythonVersionAttr();
- validateLegacyProviderNotUsedIfDisabled();
}
/** Returns the parsed value of the "srcs_version" attribute. */
@@ -244,17 +243,28 @@
/**
* Gathers transitive .py files from {@code deps} (not including this target's {@code srcs} and
* adds them to {@code builder}.
+ *
+ * <p>If a target has the PyInfo provider, the value from that provider is used. Otherwise, we
+ * fall back on collecting .py source files from the target's filesToBuild.
*/
+ // TODO(bazel-team): Eliminate the fallback behavior by returning an appropriate py provider from
+ // the relevant rules.
private static void collectTransitivePythonSourcesFromDeps(
RuleContext ruleContext, NestedSetBuilder<Artifact> builder) {
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
- try {
- builder.addTransitive(PyProviderUtils.getTransitiveSources(dep));
- } catch (EvalException e) {
- // Either the provider type or field type is bad.
- ruleContext.attributeError(
- "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+ NestedSet<Artifact> sources;
+ if (dep.get(PyInfo.PROVIDER) != null) {
+ sources = dep.get(PyInfo.PROVIDER).getTransitiveSourcesSet();
+ } else {
+ sources =
+ NestedSetBuilder.<Artifact>compileOrder()
+ .addAll(
+ FileType.filter(
+ dep.getProvider(FileProvider.class).getFilesToBuild().toList(),
+ PyRuleClasses.PYTHON_SOURCE))
+ .build();
}
+ builder.addTransitive(sources);
}
}
@@ -308,34 +318,42 @@
targets = ruleContext.getPrerequisites("deps");
}
for (TransitiveInfoCollection target : targets) {
- try {
- if (PyProviderUtils.getUsesSharedLibraries(target)) {
+ if (target.get(PyInfo.PROVIDER) != null) {
+ if (target.get(PyInfo.PROVIDER).getUsesSharedLibraries()) {
return true;
}
- } catch (EvalException e) {
- ruleContext.ruleError(String.format("In dep '%s': %s", target.getLabel(), e.getMessage()));
+ } else if (FileType.contains(
+ target.getProvider(FileProvider.class).getFilesToBuild().toList(),
+ CppFileTypes.SHARED_LIBRARY)) {
+ return true;
}
}
return false;
}
+ /**
+ * Returns the transitive import paths of a target.
+ *
+ * <p>For targets with the PyInfo provider, the value from that provider is used. Otherwise, we
+ * default to an empty set.
+ */
private static NestedSet<String> initImports(RuleContext ruleContext, PythonSemantics semantics) {
NestedSetBuilder<String> builder = NestedSetBuilder.compileOrder();
builder.addAll(semantics.getImports(ruleContext));
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
- try {
- NestedSet<String> imports = PyProviderUtils.getImports(dep);
- if (!builder.getOrder().isCompatible(imports.getOrder())) {
- // TODO(brandjon): We should make order an invariant of the Python provider, and move this
- // check into PyInfo/PyStructUtils.
- ruleContext.ruleError(
- getOrderErrorMessage(PyStructUtils.IMPORTS, builder.getOrder(), imports.getOrder()));
- } else {
- builder.addTransitive(imports);
- }
- } catch (EvalException e) {
- ruleContext.attributeError(
- "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+ NestedSet<String> imports;
+ if (dep.get(PyInfo.PROVIDER) != null) {
+ imports = dep.get(PyInfo.PROVIDER).getImportsSet();
+ } else {
+ imports = NestedSetBuilder.emptySet(Order.COMPILE_ORDER);
+ }
+ if (!builder.getOrder().isCompatible(imports.getOrder())) {
+ // TODO(brandjon): We should make order an invariant of the Python provider, and move this
+ // check into PyInfo.
+ ruleContext.ruleError(
+ getOrderErrorMessage("imports", builder.getOrder(), imports.getOrder()));
+ } else {
+ builder.addTransitive(imports);
}
}
return builder.build();
@@ -350,14 +368,9 @@
if (sourcesVersion == PythonVersion.PY2 || sourcesVersion == PythonVersion.PY2ONLY) {
return true;
}
- for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
- try {
- if (PyProviderUtils.getHasPy2OnlySources(dep)) {
- return true;
- }
- } catch (EvalException e) {
- ruleContext.attributeError(
- "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+ for (PyInfo depInfo : ruleContext.getPrerequisites("deps", PyInfo.PROVIDER)) {
+ if (depInfo.getHasPy2OnlySources()) {
+ return true;
}
}
return false;
@@ -372,14 +385,9 @@
if (sourcesVersion == PythonVersion.PY3 || sourcesVersion == PythonVersion.PY3ONLY) {
return true;
}
- for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
- try {
- if (PyProviderUtils.getHasPy3OnlySources(dep)) {
- return true;
- }
- } catch (EvalException e) {
- ruleContext.attributeError(
- "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+ for (PyInfo depInfo : ruleContext.getPrerequisites("deps", PyInfo.PROVIDER)) {
+ if (depInfo.getHasPy3OnlySources()) {
+ return true;
}
}
return false;
@@ -524,27 +532,6 @@
}
/**
- * Reports an attribute error if a target in {@code deps} passes the legacy "py" provider but this
- * is disallowed by the configuration.
- */
- private void validateLegacyProviderNotUsedIfDisabled() {
- if (!ruleContext.getFragment(PythonConfiguration.class).disallowLegacyPyProvider()) {
- return;
- }
- for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
- if (PyProviderUtils.hasLegacyProvider(dep)) {
- ruleContext.attributeError(
- "deps",
- String.format(
- "In dep '%s': The legacy 'py' provider is disallowed. Migrate to the PyInfo "
- + "provider instead. You can temporarily disable this failure with "
- + "--incompatible_disallow_legacy_py_provider=false.",
- dep.getLabel()));
- }
- }
- }
-
- /**
* If the Python version (as determined by the configuration) is inconsistent with {@link
* #hasPy2OnlySources} or {@link #hasPy3OnlySources}, emits a {@link FailAction} that "generates"
* the executable.
@@ -752,7 +739,9 @@
addPyExtraActionPseudoAction();
}
- /** @return an artifact next to the executable file with a given suffix. */
+ /**
+ * @return an artifact next to the executable file with a given suffix.
+ */
private Artifact getArtifactWithExtension(Artifact executable, String extension) {
// On Windows, the Python executable has .exe extension on Windows,
// On Linux, the Python executable has no extension.
@@ -793,21 +782,20 @@
}
/**
- * Adds a PyInfo or legacy "py" provider.
+ * Adds a PyInfo provider.
*
* <p>This is a public method because some rules just want a PyInfo provider without the other
* things py_library needs.
*/
public void addPyInfoProvider(RuleConfiguredTargetBuilder builder) {
- boolean createLegacyPyProvider =
- !ruleContext.getFragment(PythonConfiguration.class).disallowLegacyPyProvider();
- PyProviderUtils.builder(createLegacyPyProvider)
- .setTransitiveSources(transitivePythonSources)
- .setUsesSharedLibraries(usesSharedLibraries)
- .setImports(imports)
- .setHasPy2OnlySources(hasPy2OnlySources)
- .setHasPy3OnlySources(hasPy3OnlySources)
- .buildAndAddToTarget(builder);
+ builder.addNativeDeclaredProvider(
+ PyInfo.builder()
+ .setTransitiveSources(transitivePythonSources)
+ .setUsesSharedLibraries(usesSharedLibraries)
+ .setImports(imports)
+ .setHasPy2OnlySources(hasPy2OnlySources)
+ .setHasPy3OnlySources(hasPy3OnlySources)
+ .build());
}
public void addCommonTransitiveInfoProviders(
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
deleted file mode 100644
index 7325ed2..0000000
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyProviderUtils.java
+++ /dev/null
@@ -1,233 +0,0 @@
-// 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.RuleConfiguredTargetBuilder;
-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.util.FileType;
-import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.Starlark;
-
-/**
- * Static helper class for creating and accessing Python provider information.
- *
- * <p>This class exposes a unified view over both the legacy and modern Python providers.
- */
-// TODO(b/153363654): Delete this class, go directly through PyInfo instead.
-public class PyProviderUtils {
-
- // Disable construction.
- private PyProviderUtils() {}
-
- /** Returns whether a given target has the {@code PyInfo} provider. */
- public static boolean hasModernProvider(TransitiveInfoCollection target) {
- return target.get(PyInfo.PROVIDER) != null;
- }
-
- /**
- * Returns the {@code PyInfo} provider from the given target info, or null if the provider is not
- * present.
- */
- public static PyInfo getModernProvider(TransitiveInfoCollection target) {
- return target.get(PyInfo.PROVIDER);
- }
-
- /** Returns whether a given target has the legacy "py" provider. */
- public static boolean hasLegacyProvider(TransitiveInfoCollection target) {
- return target.get(PyStructUtils.PROVIDER_NAME) != null;
- }
-
- /**
- * Returns the struct representing the legacy "py" provider, from the given target info.
- *
- * @throws EvalException if the provider does not exist or has the wrong type.
- */
- public static StructImpl getLegacyProvider(TransitiveInfoCollection target) throws EvalException {
- Object info = target.get(PyStructUtils.PROVIDER_NAME);
- if (info instanceof StructImpl) {
- return (StructImpl) info;
- }
- if (info == null) {
- throw Starlark.errorf("Target does not have '%s' provider", PyStructUtils.PROVIDER_NAME);
- }
- throw Starlark.errorf(
- "'%s' provider should be a struct (got %s)",
- PyStructUtils.PROVIDER_NAME, Starlark.type(info));
- }
-
- /**
- * Returns the transitive sources of a given target.
- *
- * <p>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<Artifact> getTransitiveSources(TransitiveInfoCollection target)
- throws EvalException {
- if (hasModernProvider(target)) {
- return getModernProvider(target).getTransitiveSourcesSet();
- } else if (hasLegacyProvider(target)) {
- return PyStructUtils.getTransitiveSources(getLegacyProvider(target));
- } else {
- NestedSet<Artifact> files = target.getProvider(FileProvider.class).getFilesToBuild();
- return NestedSetBuilder.<Artifact>compileOrder()
- .addAll(FileType.filter(files.toList(), PyRuleClasses.PYTHON_SOURCE))
- .build();
- }
- }
-
- /**
- * Returns whether a target uses shared libraries.
- *
- * <p>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 (hasModernProvider(target)) {
- return getModernProvider(target).getUsesSharedLibraries();
- } else if (hasLegacyProvider(target)) {
- return PyStructUtils.getUsesSharedLibraries(getLegacyProvider(target));
- } else {
- NestedSet<Artifact> files = target.getProvider(FileProvider.class).getFilesToBuild();
- return FileType.contains(files.toList(), CppFileTypes.SHARED_LIBRARY);
- }
- }
-
- /**
- * Returns the transitive import paths of a target.
- *
- * <p>If the target has a py provider, the value from that provider is used. Otherwise, we default
- * to an empty set.
- *
- * @throws EvalException if the legacy struct provider is present but malformed
- */
- public static NestedSet<String> getImports(TransitiveInfoCollection target) throws EvalException {
- if (hasModernProvider(target)) {
- return getModernProvider(target).getImportsSet();
- } else if (hasLegacyProvider(target)) {
- return PyStructUtils.getImports(getLegacyProvider(target));
- } else {
- return NestedSetBuilder.emptySet(Order.COMPILE_ORDER);
- }
- }
-
- /**
- * Returns whether the target has transitive sources requiring Python 2.
- *
- * <p>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 (hasModernProvider(target)) {
- return getModernProvider(target).getHasPy2OnlySources();
- } else if (hasLegacyProvider(target)) {
- return PyStructUtils.getHasPy2OnlySources(getLegacyProvider(target));
- } else {
- return false;
- }
- }
-
- /**
- * Returns whether the target has transitive sources requiring Python 3.
- *
- * <p>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 (hasModernProvider(target)) {
- return getModernProvider(target).getHasPy3OnlySources();
- } else if (hasLegacyProvider(target)) {
- return PyStructUtils.getHasPy3OnlySources(getLegacyProvider(target));
- } else {
- return false;
- }
- }
-
- /**
- * Returns a builder to construct the legacy and/or modern Python providers and add them to a
- * configured target.
- *
- * <p>If {@code createLegacy} is false, only the modern {@code PyInfo} provider is produced.
- * Otherwise both {@code PyInfo} and the "py" provider are produced.
- */
- public static Builder builder(boolean createLegacy) {
- return new Builder(createLegacy);
- }
-
- /** A builder to add both the legacy and modern providers to a configured target. */
- public static class Builder {
- private final PyInfo.Builder modernBuilder = PyInfo.builder();
- private final PyStructUtils.Builder legacyBuilder = PyStructUtils.builder();
- private final boolean createLegacy;
-
- // Use the static builder() method instead.
- private Builder(boolean createLegacy) {
- this.createLegacy = createLegacy;
- }
-
- public Builder setTransitiveSources(NestedSet<Artifact> transitiveSources) {
- modernBuilder.setTransitiveSources(transitiveSources);
- legacyBuilder.setTransitiveSources(transitiveSources);
- return this;
- }
-
- public Builder setUsesSharedLibraries(boolean usesSharedLibraries) {
- modernBuilder.setUsesSharedLibraries(usesSharedLibraries);
- legacyBuilder.setUsesSharedLibraries(usesSharedLibraries);
- return this;
- }
-
- public Builder setImports(NestedSet<String> imports) {
- modernBuilder.setImports(imports);
- legacyBuilder.setImports(imports);
- return this;
- }
-
- public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) {
- modernBuilder.setHasPy2OnlySources(hasPy2OnlySources);
- legacyBuilder.setHasPy2OnlySources(hasPy2OnlySources);
- return this;
- }
-
- public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
- modernBuilder.setHasPy3OnlySources(hasPy3OnlySources);
- legacyBuilder.setHasPy3OnlySources(hasPy3OnlySources);
- return this;
- }
-
- public RuleConfiguredTargetBuilder buildAndAddToTarget(
- RuleConfiguredTargetBuilder targetBuilder) {
- targetBuilder.addNativeDeclaredProvider(modernBuilder.build());
- if (createLegacy) {
- targetBuilder.addStarlarkTransitiveInfo(PyStructUtils.PROVIDER_NAME, legacyBuilder.build());
- }
- return targetBuilder;
- }
- }
-}
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
deleted file mode 100644
index ec01775..0000000
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyStructUtils.java
+++ /dev/null
@@ -1,231 +0,0 @@
-// 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.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.collect.nestedset.Depset;
-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.packages.StructProvider;
-import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.Starlark;
-
-/** Static helper class for creating and accessing instances of the legacy "py" struct provider. */
-// TODO(b/153363654): Remove this file.
-public class PyStructUtils {
-
- // Disable construction.
- private PyStructUtils() {}
-
- /** 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 postorder-compatible depset of transitive sources (i.e., .py files in
- * {@code srcs} and in {@code srcs} of transitive {@code 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).
- */
- // TODO(brandjon): Make this a pre-order depset, since higher-level targets should get precedence
- // on PYTHONPATH. Add assertions on its order compatibility.
- public static final String IMPORTS = "imports";
-
- /**
- * Name of field holding a boolean indicating whether there are any transitive sources that
- * require a Python 2 runtime.
- */
- public static final String HAS_PY2_ONLY_SOURCES = "has_py2_only_sources";
-
- /**
- * Name of field holding a boolean indicating whether there are any transitive sources that
- * require a Python 3 runtime.
- */
- public static final String HAS_PY3_ONLY_SOURCES = "has_py3_only_sources";
-
- private static final ImmutableMap<String, Object> DEFAULTS;
-
- static {
- ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
- // TRANSITIVE_SOURCES is mandatory
- builder.put(USES_SHARED_LIBRARIES, false);
- builder.put(
- IMPORTS,
- Depset.of(
- Depset.ElementType.STRING, NestedSetBuilder.<String>emptySet(Order.COMPILE_ORDER)));
- builder.put(HAS_PY2_ONLY_SOURCES, false);
- builder.put(HAS_PY3_ONLY_SOURCES, false);
- DEFAULTS = builder.buildOrThrow();
- }
-
- private static Object getValue(StructImpl info, String fieldName) throws EvalException {
- Object fieldValue = info.getValue(fieldName);
- if (fieldValue == null) {
- fieldValue = DEFAULTS.get(fieldName);
- if (fieldValue == null) {
- throw Starlark.errorf("'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<Artifact> getTransitiveSources(StructImpl info) throws EvalException {
- Object x = getValue(info, TRANSITIVE_SOURCES);
- if (x == null) {
- throw Starlark.errorf(
- "'%s' provider's '%s' field is missing, want depset", PROVIDER_NAME, TRANSITIVE_SOURCES);
- }
- NestedSet<Artifact> set = Depset.cast(x, Artifact.class, TRANSITIVE_SOURCES);
- if (!set.getOrder().isCompatible(Order.COMPILE_ORDER)) {
- throw Starlark.errorf(
- "Incompatible depset order for '%s': expected 'default' or 'postorder', but got '%s'",
- TRANSITIVE_SOURCES, set.getOrder().getStarlarkName());
- }
- return set;
- }
-
- /**
- * Casts and returns the uses-shared-libraries field (or its default value).
- *
- * @throws EvalException if the field exists and is not a boolean
- */
- public static boolean getUsesSharedLibraries(StructImpl info) throws EvalException {
- Object v = getValue(info, USES_SHARED_LIBRARIES);
- if (v instanceof Boolean) {
- return (Boolean) v;
- }
- throw Starlark.errorf(
- "'%s' provider's '%s' field was %s, want bool",
- PROVIDER_NAME, USES_SHARED_LIBRARIES, Starlark.type(v));
- }
-
- /**
- * Casts and returns the imports field (or its default value).
- *
- * @throws EvalException if the field exists and is not a depset of strings
- */
- public static NestedSet<String> getImports(StructImpl info) throws EvalException {
- Object x = getValue(info, IMPORTS);
- if (x == null) {
- throw Starlark.errorf(
- "'%s' provider's '%s' field is missing, want depset", PROVIDER_NAME, IMPORTS);
- }
- return Depset.cast(x, String.class, IMPORTS);
- }
-
- /**
- * Casts and returns the py2-only-sources field (or its default value).
- *
- * @throws EvalException if the field exists and is not a boolean
- */
- public static boolean getHasPy2OnlySources(StructImpl info) throws EvalException {
- Object v = getValue(info, HAS_PY2_ONLY_SOURCES);
- if (v instanceof Boolean) {
- return (Boolean) v;
- }
- throw Starlark.errorf(
- "'%s' provider's '%s' field was %s, want bool",
- PROVIDER_NAME, HAS_PY2_ONLY_SOURCES, Starlark.type(v));
- }
-
- /**
- * Casts and returns the py3-only-sources field (or its default value).
- *
- * @throws EvalException if the field exists and is not a boolean
- */
- public static boolean getHasPy3OnlySources(StructImpl info) throws EvalException {
- Object v = getValue(info, HAS_PY3_ONLY_SOURCES);
- if (v instanceof Boolean) {
- return (Boolean) v;
- }
- throw Starlark.errorf(
- "'%s' provider's '%s' field was %s, want bool",
- PROVIDER_NAME, HAS_PY3_ONLY_SOURCES, Starlark.type(v));
- }
-
- public static Builder builder() {
- return new Builder();
- }
-
- /** Builder for a legacy py provider struct. */
- public static class Builder {
- Depset transitiveSources = null;
- Boolean usesSharedLibraries = null;
- Depset imports = null;
- Boolean hasPy2OnlySources = null;
- Boolean hasPy3OnlySources = null;
-
- // Use the static builder() method instead.
- private Builder() {}
-
- public Builder setTransitiveSources(NestedSet<Artifact> transitiveSources) {
- this.transitiveSources = Depset.of(Artifact.TYPE, transitiveSources);
- return this;
- }
-
- public Builder setUsesSharedLibraries(boolean usesSharedLibraries) {
- this.usesSharedLibraries = usesSharedLibraries;
- return this;
- }
-
- public Builder setImports(NestedSet<String> imports) {
- this.imports = Depset.of(Depset.ElementType.STRING, imports);
- return this;
- }
-
- public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) {
- this.hasPy2OnlySources = hasPy2OnlySources;
- return this;
- }
-
- public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
- this.hasPy3OnlySources = hasPy3OnlySources;
- return this;
- }
-
- private static void put(
- ImmutableMap.Builder<String, Object> fields, String fieldName, Object value) {
- fields.put(fieldName, value != null ? value : DEFAULTS.get(fieldName));
- }
-
- public StructImpl build() {
- ImmutableMap.Builder<String, Object> fields = ImmutableMap.builder();
- Preconditions.checkNotNull(transitiveSources, "setTransitiveSources is required");
- put(fields, TRANSITIVE_SOURCES, transitiveSources);
- put(fields, USES_SHARED_LIBRARIES, usesSharedLibraries);
- put(fields, IMPORTS, imports);
- put(fields, HAS_PY2_ONLY_SOURCES, hasPy2OnlySources);
- put(fields, HAS_PY3_ONLY_SOURCES, hasPy3OnlySources);
- return StructProvider.STRUCT.create(fields.buildOrThrow(), "No such attribute '%s'");
- }
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
index 7cfaea2..170580c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
@@ -46,9 +46,6 @@
// TODO(brandjon): Remove this once migration to PY3-as-default is complete.
private final boolean py2OutputsAreSuffixed;
- // TODO(brandjon): Remove this once migration to the new provider is complete (#7010).
- private final boolean disallowLegacyPyProvider;
-
// TODO(brandjon): Remove this once migration to Python toolchains is complete.
private final boolean useToolchains;
@@ -63,7 +60,6 @@
this.buildPythonZip = pythonOptions.buildPythonZip;
this.buildTransitiveRunfilesTrees = pythonOptions.buildTransitiveRunfilesTrees;
this.py2OutputsAreSuffixed = pythonOptions.incompatiblePy2OutputsAreSuffixed;
- this.disallowLegacyPyProvider = pythonOptions.incompatibleDisallowLegacyPyProvider;
this.useToolchains = pythonOptions.incompatibleUsePythonToolchains;
this.defaultToExplicitInitPy = pythonOptions.incompatibleDefaultToExplicitInitPy;
}
@@ -139,16 +135,6 @@
}
/**
- * Returns true if Python rules should omit the legacy "py" provider and fail-fast when given this
- * provider from their {@code deps}.
- *
- * <p>Any rules that pass this provider should be updated to pass {@code PyInfo} instead.
- */
- public boolean disallowLegacyPyProvider() {
- return disallowLegacyPyProvider;
- }
-
- /**
* Returns true if executable Python rules should obtain their runtime from the Python toolchain
* rather than legacy flags.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
index 14aa2fe..6795f35 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
@@ -199,16 +199,14 @@
private static final OptionDefinition HOST_FORCE_PYTHON_DEFINITION =
OptionsParser.getOptionDefinitionByName(PythonOptions.class, "host_force_python");
+ // TODO(b/230490091): Delete this flag (see also bazelbuild issue #7741)
@Option(
name = "incompatible_disallow_legacy_py_provider",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
- help =
- "If set to true, native Python rules will neither produce nor consume the legacy \"py\" "
- + "provider. Use PyInfo instead. Under this flag, passing the legacy provider to a "
- + "Python target will be an error.")
+ help = "No-op, will be removed soon.")
public boolean incompatibleDisallowLegacyPyProvider;
// TODO(b/153369373): Delete this flag.
@@ -326,7 +324,6 @@
hostPythonOptions.incompatiblePy3IsDefault = incompatiblePy3IsDefault;
hostPythonOptions.incompatiblePy2OutputsAreSuffixed = incompatiblePy2OutputsAreSuffixed;
hostPythonOptions.buildPythonZip = buildPythonZip;
- hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
hostPythonOptions.incompatibleUsePythonToolchains = incompatibleUsePythonToolchains;
// Save host options in case of a further exec->host transition.
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 9edc440..8e8b97b 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
@@ -18,11 +18,9 @@
":PyBinaryConfiguredTargetTest",
":PyInfoTest",
":PyLibraryConfiguredTargetTest",
- ":PyProviderUtilsTest",
":PyRuntimeConfiguredTargetTest",
":PyRuntimeInfoTest",
":PyStarlarkTransitionsTest",
- ":PyStructUtilsTest",
":PyTestConfiguredTargetTest",
":PythonConfigurationTest",
":PythonSrcsVersionAspectTest",
@@ -185,37 +183,6 @@
)
java_test(
- name = "PyProviderUtilsTest",
- srcs = ["PyProviderUtilsTest.java"],
- deps = [
- "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
- "//src/main/java/com/google/devtools/build/lib/rules/python",
- "//src/main/java/net/starlark/java/eval",
- "//src/test/java/com/google/devtools/build/lib/analysis/util",
- "//third_party:junit4",
- "//third_party:truth",
- ],
-)
-
-java_test(
- name = "PyStructUtilsTest",
- srcs = ["PyStructUtilsTest.java"],
- deps = [
- "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
- "//src/main/java/com/google/devtools/build/lib/packages",
- "//src/main/java/com/google/devtools/build/lib/rules/python",
- "//src/main/java/com/google/devtools/build/lib/vfs",
- "//src/main/java/net/starlark/java/eval",
- "//src/test/java/com/google/devtools/build/lib/actions/util",
- "//src/test/java/com/google/devtools/build/lib/testutil",
- "//third_party:guava",
- "//third_party:junit4",
- "//third_party:truth",
- ],
-)
-
-java_test(
name = "PyInfoTest",
srcs = ["PyInfoTest.java"],
deps = [
@@ -267,7 +234,6 @@
deps = [
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
- "//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules/python",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
index 610926a..3f7452b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
@@ -79,8 +79,7 @@
}
@Test
- public void producesBothModernAndLegacyProviders_WithoutIncompatibleFlag() throws Exception {
- useConfiguration("--incompatible_disallow_legacy_py_provider=false");
+ public void producesProvider() throws Exception {
scratch.file(
"pkg/BUILD", //
ruleName + "(",
@@ -88,77 +87,10 @@
" srcs = ['foo.py'])");
ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
assertThat(target.get(PyInfo.PROVIDER)).isNotNull();
- assertThat(target.get(PyStructUtils.PROVIDER_NAME)).isNotNull();
}
@Test
- public void producesOnlyModernProvider_WithIncompatibleFlag() throws Exception {
- useConfiguration("--incompatible_disallow_legacy_py_provider=true");
- scratch.file(
- "pkg/BUILD", //
- ruleName + "(",
- " name = 'foo',",
- " srcs = ['foo.py'])");
- ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
- assertThat(target.get(PyInfo.PROVIDER)).isNotNull();
- assertThat(target.get(PyStructUtils.PROVIDER_NAME)).isNull();
- }
-
- @Test
- public void consumesLegacyProvider_WithoutIncompatibleFlag() throws Exception {
- useConfiguration("--incompatible_disallow_legacy_py_provider=false");
- scratch.file(
- "pkg/rules.bzl",
- "def _myrule_impl(ctx):",
- " return struct(py=struct(transitive_sources=depset([])))",
- "myrule = rule(",
- " implementation = _myrule_impl,",
- ")");
- scratch.file(
- "pkg/BUILD",
- "load(':rules.bzl', 'myrule')",
- "myrule(",
- " name = 'dep',",
- ")",
- ruleName + "(",
- " name = 'foo',",
- " srcs = ['foo.py'],",
- " deps = [':dep'],",
- ")");
- ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
- assertThat(target).isNotNull();
- assertNoEvents();
- }
-
- @Test
- public void rejectsLegacyProvider_WithIncompatibleFlag() throws Exception {
- useConfiguration("--incompatible_disallow_legacy_py_provider=true");
- scratch.file(
- "pkg/rules.bzl",
- "def _myrule_impl(ctx):",
- " return struct(py=struct(transitive_sources=depset([])))",
- "myrule = rule(",
- " implementation = _myrule_impl,",
- ")");
- checkError(
- "pkg",
- "foo",
- // error:
- "In dep '//pkg:dep': The legacy 'py' provider is disallowed.",
- // build file:
- "load(':rules.bzl', 'myrule')",
- "myrule(",
- " name = 'dep',",
- ")",
- ruleName + "(",
- " name = 'foo',",
- " srcs = ['foo.py'],",
- " deps = [':dep'],",
- ")");
- }
-
- @Test
- public void consumesModernProvider() throws Exception {
+ public void consumesProvider() throws Exception {
scratch.file(
"pkg/rules.bzl",
"def _myrule_impl(ctx):",
@@ -207,4 +139,17 @@
" deps = [':dep'],",
")");
}
+
+ @Test
+ public void dataSetsUsesSharedLibrary() throws Exception {
+ scratch.file(
+ "pkg/BUILD",
+ ruleName + "(",
+ " name = 'foo',",
+ " srcs = ['foo.py'],",
+ " data = ['lib.so']",
+ ")");
+ ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
+ assertThat(target.get(PyInfo.PROVIDER).getUsesSharedLibraries()).isTrue();
+ }
}
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
deleted file mode 100644
index b005a7a..0000000
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyProviderUtilsTest.java
+++ /dev/null
@@ -1,312 +0,0 @@
-// 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 org.junit.Assert.assertThrows;
-
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import net.starlark.java.eval.EvalException;
-import org.junit.Test;
-import org.junit.function.ThrowingRunnable;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for {@link PyProviderUtils}. */
-@RunWith(JUnit4.class)
-public class PyProviderUtilsTest extends BuildViewTestCase {
-
- private static String join(String... lines) {
- return String.join("\n", lines);
- }
-
- 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.
- *
- * <p>The body is formed by joining the input strings with newlines, then inserting 4 spaces of
- * indentation before the first line and each newline. This allows a single string argument to
- * contain multiple pre-joined lines and still be indented correctly.
- */
- private void declareTargetWithImplementation(String... input) throws Exception {
- String indentedBody;
- if (input.length == 0) {
- indentedBody = " pass";
- } else {
- indentedBody = " " + String.join("\n", input).replace("\n", "\n ");
- }
- scratch.file(
- "pkg/rules.bzl",
- "def _myrule_impl(ctx):",
- indentedBody,
- "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 getAndHasModernProvider_Present() throws Exception {
- declareTargetWithImplementation( //
- "return [PyInfo(transitive_sources=depset([]))]");
- assertThat(PyProviderUtils.hasModernProvider(getTarget())).isTrue();
- assertThat(PyProviderUtils.getModernProvider(getTarget())).isNotNull();
- }
-
- @Test
- public void getAndHasModernProvider_Absent() throws Exception {
- declareTargetWithImplementation( //
- "return []");
- assertThat(PyProviderUtils.hasModernProvider(getTarget())).isFalse();
- assertThat(PyProviderUtils.getModernProvider(getTarget())).isNull();
- }
-
- @Test
- public void getAndHasLegacyProvider_Present() throws Exception {
- declareTargetWithImplementation( //
- "return struct(py=struct())"); // Accessor doesn't actually check fields
- assertThat(PyProviderUtils.hasLegacyProvider(getTarget())).isTrue();
- assertThat(PyProviderUtils.getLegacyProvider(getTarget())).isNotNull();
- }
-
- @Test
- public void getAndHasLegacyProvider_Absent() throws Exception {
- declareTargetWithImplementation( //
- "return []");
- assertThat(PyProviderUtils.hasLegacyProvider(getTarget())).isFalse();
- assertThrowsEvalExceptionContaining(
- () -> PyProviderUtils.getLegacyProvider(getTarget()), "Target does not have 'py' provider");
- }
-
- @Test
- public void getLegacyProvider_NotAStruct() throws Exception {
- declareTargetWithImplementation( //
- "return struct(py=123)");
- assertThrowsEvalExceptionContaining(
- () -> PyProviderUtils.getLegacyProvider(getTarget()), "'py' provider should be a struct");
- }
-
- private static final String TRANSITIVE_SOURCES_SETUP_CODE =
- join(
- "afile = ctx.actions.declare_file('a.py')",
- "ctx.actions.write(output=afile, content='a')",
- "bfile = ctx.actions.declare_file('b.py')",
- "ctx.actions.write(output=bfile, content='b')",
- "modern_info = PyInfo(transitive_sources = depset(direct=[afile]))",
- "legacy_info = struct(transitive_sources = depset(direct=[bfile]))");
-
- @Test
- public void getTransitiveSources_ModernProvider() throws Exception {
- declareTargetWithImplementation( //
- TRANSITIVE_SOURCES_SETUP_CODE, //
- "return [modern_info]");
- assertThat(PyProviderUtils.getTransitiveSources(getTarget()).toList())
- .containsExactly(getBinArtifact("a.py", getTarget()));
- }
-
- @Test
- public void getTransitiveSources_LegacyProvider() throws Exception {
- declareTargetWithImplementation( //
- TRANSITIVE_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info)");
- assertThat(PyProviderUtils.getTransitiveSources(getTarget()).toList())
- .containsExactly(getBinArtifact("b.py", getTarget()));
- }
-
- @Test
- public void getTransitiveSources_BothProviders() throws Exception {
- declareTargetWithImplementation( //
- TRANSITIVE_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info, providers=[modern_info])");
- assertThat(PyProviderUtils.getTransitiveSources(getTarget()).toList())
- .containsExactly(getBinArtifact("a.py", getTarget()));
- }
-
- @Test
- public void getTransitiveSources_NoProvider() throws Exception {
- declareTargetWithImplementation( //
- TRANSITIVE_SOURCES_SETUP_CODE, //
- "return [DefaultInfo(files=depset(direct=[afile]))]");
- assertThat(PyProviderUtils.getTransitiveSources(getTarget()).toList())
- .containsExactly(getBinArtifact("a.py", getTarget()));
- }
-
- private static final String USES_SHARED_LIBRARIES_SETUP_CODE =
- join(
- "modern_info = PyInfo(transitive_sources = depset([]), uses_shared_libraries=False)",
- "legacy_info = struct(transitive_sources = depset([]), uses_shared_libraries=True)");
-
- @Test
- public void getUsesSharedLibraries_ModernProvider() throws Exception {
- declareTargetWithImplementation( //
- USES_SHARED_LIBRARIES_SETUP_CODE, //
- "return [modern_info]");
- assertThat(PyProviderUtils.getUsesSharedLibraries(getTarget())).isFalse();
- }
-
- @Test
- public void getUsesSharedLibraries_LegacyProvider() throws Exception {
- declareTargetWithImplementation( //
- USES_SHARED_LIBRARIES_SETUP_CODE, //
- "return struct(py=legacy_info)");
- assertThat(PyProviderUtils.getUsesSharedLibraries(getTarget())).isTrue();
- }
-
- @Test
- public void getUsesSharedLibraries_BothProviders() throws Exception {
- declareTargetWithImplementation( //
- USES_SHARED_LIBRARIES_SETUP_CODE, //
- "return struct(py=legacy_info, providers=[modern_info])");
- assertThat(PyProviderUtils.getUsesSharedLibraries(getTarget())).isFalse();
- }
-
- @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();
- }
-
- private static final String IMPORTS_SETUP_CODE =
- join(
- "modern_info = PyInfo(transitive_sources = depset([]), imports = depset(direct=['abc']))",
- "legacy_info = struct(",
- " transitive_sources = depset([]),",
- " imports = depset(direct=['def']))");
-
- @Test
- public void getImports_ModernProvider() throws Exception {
- declareTargetWithImplementation( //
- IMPORTS_SETUP_CODE, //
- "return [modern_info]");
- assertThat(PyProviderUtils.getImports(getTarget()).toList()).containsExactly("abc");
- }
-
- @Test
- public void getImports_LegacyProvider() throws Exception {
- declareTargetWithImplementation( //
- IMPORTS_SETUP_CODE, //
- "return struct(py=legacy_info)");
- assertThat(PyProviderUtils.getImports(getTarget()).toList()).containsExactly("def");
- }
-
- @Test
- public void getImports_BothProviders() throws Exception {
- declareTargetWithImplementation( //
- IMPORTS_SETUP_CODE, //
- "return struct(py=legacy_info, providers=[modern_info])");
- assertThat(PyProviderUtils.getImports(getTarget()).toList()).containsExactly("abc");
- }
-
- @Test
- public void getImports_NoProvider() throws Exception {
- declareTargetWithImplementation( //
- IMPORTS_SETUP_CODE, //
- "return []");
- assertThat(PyProviderUtils.getImports(getTarget()).toList()).isEmpty();
- }
-
- private static final String HAS_PY2_ONLY_SOURCES_SETUP_CODE =
- join(
- "modern_info = PyInfo(transitive_sources = depset([]), has_py2_only_sources = False)",
- "legacy_info = struct(transitive_sources = depset([]), has_py2_only_sources = True)");
-
- @Test
- public void getHasPy2OnlySources_ModernProvider() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY2_ONLY_SOURCES_SETUP_CODE, //
- "return [modern_info]");
- assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isFalse();
- }
-
- @Test
- public void getHasPy2OnlySources_LegacyProvider() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY2_ONLY_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info)");
- assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isTrue();
- }
-
- @Test
- public void getHasPy2OnlySources_BothProviders() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY2_ONLY_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info, providers=[modern_info])");
- assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isFalse();
- }
-
- @Test
- public void getHasPy2OnlySources_NoProvider() throws Exception {
- declareTargetWithImplementation( //
- "return []");
- assertThat(PyProviderUtils.getHasPy2OnlySources(getTarget())).isFalse();
- }
-
- private static final String HAS_PY3_ONLY_SOURCES_SETUP_CODE =
- join(
- "modern_info = PyInfo(transitive_sources = depset([]), has_py3_only_sources = False)",
- "legacy_info = struct(transitive_sources = depset([]), has_py3_only_sources = True)");
-
- @Test
- public void getHasPy3OnlySources_ModernProvider() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY3_ONLY_SOURCES_SETUP_CODE, //
- "return [modern_info]");
- assertThat(PyProviderUtils.getHasPy3OnlySources(getTarget())).isFalse();
- }
-
- @Test
- public void getHasPy3OnlySources_LegacyProvider() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY3_ONLY_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info)");
- assertThat(PyProviderUtils.getHasPy3OnlySources(getTarget())).isTrue();
- }
-
- @Test
- public void getHasPy3OnlySources_BothProviders() throws Exception {
- declareTargetWithImplementation( //
- HAS_PY3_ONLY_SOURCES_SETUP_CODE, //
- "return struct(py=legacy_info, providers=[modern_info])");
- assertThat(PyProviderUtils.getHasPy3OnlySources(getTarget())).isFalse();
- }
-
- @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
deleted file mode 100644
index 9a3a906..0000000
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyStructUtilsTest.java
+++ /dev/null
@@ -1,263 +0,0 @@
-// 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 org.junit.Assert.assertThrows;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactRoot;
-import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
-import com.google.devtools.build.lib.collect.nestedset.Depset;
-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.StarlarkInfo;
-import com.google.devtools.build.lib.packages.StructImpl;
-import com.google.devtools.build.lib.packages.StructProvider;
-import com.google.devtools.build.lib.testutil.FoundationTestCase;
-import com.google.devtools.build.lib.vfs.Root;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.StarlarkInt;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.function.ThrowingRunnable;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for {@link PyStructUtils}. */
-@RunWith(JUnit4.class)
-public class PyStructUtilsTest extends FoundationTestCase {
-
- private Artifact dummyArtifact;
-
- @Before
- public void setUp() {
- this.dummyArtifact =
- ActionsTestUtil.createArtifact(
- ArtifactRoot.asSourceRoot(Root.fromPath(rootDirectory)), "dummy");
- }
-
- /**
- * Constructs a py provider struct with the given field values and with default values for any
- * field not specified.
- *
- * <p>The struct is constructed directly, rather than using {@link PyStructUtils.Builder}, so that
- * the resulting instance is suitable for asserting on {@code PyStructUtils}'s operations over
- * structs with known contents. {@code overrides} is applied directly without validating the
- * fields' names or types.
- */
- private StructImpl makeStruct(Map<String, Object> overrides) {
- Map<String, Object> fields = new LinkedHashMap<>();
- fields.put(
- PyStructUtils.TRANSITIVE_SOURCES,
- Depset.of(Artifact.TYPE, NestedSetBuilder.emptySet(Order.COMPILE_ORDER)));
- fields.put(PyStructUtils.USES_SHARED_LIBRARIES, false);
- fields.put(
- PyStructUtils.IMPORTS,
- Depset.of(Depset.ElementType.STRING, NestedSetBuilder.emptySet(Order.COMPILE_ORDER)));
- fields.put(PyStructUtils.HAS_PY2_ONLY_SOURCES, false);
- fields.put(PyStructUtils.HAS_PY3_ONLY_SOURCES, false);
- fields.putAll(overrides);
- return StructProvider.STRUCT.create(fields, "No such attribute '%s'");
- }
-
- 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 was int, want %s", fieldName, expectedType));
- }
-
- /** We need this because {@code NestedSet}s don't have value equality. */
- private static void assertHasOrderAndContainsExactly(
- NestedSet<?> set, Order order, Object... values) {
- assertThat(set.getOrder()).isEqualTo(order);
- assertThat(set.toList()).containsExactly(values);
- }
-
- @Test
- public void getTransitiveSources_Good() throws Exception {
- NestedSet<Artifact> sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact);
- StructImpl info =
- makeStruct(
- ImmutableMap.of(PyStructUtils.TRANSITIVE_SOURCES, Depset.of(Artifact.TYPE, sources)));
- assertThat(PyStructUtils.getTransitiveSources(info)).isSameInstanceAs(sources);
- }
-
- private static final StructImpl emptyInfo =
- StarlarkInfo.create(StructProvider.STRUCT, ImmutableMap.of(), /* loc= */ null);
-
- @Test
- public void getTransitiveSources_Missing() {
- assertHasMissingFieldMessage(
- () -> PyStructUtils.getTransitiveSources(emptyInfo), "transitive_sources");
- }
-
- @Test
- public void getTransitiveSources_WrongType() {
- StructImpl info =
- makeStruct(ImmutableMap.of(PyStructUtils.TRANSITIVE_SOURCES, StarlarkInt.of(123)));
- assertThrowsEvalExceptionContaining(
- () -> PyStructUtils.getTransitiveSources(info),
- "for transitive_sources, got int, want a depset of File");
- }
-
- @Test
- public void getTransitiveSources_OrderMismatch() throws Exception {
- NestedSet<Artifact> sources = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
- StructImpl info =
- makeStruct(
- ImmutableMap.of(PyStructUtils.TRANSITIVE_SOURCES, Depset.of(Artifact.TYPE, sources)));
- assertThrowsEvalExceptionContaining(
- () -> PyStructUtils.getTransitiveSources(info),
- "Incompatible depset order for 'transitive_sources': expected 'default' or 'postorder', "
- + "but got 'preorder'");
- }
-
- @Test
- public void getUsesSharedLibraries_Good() throws Exception {
- StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.USES_SHARED_LIBRARIES, true));
- assertThat(PyStructUtils.getUsesSharedLibraries(info)).isTrue();
- }
-
- @Test
- public void getUsesSharedLibraries_Missing() throws Exception {
- assertThat(PyStructUtils.getUsesSharedLibraries(emptyInfo)).isFalse();
- }
-
- @Test
- public void getUsesSharedLibraries_WrongType() {
- StructImpl info =
- makeStruct(ImmutableMap.of(PyStructUtils.USES_SHARED_LIBRARIES, StarlarkInt.of(123)));
- assertHasWrongTypeMessage(
- () -> PyStructUtils.getUsesSharedLibraries(info), "uses_shared_libraries", "bool");
- }
-
- @Test
- public void getImports_Good() throws Exception {
- NestedSet<String> imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc");
- StructImpl info =
- makeStruct(
- ImmutableMap.of(PyStructUtils.IMPORTS, Depset.of(Depset.ElementType.STRING, imports)));
- assertThat(PyStructUtils.getImports(info)).isSameInstanceAs(imports);
- }
-
- @Test
- public void getImports_Missing() throws Exception {
- assertHasOrderAndContainsExactly(PyStructUtils.getImports(emptyInfo), Order.COMPILE_ORDER);
- }
-
- @Test
- public void getImports_WrongType() {
- StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.IMPORTS, StarlarkInt.of(123)));
- assertThrowsEvalExceptionContaining(
- () -> PyStructUtils.getImports(info), "for imports, got int, want a depset of string");
- }
-
- @Test
- public void getHasPy2OnlySources_Good() throws Exception {
- StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY2_ONLY_SOURCES, true));
- assertThat(PyStructUtils.getHasPy2OnlySources(info)).isTrue();
- }
-
- @Test
- public void getHasPy2OnlySources_Missing() throws Exception {
- assertThat(PyStructUtils.getHasPy2OnlySources(emptyInfo)).isFalse();
- }
-
- @Test
- public void getHasPy2OnlySources_WrongType() {
- StructImpl info =
- makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY2_ONLY_SOURCES, StarlarkInt.of(123)));
- assertHasWrongTypeMessage(
- () -> PyStructUtils.getHasPy2OnlySources(info), "has_py2_only_sources", "bool");
- }
-
- @Test
- public void getHasPy3OnlySources_Good() throws Exception {
- StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY3_ONLY_SOURCES, true));
- assertThat(PyStructUtils.getHasPy3OnlySources(info)).isTrue();
- }
-
- @Test
- public void getHasPy3OnlySources_Missing() throws Exception {
- assertThat(PyStructUtils.getHasPy3OnlySources(emptyInfo)).isFalse();
- }
-
- @Test
- public void getHasPy3OnlySources_WrongType() {
- StructImpl info =
- makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY3_ONLY_SOURCES, StarlarkInt.of(123)));
- assertHasWrongTypeMessage(
- () -> PyStructUtils.getHasPy3OnlySources(info), "has_py3_only_sources", "bool");
- }
-
- /** Checks values set by the builder. */
- @Test
- public void builder() throws Exception {
- NestedSet<Artifact> sources = NestedSetBuilder.create(Order.COMPILE_ORDER, dummyArtifact);
- NestedSet<String> imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc");
- StructImpl info =
- PyStructUtils.builder()
- .setTransitiveSources(sources)
- .setUsesSharedLibraries(true)
- .setImports(imports)
- .setHasPy2OnlySources(true)
- .setHasPy3OnlySources(true)
- .build();
- // Assert using struct operations, not PyStructUtils accessors, which aren't necessarily trusted
- // to be correct.
- assertHasOrderAndContainsExactly(
- ((Depset) info.getValue(PyStructUtils.TRANSITIVE_SOURCES)).getSet(Artifact.class),
- Order.COMPILE_ORDER,
- dummyArtifact);
- assertThat((Boolean) info.getValue(PyStructUtils.USES_SHARED_LIBRARIES)).isTrue();
- assertHasOrderAndContainsExactly(
- ((Depset) info.getValue(PyStructUtils.IMPORTS)).getSet(String.class),
- Order.COMPILE_ORDER,
- "abc");
- assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY2_ONLY_SOURCES)).isTrue();
- assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY3_ONLY_SOURCES)).isTrue();
- }
-
- /** 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<Artifact> 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.
- assertThat((Boolean) info.getValue(PyStructUtils.USES_SHARED_LIBRARIES)).isFalse();
- assertHasOrderAndContainsExactly(
- ((Depset) info.getValue(PyStructUtils.IMPORTS)).getSet(String.class), Order.COMPILE_ORDER);
- assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY2_ONLY_SOURCES)).isFalse();
- assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY3_ONLY_SOURCES)).isFalse();
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
index a009109..54b1987 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
@@ -96,13 +96,11 @@
"--incompatible_py3_is_default=true",
"--incompatible_py2_outputs_are_suffixed=true",
"--build_python_zip=true",
- "--incompatible_disallow_legacy_py_provider=true",
"--incompatible_use_python_toolchains=true");
PythonOptions hostOpts = (PythonOptions) opts.getHost();
assertThat(hostOpts.incompatiblePy3IsDefault).isTrue();
assertThat(hostOpts.incompatiblePy2OutputsAreSuffixed).isTrue();
assertThat(hostOpts.buildPythonZip).isEqualTo(TriState.YES);
- assertThat(hostOpts.incompatibleDisallowLegacyPyProvider).isTrue();
assertThat(hostOpts.incompatibleUsePythonToolchains).isTrue();
}
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 97bffef..3ba118e 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
@@ -19,7 +19,6 @@
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.StructImpl;
import com.google.devtools.build.lib.testutil.TestConstants;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -29,16 +28,8 @@
@RunWith(JUnit4.class)
public class PythonStarlarkApiTest extends BuildViewTestCase {
- /**
- * Defines userlib in //pkg:rules.bzl, which acts as a Starlark-defined version of py_library.
- *
- * <p>If {@code legacyProviderAllowed} is true, both the legacy and modern providers are returned.
- * Otherwise only the modern provider is returned. In both cases the modern provider of the
- * dependency is consumed while the native one is ignored.
- */
- private void defineUserlibRule(boolean legacyProviderAllowed) throws Exception {
- String returnExpr =
- legacyProviderAllowed ? "struct(py=legacy_info, providers=[modern_info])" : "[modern_info]";
+ /** Defines userlib in //pkg:rules.bzl, which acts as a Starlark-defined version of py_library. */
+ private void defineUserlibRule() throws Exception {
scratch.file(
"pkg/rules.bzl",
"def _userlib_impl(ctx):",
@@ -59,19 +50,13 @@
" has_py3_only_sources = \\",
" any([py.has_py3_only_sources for py in dep_infos]) or \\",
" ctx.attr.has_py3_only_sources",
- " legacy_info = struct(",
+ " info = PyInfo(",
" transitive_sources = transitive_sources,",
" uses_shared_libraries = uses_shared_libraries,",
" imports = imports,",
" has_py2_only_sources = has_py2_only_sources,",
" has_py3_only_sources = has_py3_only_sources)",
- " modern_info = PyInfo(",
- " transitive_sources = transitive_sources,",
- " uses_shared_libraries = uses_shared_libraries,",
- " imports = imports,",
- " has_py2_only_sources = has_py2_only_sources,",
- " has_py3_only_sources = has_py3_only_sources)",
- " return " + returnExpr,
+ " return [info]",
"",
"userlib = rule(",
" implementation = _userlib_impl,",
@@ -87,19 +72,8 @@
}
@Test
- public void librarySandwich_LegacyProviderAllowed() throws Exception {
- doLibrarySandwichTest(/*legacyProviderAllowed=*/ true);
- }
-
- @Test
- public void librarySandwich_LegacyProviderDisallowed() throws Exception {
- doLibrarySandwichTest(/*legacyProviderAllowed=*/ false);
- }
-
- private void doLibrarySandwichTest(boolean legacyProviderAllowed) throws Exception {
- useConfiguration(
- "--incompatible_disallow_legacy_py_provider=" + (legacyProviderAllowed ? "false" : "true"));
- defineUserlibRule(legacyProviderAllowed);
+ public void librarySandwich() throws Exception {
+ defineUserlibRule();
scratch.file(
"pkg/BUILD",
"load(':rules.bzl', 'userlib')",
@@ -125,31 +99,17 @@
")");
ConfiguredTarget target = getConfiguredTarget("//pkg:upperuserlib");
- if (legacyProviderAllowed) {
- StructImpl legacyInfo = PyProviderUtils.getLegacyProvider(target);
- assertThat(PyStructUtils.getTransitiveSources(legacyInfo).toList())
- .containsExactly(
- getSourceArtifact("pkg/loweruserlib.py"),
- getSourceArtifact("pkg/pylib.py"),
- getSourceArtifact("pkg/upperuserlib.py"));
- assertThat(PyStructUtils.getUsesSharedLibraries(legacyInfo)).isTrue();
- assertThat(PyStructUtils.getImports(legacyInfo).toList())
- .containsExactly("loweruserlib_path", "upperuserlib_path");
- assertThat(PyStructUtils.getHasPy2OnlySources(legacyInfo)).isTrue();
- assertThat(PyStructUtils.getHasPy3OnlySources(legacyInfo)).isTrue();
- }
-
- PyInfo modernInfo = PyProviderUtils.getModernProvider(target);
- assertThat(modernInfo.getTransitiveSources().toList(Artifact.class))
+ PyInfo info = target.get(PyInfo.PROVIDER);
+ assertThat(info.getTransitiveSources().toList(Artifact.class))
.containsExactly(
getSourceArtifact("pkg/loweruserlib.py"),
getSourceArtifact("pkg/pylib.py"),
getSourceArtifact("pkg/upperuserlib.py"));
- assertThat(modernInfo.getUsesSharedLibraries()).isTrue();
- assertThat(modernInfo.getImports().toList(String.class))
+ assertThat(info.getUsesSharedLibraries()).isTrue();
+ assertThat(info.getImports().toList(String.class))
.containsExactly("loweruserlib_path", "upperuserlib_path");
- assertThat(modernInfo.getHasPy2OnlySources()).isTrue();
- assertThat(modernInfo.getHasPy3OnlySources()).isTrue();
+ assertThat(info.getHasPy2OnlySources()).isTrue();
+ assertThat(info.getHasPy3OnlySources()).isTrue();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
index 075fd3f..b8a9b1d 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -52,7 +52,6 @@
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.PyProviderUtils;
import com.google.devtools.build.lib.starlark.util.BazelEvaluationTestCase;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.FileTypeSet;
@@ -500,7 +499,7 @@
TransitiveInfoCollection tic1 = (TransitiveInfoCollection) ((Sequence) result).get(0);
assertThat(JavaInfo.getProvider(JavaSourceJarsProvider.class, tic1)).isNotNull();
// Check an unimplemented provider too
- assertThat(PyProviderUtils.hasLegacyProvider(tic1)).isFalse();
+ assertThat(tic1.get("not_implemented_provider")).isNull();
}
@Test
