Add --incompatible_python_disable_py2, which fails if Python 2 is spe…

…cified. When true, using Python 2 only attribute values will cause an error. More specifically, using `python_version=PY2`, `srcs_version=PY2` or `srcs_version=PY2ONLY` with `py_binary`, `py_test`, `py_library`, `py_runtime`, or `py_runtime_pair` will result in an error. Work towards #15684 PiperOrigin-RevId: 501959931 Change-Id: I7bc089a2f7b46f2538e4a92d8753c788193a71d5
bazelbuild · Jan 14, 2023 · d1bbf4b · d1bbf4b
1 parent 24dbe0d
commit d1bbf4b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 8 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -39,6 +39,7 @@
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.python.PyCommon;
@@ -77,6 +78,19 @@ public String getSrcsVersionDocURL() {
 
   @Override
   public void validate(RuleContext ruleContext, PyCommon common) {
+    PythonConfiguration config = ruleContext.getFragment(PythonConfiguration.class);
+    if (config.getDisablePy2()) {
+      var attrs = ruleContext.attributes();
+      if (config.getDefaultPythonVersion().equals(PythonVersion.PY2)
+          || attrs.getOrDefault("python_version", Type.STRING, "UNSET").equals("PY2")
+          || attrs.getOrDefault("srcs_version", Type.STRING, "UNSET").equals("PY2")
+          || attrs.getOrDefault("srcs_version", Type.STRING, "UNSET").equals("PY2ONLY")) {
+        ruleContext.ruleError(
+            "Using Python 2 is not supported and disabled; see "
+                + "https://github.com/bazelbuild/bazel/issues/15684");
+        return;
+      }
+    }
   }
 
   @Override
@@ -116,18 +130,18 @@ public List<String> getImports(RuleContext ruleContext) {
     PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getRunfilesPath();
     // Python scripts start with x.runfiles/ as the module space, so everything must be manually
     // adjusted to be relative to the workspace name.
-    packageFragment = PathFragment.create(ruleContext.getWorkspaceName())
-        .getRelative(packageFragment);
+    packageFragment =
+        PathFragment.create(ruleContext.getWorkspaceName()).getRelative(packageFragment);
     for (String importsAttr : ruleContext.getExpander().list("imports")) {
       if (importsAttr.startsWith("/")) {
-        ruleContext.attributeWarning("imports",
-            "ignoring invalid absolute path '" + importsAttr + "'");
+        ruleContext.attributeWarning(
+            "imports", "ignoring invalid absolute path '" + importsAttr + "'");
         continue;
       }
       PathFragment importsPath = packageFragment.getRelative(importsAttr);
       if (importsPath.containsUplevelReferences()) {
-        ruleContext.attributeError("imports",
-            "Path " + importsAttr + " references a path above the execution root");
+        ruleContext.attributeError(
+            "imports", "Path " + importsAttr + " references a path above the execution root");
       }
       result.add(importsPath.getPathString());
     }
@@ -279,7 +293,7 @@ public void createExecutable(
           // unix. See also https://github.com/bazelbuild/bazel/issues/7947#issuecomment-491385802.
           pythonBinary,
           executable,
-          /*useZipFile=*/ buildPythonZip);
+          /* useZipFile= */ buildPythonZip);
     }
   }
 
@@ -468,7 +482,7 @@ private static String getPythonBinary(
         pythonBinary =
             workspaceName.getRelative(provider.getInterpreter().getRunfilesPath()).getPathString();
       }
-    } else  {
+    } else {
       // make use of the Python interpreter in an absolute path
       pythonBinary = bazelConfig.getPythonPath();
     }

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
@@ -52,6 +52,7 @@ public class PythonConfiguration extends Fragment implements StarlarkValue {
   private final boolean useToolchains;
 
   private final boolean defaultToExplicitInitPy;
+  private final boolean disablePy2;
 
   public PythonConfiguration(BuildOptions buildOptions) {
     PythonOptions pythonOptions = buildOptions.get(PythonOptions.class);
@@ -64,6 +65,7 @@ public PythonConfiguration(BuildOptions buildOptions) {
     this.py2OutputsAreSuffixed = pythonOptions.incompatiblePy2OutputsAreSuffixed;
     this.useToolchains = pythonOptions.incompatibleUsePythonToolchains;
     this.defaultToExplicitInitPy = pythonOptions.incompatibleDefaultToExplicitInitPy;
+    this.disablePy2 = pythonOptions.disablePy2;
   }
 
   @Override
@@ -172,4 +174,12 @@ public boolean useToolchains() {
   public boolean defaultToExplicitInitPy() {
     return defaultToExplicitInitPy;
   }
+
+  @StarlarkMethod(
+      name = "disable_py2",
+      structField = true,
+      doc = "The value of the --incompatible_python_disable_py2 flag.")
+  public boolean getDisablePy2() {
+    return disablePy2;
+  }
 }
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
@@ -119,6 +119,18 @@ public String getTypeDescription() {
               + "`--incompatible_py2_outputs_are_suffixed`.")
   public boolean incompatiblePy3IsDefault;
 
+  @Option(
+      name = "incompatible_python_disable_py2",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If true, using Python 2 settings will cause an error. This includes "
+              + "python_version=PY2, srcs_version=PY2, and srcs_version=PY2ONLY. See "
+              + "https://github.com/bazelbuild/bazel/issues/15684 for more information.")
+  public boolean disablePy2;
+
   @Option(
       name = "incompatible_py2_outputs_are_suffixed",
       defaultValue = "true",
@@ -342,6 +354,7 @@ public FragmentOptions getExec() {
     hostPythonOptions.incompatibleDefaultToExplicitInitPy = incompatibleDefaultToExplicitInitPy;
     hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
     hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
+    hostPythonOptions.disablePy2 = disablePy2;
 
     // Save host options in case of a further exec->host transition.
     hostPythonOptions.hostForcePython = hostForcePython;

diff --git a/src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl b/src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl
@@ -66,6 +66,10 @@ def _py_runtime_impl(ctx):
         else:
             python_version = ctx.fragments.py.default_python_version
 
+    if ctx.fragments.py.disable_py2 and python_version == "PY2":
+        fail("Using Python 2 is not supported and disabled; see " +
+             "https://github.com/bazelbuild/bazel/issues/15684")
+
     return [
         _PyRuntimeInfo(
             interpreter_path = interpreter_path or None,

diff --git a/tools/python/toolchain.bzl b/tools/python/toolchain.bzl
@@ -34,11 +34,23 @@ def _py_runtime_pair_impl(ctx):
     else:
         py3_runtime = None
 
+    if _is_py2_disabled(ctx) and py2_runtime != None:
+        fail("Using Python 2 is not supported and disabled; see " +
+             "https://github.com/bazelbuild/bazel/issues/15684")
+
     return [platform_common.ToolchainInfo(
         py2_runtime = py2_runtime,
         py3_runtime = py3_runtime,
     )]
 
+def _is_py2_disabled(ctx):
+    # In Google, this file isn't bundled with Bazel, so we have to conditionally
+    # check for this flag.
+    # TODO: Remove this once a build with the flag is released in Google
+    if not hasattr(ctx.fragments.py, "disable_py"):
+        return False
+    return ctx.fragments.py.disable_py2
+
 py_runtime_pair = rule(
     implementation = _py_runtime_pair_impl,
     attrs = {
@@ -61,6 +73,7 @@ The runtime to use for Python 3 targets. Must have `python_version` set to
 """,
         ),
     },
+    fragments = ["py"],
     doc = """\
 A toolchain rule for Python.