Remove support for implicit py2to3, which never worked in Bazel
See https://github.com/bazelbuild/bazel/issues/1393.
This treats py_*.srcs_version="PY2" the same as "PY2ONLY". Targets which are PY3-compatible should use srcs_version="PY2AND3" instead.
Fixes #1393.
RELNOTES: Treat py_*.srcs_version="PY2" the same as "PY2ONLY".
PiperOrigin-RevId: 407116957
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 e002946..859784f 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
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
-import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses.CcToolchainRequiringRule;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault;
@@ -129,13 +128,6 @@
attr("srcs_version", STRING)
.value(PythonVersion.DEFAULT_SRCS_VALUE.toString())
.allowedValues(new AllowedValueSet(PythonVersion.SRCS_STRINGS)))
- // do not depend on lib2to3:2to3 rule, because it creates circular dependencies
- // 2to3 is itself written in Python and depends on many libraries.
- .add(
- attr("$python2to3", LABEL)
- .cfg(HostTransition.createFactory())
- .exec()
- .value(env.getToolsLabel("//tools/python:2to3")))
.setPreferredDependencyPredicate(PyRuleClasses.PYTHON_SOURCE)
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/BUILD b/src/main/java/com/google/devtools/build/lib/rules/python/BUILD
index 9c044c1..501fcc8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/BUILD
@@ -18,7 +18,6 @@
"//src/main/java/com/google/devtools/build/lib:build-request-options",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/analysis:actions/custom_command_line",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
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 e61b4e0..fd2c41f 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
@@ -58,7 +58,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import java.util.Map;
import java.util.UUID;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
@@ -170,13 +169,6 @@
*/
@Nullable private final PyRuntimeInfo runtimeFromToolchain;
- /**
- * Symlink map from root-relative paths to 2to3 converted source artifacts.
- *
- * <p>Null if no 2to3 conversion is required.
- */
- @Nullable private final Map<PathFragment, Artifact> convertedFiles;
-
// Null if requiresMainFile is false, or the main artifact couldn't be determined.
@Nullable private Artifact mainArtifact = null;
@@ -213,7 +205,6 @@
this.hasPy2OnlySources = initHasPy2OnlySources(ruleContext, this.sourcesVersion);
this.hasPy3OnlySources = initHasPy3OnlySources(ruleContext, this.sourcesVersion);
this.runtimeFromToolchain = initRuntimeFromToolchain(ruleContext, this.version);
- this.convertedFiles = makeAndInitConvertedFiles(ruleContext, version, this.sourcesVersion);
validatePythonVersionAttr();
validateLegacyProviderNotUsedIfDisabled();
}
@@ -351,12 +342,11 @@
/**
* Returns true if any of {@code deps} has a py provider with {@code has_py2_only_sources} set, or
- * this target has a {@code srcs_version} of {@code PY2ONLY}.
+ * this target has a {@code srcs_version} of {@code PY2} or {@code PY2ONLY}.
*/
- // TODO(#1393): For Bazel, deprecate 2to3 support and treat PY2 the same as PY2ONLY.
private static boolean initHasPy2OnlySources(
RuleContext ruleContext, PythonVersion sourcesVersion) {
- if (sourcesVersion == PythonVersion.PY2ONLY) {
+ if (sourcesVersion == PythonVersion.PY2 || sourcesVersion == PythonVersion.PY2ONLY) {
return true;
}
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps")) {
@@ -511,27 +501,6 @@
}
/**
- * If 2to3 conversion is to be done, creates the 2to3 actions and returns the map of converted
- * files; otherwise returns null.
- *
- * <p>May also return null and report a rule error if there is a problem creating an output file
- * for 2to3 conversion.
- */
- // TODO(#1393): 2to3 conversion doesn't work in Bazel and the attempt to invoke it for Bazel
- // should be removed / factored away into PythonSemantics.
- @Nullable
- private static Map<PathFragment, Artifact> makeAndInitConvertedFiles(
- RuleContext ruleContext, PythonVersion version, PythonVersion sourcesVersion) {
- if (sourcesVersion == PythonVersion.PY2 && version == PythonVersion.PY3) {
- Iterable<Artifact> artifacts =
- ruleContext.getPrerequisiteArtifacts("srcs").filter(PyRuleClasses.PYTHON_SOURCE).list();
- return PythonUtils.generate2to3Actions(ruleContext, artifacts);
- } else {
- return null;
- }
- }
-
- /**
* Reports an attribute error if {@code python_version} cannot be parsed as {@code PY2}, {@code
* PY3}, or the sentinel value.
*
@@ -752,10 +721,6 @@
return runtimeFromToolchain;
}
- public Map<PathFragment, Artifact> getConvertedFiles() {
- return convertedFiles;
- }
-
public void initBinary(List<Artifact> srcs) {
Preconditions.checkNotNull(version);
@@ -864,9 +829,7 @@
/** Returns a list of the source artifacts */
public List<Artifact> getPythonSources() {
- return convertedFiles != null
- ? ImmutableList.copyOf(convertedFiles.values())
- : directPythonSources;
+ return directPythonSources;
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java
index 246cb9d..159f499 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java
@@ -149,11 +149,7 @@
ruleContext.getWorkspaceName(),
ruleContext.getConfiguration().legacyExternalRunfiles());
builder.addArtifact(common.getExecutable());
- if (common.getConvertedFiles() != null) {
- builder.addSymlinks(common.getConvertedFiles());
- } else {
- builder.addTransitiveArtifacts(common.getFilesToBuild());
- }
+ builder.addTransitiveArtifacts(common.getFilesToBuild());
semantics.collectDefaultRunfiles(ruleContext, builder);
builder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java
index 8446d4c..3bdaa0a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java
@@ -58,11 +58,7 @@
Runfiles.Builder runfilesBuilder = new Runfiles.Builder(
ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles());
- if (common.getConvertedFiles() != null) {
- runfilesBuilder.addSymlinks(common.getConvertedFiles());
- } else {
- runfilesBuilder.addTransitiveArtifacts(filesToBuild);
- }
+ runfilesBuilder.addTransitiveArtifacts(filesToBuild);
runfilesBuilder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);
runfilesBuilder.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
index 85e8367..1956dbc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
@@ -14,21 +14,12 @@
package com.google.devtools.build.lib.rules.python;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactRoot;
-import com.google.devtools.build.lib.analysis.FilesToRunProvider;
-import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
-import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.HashMap;
import java.util.HashSet;
-import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
-import javax.annotation.Nullable;
/** Various utility methods for Python support. */
public final class PythonUtils {
@@ -108,85 +99,4 @@
private PythonUtils() {
// This is a utility class, not to be instantiated.
}
-
- /**
- * Get the artifact generated by the 2to3 action.
- *
- * <p>There might be conflicts eg. when the input file is generated, but that case is unsupported
- * because 2to3 is obsolete.
- *
- * <p>Returns null and reports a rule error if the output file cannot be created because it is not
- * underneath the target's package.
- */
- @Nullable
- private static Artifact get2to3OutputArtifact(RuleContext ruleContext, Artifact input) {
- PathFragment rootRelativePath =
- input.getOutputDirRelativePath(ruleContext.getConfiguration().isSiblingRepositoryLayout());
- if (!rootRelativePath.startsWith(ruleContext.getPackageDirectory())) {
- ruleContext.ruleError(
- String.format(
- "cannot perform 2to3 conversion on source file %s from another package",
- rootRelativePath));
- return null;
- }
- ArtifactRoot root = ruleContext.getGenfilesDirectory();
- return ruleContext.getDerivedArtifact(rootRelativePath, root);
- }
-
- /**
- * Creates an action for each Python 2 file to convert to Python 3.
- *
- * <p>Returns null and reports a rule error if an output file cannot be created.
- */
- @Nullable
- public static Map<PathFragment, Artifact> generate2to3Actions(
- RuleContext ruleContext, Iterable<Artifact> inputs) {
- // This creates many actions, but this is fine. Creating one action per library leads
- // to some problems (when the same file is generated by two different actions), with
- // little benefits and negligible memory improvement.
-
- Map<PathFragment, Artifact> symlinks = new HashMap<>();
- for (Artifact input : inputs) {
- Artifact output = generate2to3Action(ruleContext, input);
- if (output == null) {
- return null;
- }
- symlinks.put(input.getRootRelativePath(), output);
- }
- return symlinks;
- }
-
- /**
- * Generates and registers one 2to3 action.
- *
- * <p>Returns null and reports an error if the output file cannot be created.
- */
- @Nullable
- private static Artifact generate2to3Action(RuleContext ruleContext, Artifact input) {
- FilesToRunProvider py2to3converter = ruleContext.getExecutablePrerequisite("$python2to3");
- Artifact output = get2to3OutputArtifact(ruleContext, input);
- if (output == null) {
- return null;
- }
-
- CustomCommandLine.Builder commandLine =
- CustomCommandLine.builder()
- .add("--no-diffs")
- .add("--nobackups")
- .add("--write")
- .addPath("--output-dir", output.getExecPath().getParentDirectory())
- .add("--write-unchanged-files")
- .addExecPath(input);
-
- ruleContext.registerAction(
- new SpawnAction.Builder()
- .addInput(input)
- .addOutput(output)
- .setExecutable(py2to3converter)
- .setProgressMessage("Converting to Python 3: %s", input.prettyPrint())
- .setMnemonic("2to3")
- .addCommandLine(commandLine.build())
- .build(ruleContext));
- return output;
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
index 39e91db..a95f246 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
@@ -80,8 +80,7 @@
" toolchain = ':default_py_runtime_pair',",
" toolchain_type = ':toolchain_type',",
")",
- "exports_files(['precompile.py'])",
- "sh_binary(name='2to3', srcs=['2to3.sh'])");
+ "exports_files(['precompile.py'])");
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
index 8ac9cbf..cdb1e34 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
@@ -1068,8 +1068,8 @@
// Works for implicit edges too. This is for consistency with --output
// xml, which exposes them too.
String toolsRepository = helper.getToolsRepository();
- assertThat(eval("labels(\"$python2to3\", //k)"))
- .isEqualTo(eval(toolsRepository + "//tools/python:2to3"));
+ assertThat(eval("labels(\"$py_toolchain_type\", //k)"))
+ .isEqualTo(eval(toolsRepository + "//tools/python:toolchain_type"));
// Configurable deps:
if (testConfigurableAttributes()) {
diff --git a/tools/python/BUILD.tools b/tools/python/BUILD.tools
index e289d95..5031ee9 100644
--- a/tools/python/BUILD.tools
+++ b/tools/python/BUILD.tools
@@ -19,30 +19,6 @@
package(default_visibility = ["//visibility:public"])
-write_file(
- name = "2to3",
- out = select({
- "//src/conditions:host_windows": "2to3.bat",
- "//conditions:default": "2to3.sh",
- }),
- content = select({
- "//src/conditions:host_windows": [
- "@echo >&2 ERROR: 2to3 is not implemented.",
- "@echo >&2 Please either build this target for PY2 or else set srcs_version to PY2ONLY instead of PY2.",
- "@echo >&2 See https://github.com/bazelbuild/bazel/issues/1393#issuecomment-431110617",
- "@exit /B 1",
- ],
- "//conditions:default": [
- "#!/bin/bash",
- "echo >&2 'ERROR: 2to3 is not implemented.'",
- "echo >&2 'Please either build this target for PY2 or else set srcs_version to PY2ONLY instead of PY2.'",
- "echo >&2 'See https://github.com/bazelbuild/bazel/issues/1393#issuecomment-431110617'",
- "exit 1",
- ],
- }),
- is_executable = True,
-)
-
# These exports are needed for Starlark tooling to work on files that
# transitive load these ones. Ideally they would be bzl_library targets, but we
# don't have access to Skylib here. See
