From d1bbf4b7698e019c11b7d1c483eb0b4959954060 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 13 Jan 2023 16:02:28 -0800 Subject: [PATCH] Add --incompatible_python_disable_py2, which fails if Python 2 is specified. 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 --- .../rules/python/BazelPythonSemantics.java | 30 ++++++++++++++----- .../lib/rules/python/PythonConfiguration.java | 10 +++++++ .../build/lib/rules/python/PythonOptions.java | 13 ++++++++ .../common/python/py_runtime_rule.bzl | 4 +++ tools/python/toolchain.bzl | 13 ++++++++ 5 files changed, 62 insertions(+), 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 index 1c7b478facf743..56391e6cc4010b 100644 --- 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 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 index c60884146512dc..8840e3bb694910 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 @@ -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 index e142ac123d728d..cefda45dc6117a 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 @@ -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 index e50103b0b12475..b6ce98d3c8dfdb 100644 --- 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 index 0870173b81ea97..4017bb75794d07 100644 --- 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.