Skip to content

Commit

Permalink
Remove tests requiring Python 2 support.
Browse files Browse the repository at this point in the history 
This is in preparation for disabling Python 2 functionality. There is more code
and tests related to Python 2 support, but these are the ones that fail when
`--incompatible_python_disable_py2=true`.

This also revealed that rejecting Python 2 settings for `py_runtime` and
`py_runtime_pair` will have to wait until after the flag flip. This is because
the auto-detecting toolchains define some py2 toolchains, so in order to make
tests pass, those have to be removed, but doing so would break Python 2 support
prior to the flag flip. The other rules still reject them, though, which is
plenty sufficient.

Work towards #15684

PiperOrigin-RevId: 504086890
Change-Id: Ib39e3b32076072f5510418241cd0ca4765e6ab44
  • Loading branch information
@rickeylev @copybara-github
rickeylev authored and copybara-github committed Jan 23, 2023
1 parent b24930f commit bcb1fea
Show file tree
Hide file tree
Showing 16 changed files with 81 additions and 830 deletions.
7 changes: 4 additions & 3 deletions src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +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")
# TODO: Uncomment this after --incompatible_python_disable_py2 defaults to true
# 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(
Expand Down
112 changes: 35 additions & 77 deletions ...a/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ public void runtimeSetByPythonTop() throws Exception {
"pkg/BUILD",
"py_runtime(",
" name = 'my_py_runtime',",
" interpreter_path = '/system/python2',",
" python_version = 'PY2',",
" interpreter_path = '/system/python3',",
" python_version = 'PY3',",
")",
"py_binary(",
" name = 'pybin',",
Expand All @@ -114,7 +114,7 @@ public void runtimeSetByPythonTop() throws Exception {
analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:my_py_runtime");
useConfiguration("--incompatible_use_python_toolchains=false", "--python_top=" + pythonTop);
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
assertThat(path).isEqualTo("/system/python2");
assertThat(path).isEqualTo("/system/python3");
}

@Test
Expand Down Expand Up @@ -149,8 +149,8 @@ public void pythonTopTakesPrecedenceOverPythonPath() throws Exception {
"pkg/BUILD",
"py_runtime(",
" name = 'my_py_runtime',",
" interpreter_path = '/system/python2',",
" python_version = 'PY2',",
" interpreter_path = '/system/python3',",
" python_version = 'PY3',",
")",
"py_binary(",
" name = 'pybin',",
Expand All @@ -163,7 +163,7 @@ public void pythonTopTakesPrecedenceOverPythonPath() throws Exception {
"--python_top=" + pythonTop,
"--python_path=/better/not/be/this/one");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
assertThat(path).isEqualTo("/system/python2");
assertThat(path).isEqualTo("/system/python3");
}

// TODO(brandjon): Move generic toolchain tests that don't access legacy behavior to
Expand All @@ -177,20 +177,13 @@ private void defineToolchains() throws Exception {
"toolchains/BUILD",
"load('" + TOOLCHAIN_BZL + "', 'py_runtime_pair')",
"py_runtime(",
" name = 'py2_runtime',",
" interpreter_path = '/system/python2',",
" python_version = 'PY2',",
" stub_shebang = '#!/usr/bin/env python',",
")",
"py_runtime(",
" name = 'py3_runtime',",
" interpreter_path = '/system/python3',",
" python_version = 'PY3',",
" stub_shebang = '#!/usr/bin/env python3',",
")",
"py_runtime_pair(",
" name = 'py_runtime_pair',",
" py2_runtime = ':py2_runtime',",
" py3_runtime = ':py3_runtime',",
")",
"toolchain(",
Expand All @@ -199,12 +192,12 @@ private void defineToolchains() throws Exception {
" toolchain_type = '" + TOOLCHAIN_TYPE + "',",
")",
"py_runtime_pair(",
" name = 'py_runtime_pair_for_py2_only',",
" py2_runtime = ':py2_runtime',",
" name = 'py_runtime_pair_for_py3_only',",
" py3_runtime = ':py3_runtime',",
")",
"toolchain(",
" name = 'py_toolchain_for_py2_only',",
" toolchain = ':py_runtime_pair_for_py2_only',",
" name = 'py_toolchain_for_py3_only',",
" toolchain = ':py_runtime_pair_for_py3_only',",
" toolchain_type = '" + TOOLCHAIN_TYPE + "',",
")");
}
Expand All @@ -215,11 +208,6 @@ public void runtimeObtainedFromToolchain() throws Exception {
scratch.file(
"pkg/BUILD",
"py_binary(",
" name = 'py2_bin',",
" srcs = ['py2_bin.py'],",
" python_version = 'PY2',",
")",
"py_binary(",
" name = 'py3_bin',",
" srcs = ['py3_bin.py'],",
" python_version = 'PY3',",
Expand All @@ -228,17 +216,12 @@ public void runtimeObtainedFromToolchain() throws Exception {
"--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain");

ConfiguredTarget py2 = getConfiguredTarget("//pkg:py2_bin");
ConfiguredTarget py3 = getConfiguredTarget("//pkg:py3_bin");

String py2Path = getInterpreterPathFromStub(py2);
String py3Path = getInterpreterPathFromStub(py3);
assertThat(py2Path).isEqualTo("/system/python2");
assertThat(py3Path).isEqualTo("/system/python3");

String py2Shebang = getShebangFromStub(py2);
String py3Shebang = getShebangFromStub(py3);
assertThat(py2Shebang).isEqualTo("#!/usr/bin/env python");
assertThat(py3Shebang).isEqualTo("#!/usr/bin/env python3");
}

Expand All @@ -248,41 +231,21 @@ public void toolchainCanOmitUnusedRuntimeVersion() throws Exception {
scratch.file(
"pkg/BUILD",
"py_binary(",
" name = 'py2_bin',",
" srcs = ['py2_bin.py'],",
" python_version = 'PY2',",
" name = 'py3_bin',",
" srcs = ['py3_bin.py'],",
" python_version = 'PY3',",
")");
useConfiguration(
"--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
"--extra_toolchains=//toolchains:py_toolchain_for_py3_only");

String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
assertThat(path).isEqualTo("/system/python2");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py3_bin"));
assertThat(path).isEqualTo("/system/python3");
}

@Test
public void toolchainTakesPrecedenceOverLegacyFlags() throws Exception {
defineToolchains();
scratch.file(
"pkg/BUILD",
"py_binary(",
" name = 'py2_bin',",
" srcs = ['py2_bin.py'],",
" python_version = 'PY2',",
")");
useConfiguration(
"--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain",
"--python_path=/better/not/be/this/one");

String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
assertThat(path).isEqualTo("/system/python2");
}

@Test
public void toolchainIsMissingNeededRuntime() throws Exception {
defineToolchains();
reporter.removeHandler(failFastHandler);
scratch.file(
"pkg/BUILD",
"py_binary(",
Expand All @@ -292,10 +255,11 @@ public void toolchainIsMissingNeededRuntime() throws Exception {
")");
useConfiguration(
"--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
"--extra_toolchains=//toolchains:py_toolchain",
"--python_path=/better/not/be/this/one");

getConfiguredTarget("//pkg:py3_bin");
assertContainsEvent("The Python toolchain does not provide a runtime for Python version PY3");
String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py3_bin"));
assertThat(path).isEqualTo("/system/python3");
}

/**
Expand Down Expand Up @@ -330,16 +294,16 @@ private void defineCustomToolchain(String... lines) throws Exception {
}

/**
* Defines a PY2 py_binary target at //pkg:pybin, configures it to use the custom toolchain
* Defines a py_binary target at //pkg:pybin, configures it to use the custom toolchain
* //toolchains:custom, and attempts to retrieve it with {@link #getConfiguredTarget}.
*/
private void analyzePy2BinaryTargetUsingCustomToolchain() throws Exception {
private void analyzePyBinaryTargetUsingCustomToolchain() throws Exception {
scratch.file(
"pkg/BUILD",
"py_binary(",
" name = 'pybin',",
" srcs = ['pybin.py'],",
" python_version = 'PY2',",
" python_version = 'PY3',",
")");
useConfiguration(
"--incompatible_use_python_toolchains=true",
Expand All @@ -352,48 +316,42 @@ public void toolchainInfoFieldIsMissing() throws Exception {
reporter.removeHandler(failFastHandler);
defineCustomToolchain(
"return platform_common.ToolchainInfo(",
" py2_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python2',",
" python_version = 'PY2')",
" py3_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python3',",
" python_version = 'PY3')",
")");
// Use PY2 binary to test that we still validate the PY3 field even when it's not needed.
analyzePy2BinaryTargetUsingCustomToolchain();
analyzePyBinaryTargetUsingCustomToolchain();
assertContainsEvent(
"Error parsing the Python toolchain's ToolchainInfo: field 'py3_runtime' is missing");
"Error parsing the Python toolchain's ToolchainInfo: field 'py2_runtime' is missing");
}

@Test
public void toolchainInfoFieldHasBadType() throws Exception {
reporter.removeHandler(failFastHandler);
defineCustomToolchain(
"return platform_common.ToolchainInfo(",
" py2_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python2',",
" python_version = 'PY2'),",
" py3_runtime = 'abc',",
" py3_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python3',",
" python_version = 'PY3'),",
" py2_runtime = 'abc',",
")");
// Use PY2 binary to test that we still validate the PY3 field even when it's not needed.
analyzePy2BinaryTargetUsingCustomToolchain();
analyzePyBinaryTargetUsingCustomToolchain();
assertContainsEvent(
"Error parsing the Python toolchain's ToolchainInfo: Expected a PyRuntimeInfo in field "
+ "'py3_runtime', but got 'string'");
+ "'py2_runtime', but got 'string'");
}

@Test
public void toolchainInfoFieldHasBadVersion() throws Exception {
reporter.removeHandler(failFastHandler);
defineCustomToolchain(
"return platform_common.ToolchainInfo(",
" py2_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python2',",
" python_version = 'PY2'),",
" py3_runtime = PyRuntimeInfo(",
" interpreter_path = '/system/python3',",
// python_version is erroneously set to PY2 for the PY3 field.
" python_version = 'PY2'),",
")");
// Use PY2 binary to test that we still validate the PY3 field even when it's not needed.
analyzePy2BinaryTargetUsingCustomToolchain();
analyzePyBinaryTargetUsingCustomToolchain();
assertContainsEvent(
"Error retrieving the Python runtime from the toolchain: Expected field 'py3_runtime' to "
+ "have a runtime with python_version = 'PY3', but got python_version = 'PY2'");
Expand Down
6 changes: 0 additions & 6 deletions src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,12 @@ public void setup(MockToolsConfig config) throws IOException {
"constraint_setting(name = 'py2_interpreter_path')",
"constraint_setting(name = 'py3_interpreter_path')",
"py_runtime(",
" name = 'py2_interpreter',",
" interpreter_path = '/usr/bin/mockpython2',",
" python_version = 'PY2',",
")",
"py_runtime(",
" name = 'py3_interpreter',",
" interpreter_path = '/usr/bin/mockpython3',",
" python_version = 'PY3',",
")",
"py_runtime_pair(",
" name = 'default_py_runtime_pair',",
" py2_runtime = ':py2_interpreter',",
" py3_runtime = ':py3_interpreter',",
")",
"toolchain(",
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void goodSrcsVersionValue() throws Exception {
"pkg/BUILD",
ruleName + "(",
" name = 'foo',",
" srcs_version = 'PY2',",
" srcs_version = 'PY3',",
" srcs = ['foo.py'])");
getConfiguredTarget("//pkg:foo");
assertNoEvents();
Expand Down
49 changes: 0 additions & 49 deletions src/test/java/com/google/devtools/build/lib/rules/python/PyBinaryConfiguredTargetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,55 +32,6 @@ public PyBinaryConfiguredTargetTest() {
super("py_binary");
}

/**
* Creates a target //pkg:bin with the given version attr and that depends on a target //pkg:lib
* having the given sources version attr.
*/
private void declareBinDependingOnLibWithVersions(String binVersion, String libSrcsVersion)
throws Exception {
scratch.file(
"pkg/BUILD",
"py_library(name = 'lib',",
" srcs = [],",
" srcs_version = '" + libSrcsVersion + "')",
"py_binary(name = 'bin',",
" srcs = ['bin.py'],",
" deps = [':lib'],",
" python_version = '" + binVersion + "')");
}

@Test
public void python2WithPy3SrcsVersionDependency() throws Exception {
PythonTestUtils.assumeIsBazel(); // Google has py2 disabled
setBuildLanguageOptions(
"--experimental_builtins_injection_override=-py_test,-py_binary,-py_library");
declareBinDependingOnLibWithVersions("PY2", "PY3");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.startsWith(
"//pkg:bin: This target is being built for Python 2 but (transitively) "
+ "includes Python 3-only sources");
}

@Test
public void python2WithPy3OnlySrcsVersionDependency() throws Exception {
PythonTestUtils.assumeIsBazel(); // Google has py2 disabled
setBuildLanguageOptions(
"--experimental_builtins_injection_override=-py_test,-py_binary,-py_library");
declareBinDependingOnLibWithVersions("PY2", "PY3ONLY");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.contains("being built for Python 2 but (transitively) includes Python 3-only sources");
}

@Test
public void python3WithPy2OnlySrcsVersionDependency_NewSemantics() throws Exception {
PythonTestUtils.assumeIsBazel(); // Google has py2 disabled
setBuildLanguageOptions(
"--experimental_builtins_injection_override=-py_test,-py_binary,-py_library");
declareBinDependingOnLibWithVersions("PY3", "PY2ONLY");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.contains("being built for Python 3 but (transitively) includes Python 2-only sources");
}

@Test
public void filesToBuild() throws Exception {
scratch.file("pkg/BUILD",
Expand Down
Loading

0 comments on commit bcb1fea

Please sign in to comment.