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 ee4b74b280076e..5f94e4b114451c 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 @@ -177,9 +177,6 @@ private static void createStubFile( Substitution.of("%is_zipfile%", boolToLiteral(isForZipFile)), Substitution.of( "%import_all%", boolToLiteral(bazelConfig.getImportAllRepositories())), - Substitution.of( - "%enable_host_version_warning%", - boolToLiteral(common.shouldWarnAboutHostVersionUponFailure())), Substitution.of("%target%", ruleContext.getRule().getLabel().getCanonicalForm()), Substitution.of( "%python_version_from_config%", versionToLiteral(common.getVersion())), diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt index e6dc9392e7a3d9..eb31c8dd1e4a48 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt @@ -205,84 +205,6 @@ def RunfilesEnvvar(module_space): return (None, None) -# TODO(#6443): Remove this once there's no longer a host configuration for -# Python targets to appear in. -def MaybeEmitHostVersionWarning(ret_code): - """Warns the user if a failure may be due to the host config's version. - - This emits a message to stderr if - 1) ret_code is non-zero, - 2) the target was built in the host config and with toolchains enabled, and - 3) at analysis time we detected a mismatch between the host config's version - and this target's explicitly declared version, or else this target did - not explicitly declare its version. (The former diagnoses targets - affected by #6443, and the latter diagnoses targets that are broken by - fixing #4815.) - - See also #7899, #8549, and PyCommon#shouldWarnAboutHostVersionUponFailure. - - Since this warning is emitted here in the stub script and not in Bazel itself, - it will be present in all failing runs of affected targets, even when executed - directly and not via `bazel run`. However, note that this warning is never - added to non-host-configured targets, and that it can be disabled by ensuring - the correct Python version is passed to --host_force_python and declared in - tools' python_version attributes. - - Args: - ret_code: The exit code of the payload user program - """ - if ret_code == 0: - return - if not %enable_host_version_warning%: - return - - host_version = %python_version_from_config% - target_version = %python_version_from_attr% - opposite_of_host_version = '2' if host_version == '3' else '3' - - if %python_version_specified_explicitly%: - # Mismatch with explicitly declared version. - diagnostic = """\ -Note: The failure of target {target} (with exit code {ret_code}) may have been \ -caused by the fact that it is a Python {target_version} program that was built \ -in the host configuration, which uses Python {host_version}. You can change \ -the host configuration (for the entire build) to instead use Python \ -{target_version} by setting --host_force_python=PY{target_version}.\ -""".format( - target='%target%', - ret_code=ret_code, - target_version=target_version, - host_version=host_version) - else: - diagnostic = """\ -Note: The failure of target {target} (with exit code {ret_code}) may have been \ -caused by the fact that it is running under Python {host_version} instead of \ -Python {opposite_of_host_version}. Examine the error to determine if that \ -appears to be the problem. Since this target is built in the host \ -configuration, the only way to change its version is to set \ ---host_force_python=PY{opposite_of_host_version}, which affects the entire \ -build.\ -""".format( - target='%target%', - ret_code=ret_code, - host_version=host_version, - opposite_of_host_version=opposite_of_host_version) - - # TODO(brandjon): Change the wording "You are likely seeing this message - # because" to something less strong after a few releases from 0.27. By that - # point, migration for toolchains won't be the main reason this error is seen - # by users. - message = """\ ----------------- -{diagnostic} - -If this error started occurring in Bazel 0.27 and later, it may be because the \ -Python toolchain now enforces that targets analyzed as PY2 and PY3 run under a \ -Python 2 and Python 3 interpreter, respectively. See \ -https://github.com/bazelbuild/bazel/issues/7899 for more information. -----------------""".format(diagnostic=diagnostic) - print(message, file=sys.stderr) - def Deduplicate(items): """Efficiently filter out duplicates, keeping the first element only.""" seen = set() @@ -379,19 +301,12 @@ def Main(): os.chdir(os.path.join(module_space, '%workspace_name%')) ret_code = subprocess.call(args) shutil.rmtree(os.path.dirname(module_space), True) - MaybeEmitHostVersionWarning(ret_code) sys.exit(ret_code) else: # On Windows, os.execv doesn't handle arguments with spaces correctly, # and it actually starts a subprocess just like subprocess.call. - # - # If we may need to emit a host config warning after execution, don't - # execv because we need control to return here. This only happens for - # targets built in the host config, so other targets still get to take - # advantage of the performance benefits of execv. - if IsWindows() or %enable_host_version_warning%: + if IsWindows(): ret_code = subprocess.call(args) - MaybeEmitHostVersionWarning(ret_code) sys.exit(ret_code) else: os.execv(args[0], args) 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 a1b5ce196dc113..be1fe8afa8dc55 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 @@ -580,68 +580,6 @@ public PythonVersion getSourcesVersion() { return sourcesVersion; } - /** - * Returns whether, in the case that a user Python program fails, the stub script should emit a - * warning that the failure may have been caused by the host configuration using the wrong Python - * version. - * - *

This method should only be called for executable Python rules. - * - *

Background: Historically, Bazel did not necessarily launch a Python interpreter whose - * version corresponded to the one determined by the analysis phase (#4815). Enabling Python - * toolchains fixed this bug. However, this caused some builds to break due to targets that - * contained Python-2-only code yet got analyzed for (and now run with) Python 3. This is - * particularly problematic for the host configuration, where the value of {@code - * --host_force_python} overrides the declared or implicit Python version of the target. - * - *

Our mitigation for this is to warn users when a Python target has a non-zero exit code and - * the failure could be due to a bad Python version in the host configuration. In this case, - * instead of just giving the user a confusing traceback of a PY2 vs PY3 error, we append a - * diagnostic message to stderr. See #7899 and especially #8549 for context. - * - *

This method returns true when all of the following hold: - * - *

    - *
  1. Python toolchains are enabled. (The warning is needed the most when toolchains are - * enabled, since that's an incompatible change likely to cause breakages. At the same time, - * warning when toolchains are disabled could be misleading, since we don't actually know - * whether the interpreter invoked at runtime is correct.) - *
  2. The target is built in the host configuration. This avoids polluting stderr with spurious - * warnings for non-host-configured targets, while covering the most problematic case. - *
  3. Either the value of {@code --host_force_python} overrode the target's normal Python - * version to a different value (in which case we know a mismatch occurred), or else {@code - * --host_force_python} is in agreement with the target's version but the target's version - * was set by default instead of explicitly (in which case we suspect the target may have - * been defined incorrectly). - *
- * - * @throws IllegalArgumentException if there is a problem parsing the Python version from the - * attributes; see {@link #readPythonVersionFromAttribute}. - */ - // TODO(#6443): Remove this logic and the corresponding stub script logic once we no longer have - // the possibility of Python binaries appearing in the host configuration. - public boolean shouldWarnAboutHostVersionUponFailure() { - // Only warn when toolchains are used. - PythonConfiguration config = ruleContext.getFragment(PythonConfiguration.class); - if (!config.useToolchains()) { - return false; - } - // Only warn in the host config. - if (!ruleContext.getConfiguration().isToolConfiguration()) { - return false; - } - - PythonVersion configVersion = config.getPythonVersion(); - PythonVersion attrVersion = readPythonVersionFromAttribute(ruleContext.attributes()); - if (attrVersion == null) { - // Warn if the version wasn't set explicitly. - return true; - } else { - // Warn if the explicit version is different from the host config's version. - return configVersion != attrVersion; - } - } - /** * Returns the transitive Python sources collected from the deps attribute, not including sources * from the srcs attribute (unless they were separately reached via deps). diff --git a/src/test/shell/bazel/python_version_test.sh b/src/test/shell/bazel/python_version_test.sh index 260b2ebd13d2d7..c7a44708432bc8 100755 --- a/src/test/shell/bazel/python_version_test.sh +++ b/src/test/shell/bazel/python_version_test.sh @@ -601,120 +601,6 @@ function test_default_output_root_is_bazel_bin() { expect_not_log "bazel-out/.*-py3.*/bin" } -# Tests that we get a warning when host tools fail due to being built for the -# wrong Python version. See #7899 and #8549 for context, as well as -# {@link PyCommon#shouldWarnAboutHostVersionUponFailure}. -# TODO(#6443): Delete this once we no longer have the host configuration. -function test_host_version_mismatch_warning() { - mkdir -p test - - cat > test/BUILD << 'EOF' -py_binary( - name = "passing_tool_using_py2_explicitly", - srcs = ["success.py"], - main = "success.py", - python_version = "PY2", -) - -py_binary( - name = "failing_tool_using_py2_explicitly", - srcs = ["fail.py"], - main = "fail.py", - python_version = "PY2", -) - -py_binary( - name = "failing_tool_using_py3_explicitly", - srcs = ["fail.py"], - main = "fail.py", - python_version = "PY3", -) - -py_binary( - name = "failing_tool_using_py3_implicitly", - srcs = ["fail.py"], - main = "fail.py", -) - -# For each tool, a genrule target that uses it. - -genrule( - name = "invoke_passing_tool_using_py2_explicitly", - cmd = "$(location :passing_tool_using_py2_explicitly) > $@", - tools = [":passing_tool_using_py2_explicitly"], - outs = ["passing_tool_using_py2_explicitly.txt"], -) - -genrule( - name = "invoke_failing_tool_using_py2_explicitly", - cmd = "$(location :failing_tool_using_py2_explicitly) > $@", - tools = [":failing_tool_using_py2_explicitly"], - outs = ["failing_tool_using_py2_explicitly.txt"], -) - -genrule( - name = "invoke_failing_tool_using_py3_explicitly", - cmd = "$(location :failing_tool_using_py3_explicitly) > $@", - tools = [":failing_tool_using_py3_explicitly"], - outs = ["failing_tool_using_py3_explicitly.txt"], -) - -genrule( - name = "invoke_failing_tool_using_py3_implicitly", - cmd = "$(location :failing_tool_using_py3_implicitly) > $@", - tools = [":failing_tool_using_py3_implicitly"], - outs = ["failing_tool_using_py3_implicitly.txt"], -) -EOF - cat > test/success.py << EOF -print("Successfully did nothing.") -EOF - cat > test/fail.py << EOF -import sys -sys.exit(1) -EOF - - # Relies on --incompatible_py3_is_default being true (flipped in Bazel 0.25). - - # The warning should be present if the tool - # 1) was built with toolchains enabled, - # 2) in the host config, - # 3) with a mismatched version, or a version set implicitly, - # 4) and it failed during execution. - - # Warning should be present due to explicit version mismatch with host config. - bazel build //test:invoke_failing_tool_using_py2_explicitly \ - --incompatible_use_python_toolchains=true --host_force_python=PY3 \ - &> $TEST_log && fail "bazel build succeeded (expected failure)" - expect_log "it is a Python 2 program that was built in the host \ -configuration, which uses Python 3" - - # Warning should be present due to implicitly set version, even though it - # matches host config. - bazel build //test:invoke_failing_tool_using_py3_implicitly \ - --incompatible_use_python_toolchains=true --host_force_python=PY3 \ - &> $TEST_log && fail "bazel build succeeded (expected failure)" - expect_log "it is running under Python 3 instead of Python 2" - - # No warning if version was set explicitly and matches host config. - bazel build //test:invoke_failing_tool_using_py2_explicitly \ - --incompatible_use_python_toolchains=true --host_force_python=PY2 \ - &> $TEST_log && fail "bazel build succeeded (expected failure)" - expect_not_log "Note: The above Python target's failure" - - # No warning if toolchains are disabled. - bazel build //test:invoke_failing_tool_using_py2_explicitly \ - --incompatible_use_python_toolchains=false --host_force_python=PY3 \ - &> $TEST_log && fail "bazel build succeeded (expected failure)" - expect_not_log "Note: The above Python target's failure" - - # No warning if it succeeded during execution. - bazel build //test:invoke_passing_tool_using_py2_explicitly \ - --incompatible_use_python_toolchains=true --host_force_python=PY3 \ - &> $TEST_log || fail "bazel build failed (expected success)" - expect_not_log "Note: The above Python target's failure" -} - # Regression test for (bazelbuild/continuous-integration#578): Ensure that a # py_binary built with the autodetecting toolchain works when used as a tool # from Starlark rule. In particular, the wrapper script that launches the real