Introduce --incompatible_use_python_toolchains
This renames --experimental_use_python_toolchains to --incompatible. It also adds the behavior to the flag that
1) py_runtime's python_version attribute becomes manadatory, and
2) the --python_top flag is disallowed (it is superseded by toolchains), as well as --python2_path and --python3_path (they were already no-ops). --python_path is strongly discouraged but not yet banned due to an existing use on Windows (#7901).
Feature issue is #7375, incompatible change migration issue is #7899.
RELNOTES[INC]: Introduced --incompatible_use_python_toolchains, which supersedes --python_top/--python_path. See #7899 and #7375 for more information.
PiperOrigin-RevId: 241134532
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
index d19e56b..fb5caf6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
@@ -24,9 +24,10 @@
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.rules.python.PythonOptions;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
@@ -35,77 +36,61 @@
/** Bazel-specific Python configuration. */
@Immutable
public class BazelPythonConfiguration extends BuildConfiguration.Fragment {
- /**
- * A path converter for python3 path
- */
- public static class Python3PathConverter implements Converter<String> {
- @Override
- public String convert(String input) {
- if (input.equals("auto")) {
- return OS.getCurrent() == OS.WINDOWS ? "python" : "python3";
- }
- return input;
- }
-
- @Override
- public String getTypeDescription() {
- return "An option for python3 path";
- }
- }
/** Bazel-specific Python configuration options. */
public static final class Options extends FragmentOptions {
@Option(
- name = "python2_path",
- defaultValue = "python",
- documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
- metadataTags = {OptionMetadataTag.DEPRECATED},
- help =
- "Local path to the Python2 executable. "
- + "Deprecated, please use python_path or python_top instead."
- )
+ name = "python2_path",
+ defaultValue = "null",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.NO_OP},
+ metadataTags = {OptionMetadataTag.DEPRECATED},
+ help = "Deprecated, no-op. Disabled by `--incompatible_use_python_toolchains`.")
public String python2Path;
@Option(
- name = "python3_path",
- converter = Python3PathConverter.class,
- defaultValue = "auto",
- documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
- metadataTags = {OptionMetadataTag.DEPRECATED},
- help =
- "Local path to the Python3 executable. "
- + "Deprecated, please use python_path or python_top instead."
- )
+ name = "python3_path",
+ defaultValue = "null",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.NO_OP},
+ metadataTags = {OptionMetadataTag.DEPRECATED},
+ help = "Deprecated, no-op. Disabled by `--incompatible_use_python_toolchains`.")
public String python3Path;
@Option(
- name = "python_top",
- converter = LabelConverter.class,
- defaultValue = "null",
- documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
- help = "The label of py_runtime rule used for the Python interpreter invoked by Bazel."
- )
+ name = "python_top",
+ converter = LabelConverter.class,
+ defaultValue = "null",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+ help =
+ "The label of a py_runtime representing the Python interpreter invoked to run Python "
+ + "targets on the target platform. Deprecated; disabled by "
+ + "--incompatible_use_python_toolchains.")
public Label pythonTop;
@Option(
- name = "python_path",
- defaultValue = "python",
- documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
- help = "The absolute path of the Python interpreter invoked by Bazel."
- )
+ name = "python_path",
+ defaultValue = "null",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+ help =
+ "The absolute path of the Python interpreter invoked to run Python targets on the "
+ + "target platform. Deprecated; disabled by --incompatible_use_python_toolchains.")
public String pythonPath;
@Option(
- name = "experimental_python_import_all_repositories",
- defaultValue = "true",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
- help = "Do not use."
- )
+ name = "experimental_python_import_all_repositories",
+ defaultValue = "true",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ help =
+ "If true, the roots of repositories in the runfiles tree are added to PYTHONPATH, so "
+ + "that imports like `import mytoplevelpackage.package.module` are valid."
+ + " Regardless of whether this flag is true, the runfiles root itself is also"
+ + " added to the PYTHONPATH, so "
+ + "`import myreponame.mytoplevelpackage.package.module` is valid. The latter form "
+ + "is less likely to experience import name collisions.")
public boolean experimentalPythonImportAllRepositories;
/**
@@ -143,7 +128,7 @@
@Override
public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
- return ImmutableSet.<Class<? extends FragmentOptions>>of(Options.class);
+ return ImmutableSet.of(Options.class, PythonOptions.class);
}
}
@@ -153,12 +138,35 @@
this.options = options;
}
- public String getPython2Path() {
- return options.python2Path;
- }
-
- public String getPython3Path() {
- return options.python3Path;
+ @Override
+ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
+ PythonOptions pythonOpts = buildOptions.get(PythonOptions.class);
+ Options opts = buildOptions.get(Options.class);
+ if (pythonOpts.incompatibleUsePythonToolchains) {
+ // Forbid deprecated flags.
+ if (opts.python2Path != null) {
+ reporter.handle(
+ Event.error(
+ "`--python2_path` is disabled by `--incompatible_use_python_toolchains`. Since "
+ + "`--python2_path` is a deprecated no-op, there is no need to pass it."));
+ }
+ if (opts.python3Path != null) {
+ reporter.handle(
+ Event.error(
+ "`--python3_path` is disabled by `--incompatible_use_python_toolchains`. Since "
+ + "`--python3_path` is a deprecated no-op, there is no need to pass it."));
+ }
+ if (opts.pythonTop != null) {
+ reporter.handle(
+ Event.error(
+ "`--python_top` is disabled by `--incompatible_use_python_toolchains`. Instead of "
+ + "configuring the Python runtime directly, register a Python toolchain. See "
+ + "https://github.com/bazelbuild/bazel/issues/7899. You can temporarily revert "
+ + "to the legacy flag-based way of specifying toolchains by setting "
+ + "`--incompatible_use_python_toolchains=false`."));
+ }
+ // TODO(#7901): Also prohibit --python_path here.
+ }
}
public Label getPythonTop() {
@@ -166,7 +174,7 @@
}
public String getPythonPath() {
- return options.pythonPath;
+ return options.pythonPath == null ? "python" : options.pythonPath;
}
public boolean getImportAllRepositories() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java
index 4459f39..3b145b6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java
@@ -33,6 +33,8 @@
@Override
public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictException {
+ PythonConfiguration pyConfig = ruleContext.getFragment(PythonConfiguration.class);
+
NestedSet<Artifact> files =
PrerequisiteArtifacts.nestedSet(ruleContext, "files", Mode.TARGET);
Artifact interpreter = ruleContext.getPrerequisiteArtifact("interpreter", Mode.TARGET);
@@ -58,15 +60,24 @@
}
if (pythonVersion == PythonVersion._INTERNAL_SENTINEL) {
- // Use the same default as py_binary/py_test would use for their python_version attribute.
- // (Of course, in our case there's no configuration transition involved.)
- pythonVersion = ruleContext.getFragment(PythonConfiguration.class).getDefaultPythonVersion();
+ if (pyConfig.useToolchains()) {
+ ruleContext.attributeError(
+ "python_version",
+ "When using Python toolchains, this attribute must be set explicitly to either 'PY2' "
+ + "or 'PY3'. See https://github.com/bazelbuild/bazel/issues/7899 for more "
+ + "information. You can temporarily avoid this error by reverting to the legacy "
+ + "Python runtime mechanism (`--incompatible_use_python_toolchains=false`).");
+ } else {
+ // Use the same default as py_binary/py_test would use for their python_version attribute.
+ // (Of course, in our case there's no configuration transition involved.)
+ pythonVersion = pyConfig.getDefaultPythonVersion();
+ }
}
- Preconditions.checkState(pythonVersion.isTargetValue());
if (ruleContext.hasErrors()) {
return null;
}
+ Preconditions.checkState(pythonVersion.isTargetValue());
PyRuntimeInfo provider =
hermetic
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 0468469..fa99438 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
@@ -230,11 +230,14 @@
public boolean incompatibleDisallowLegacyPyProvider;
@Option(
- // TODO(brandjon): Rename to --incompatible_use_python_toolchains when ready to make available
- name = "experimental_use_python_toolchains",
+ name = "incompatible_use_python_toolchains",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
help =
"If set to true, executable native Python rules will use the Python runtime specified by "
+ "the Python toolchain, rather than the runtime given by legacy flags like "
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
index 4e96207..a878000 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
@@ -94,7 +94,7 @@
")");
String pythonTop =
analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:my_py_runtime");
- useConfiguration("--experimental_use_python_toolchains=false", "--python_top=" + pythonTop);
+ useConfiguration("--incompatible_use_python_toolchains=false", "--python_top=" + pythonTop);
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
assertThat(path).isEqualTo("/system/python2");
}
@@ -107,7 +107,7 @@
" name = 'pybin',",
" srcs = ['pybin.py'],",
")");
- useConfiguration("--experimental_use_python_toolchains=false", "--python_path=/system/python2");
+ useConfiguration("--incompatible_use_python_toolchains=false", "--python_path=/system/python2");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
assertThat(path).isEqualTo("/system/python2");
}
@@ -120,7 +120,7 @@
" name = 'pybin',",
" srcs = ['pybin.py'],",
")");
- useConfiguration("--experimental_use_python_toolchains=false");
+ useConfiguration("--incompatible_use_python_toolchains=false");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
assertThat(path).isEqualTo("python");
}
@@ -141,7 +141,7 @@
String pythonTop =
analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:my_py_runtime");
useConfiguration(
- "--experimental_use_python_toolchains=false",
+ "--incompatible_use_python_toolchains=false",
"--python_top=" + pythonTop,
"--python_path=/better/not/be/this/one");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
@@ -205,7 +205,7 @@
" python_version = 'PY3',",
")");
useConfiguration(
- "--experimental_use_python_toolchains=true",
+ "--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain");
String py2Path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -225,7 +225,7 @@
" python_version = 'PY2',",
")");
useConfiguration(
- "--experimental_use_python_toolchains=true",
+ "--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -237,24 +237,14 @@
defineToolchains();
scratch.file(
"pkg/BUILD",
- "py_runtime(",
- " name = 'dont_use_this_runtime',",
- " interpreter_path = '/dont/use/me',",
- " python_version = 'PY2',",
- ")",
"py_binary(",
" name = 'py2_bin',",
" srcs = ['py2_bin.py'],",
" python_version = 'PY2',",
")");
- String pythonTop =
- analysisMock
- .pySupport()
- .createPythonTopEntryPoint(mockToolsConfig, "//pkg:dont_use_this_runtime");
useConfiguration(
- "--experimental_use_python_toolchains=true",
+ "--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain",
- "--python_top=" + pythonTop,
"--python_path=/better/not/be/this/one");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -273,7 +263,7 @@
" python_version = 'PY3',",
")");
useConfiguration(
- "--experimental_use_python_toolchains=true",
+ "--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
getConfiguredTarget("//pkg:py3_bin");
@@ -324,7 +314,7 @@
" python_version = 'PY2',",
")");
useConfiguration(
- "--experimental_use_python_toolchains=true",
+ "--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:custom_toolchain");
getConfiguredTarget("//pkg:pybin");
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java
index cdb3223..5b5eb2d 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java
@@ -61,4 +61,23 @@
() -> create("--python_path=pajama"));
assertThat(expected).hasMessageThat().contains("python_path must be an absolute path");
}
+
+ @Test
+ public void legacyFlagsDeprecatedByPythonToolchains() throws Exception {
+ checkError(
+ "`--python2_path` is disabled by `--incompatible_use_python_toolchains`",
+ "--incompatible_use_python_toolchains=true",
+ "--python2_path=/system/python2");
+ checkError(
+ "`--python3_path` is disabled by `--incompatible_use_python_toolchains`",
+ "--incompatible_use_python_toolchains=true",
+ "--python3_path=/system/python3");
+ checkError(
+ "`--python_top` is disabled by `--incompatible_use_python_toolchains`",
+ "--incompatible_use_python_toolchains=true",
+ "--python_top=//mypkg:my_py_runtime");
+ // TODO(#7901): Also test that --python_path is disallowed (once implemented). Currently we
+ // still need it to communicate the location of python.exe on Windows from the client to the
+ // server.
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeConfiguredTargetTest.java
index 8569131..75e498f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeConfiguredTargetTest.java
@@ -73,6 +73,8 @@
@Test
public void pythonVersionDefault() throws Exception {
ensureDefaultIsPY2();
+ // When using toolchains, the python_version attribute is mandatory.
+ useConfiguration("--incompatible_use_python_toolchains=false");
scratch.file(
"pkg/BUILD",
"py_runtime(",
@@ -170,4 +172,19 @@
assertContainsEvent("invalid value in 'python_version' attribute");
}
+
+ @Test
+ public void versionAttributeMandatoryWhenUsingToolchains() throws Exception {
+ reporter.removeHandler(failFastHandler);
+ useConfiguration("--incompatible_use_python_toolchains=true");
+ scratch.file(
+ "pkg/BUILD",
+ "py_runtime(",
+ " name = 'myruntime',",
+ " interpreter_path = '/system/interpreter',",
+ ")");
+ getConfiguredTarget("//pkg:myruntime");
+
+ assertContainsEvent("must be set explicitly to either 'PY2' or 'PY3'");
+ }
}
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 ab93932..b2b5f12 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
@@ -221,7 +221,7 @@
"--incompatible_py2_outputs_are_suffixed=true",
"--build_python_zip=true",
"--incompatible_disallow_legacy_py_provider=true",
- "--experimental_use_python_toolchains=true");
+ "--incompatible_use_python_toolchains=true");
PythonOptions hostOpts = (PythonOptions) opts.getHost();
assertThat(hostOpts.incompatibleAllowPythonVersionTransitions).isTrue();
assertThat(hostOpts.incompatibleRemoveOldPythonVersionApi).isTrue();
diff --git a/src/test/shell/bazel/bazel_example_test.sh b/src/test/shell/bazel/bazel_example_test.sh
index 168d90c..663dfef 100755
--- a/src/test/shell/bazel/bazel_example_test.sh
+++ b/src/test/shell/bazel/bazel_example_test.sh
@@ -97,13 +97,13 @@
}
function test_native_python() {
- assert_build //examples/py_native:bin --python2_path=python
- assert_test_ok //examples/py_native:test --python2_path=python
- assert_test_fails //examples/py_native:fail --python2_path=python
+ assert_build //examples/py_native:bin
+ assert_test_ok //examples/py_native:test
+ assert_test_fails //examples/py_native:fail
}
function test_native_python_with_zip() {
- assert_build //examples/py_native:bin --python2_path=python --build_python_zip
+ assert_build //examples/py_native:bin --build_python_zip
# run the python package directly
./bazel-bin/examples/py_native/bin >& $TEST_log \
|| fail "//examples/py_native:bin execution failed"
@@ -112,8 +112,8 @@
python ./bazel-bin/examples/py_native/bin >& $TEST_log \
|| fail "//examples/py_native:bin execution failed"
expect_log "Fib(5) == 8"
- assert_test_ok //examples/py_native:test --python2_path=python --build_python_zip
- assert_test_fails //examples/py_native:fail --python2_path=python --build_python_zip
+ assert_test_ok //examples/py_native:test --build_python_zip
+ assert_test_fails //examples/py_native:fail --build_python_zip
}
function test_shell() {
diff --git a/tools/python/pywrapper_template.txt b/tools/python/pywrapper_template.txt
index c0a67d1..c2f5b18 100644
--- a/tools/python/pywrapper_template.txt
+++ b/tools/python/pywrapper_template.txt
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# TODO(#7843): integration tests for failure to find python / wrong version
# found / error while trying to print version
