From 5eb139f36793494313aa84429c50588c802f82e3 Mon Sep 17 00:00:00 2001
From: Chowder <16789070+chowder@users.noreply.github.com>
Date: Wed, 4 Dec 2024 04:51:35 +0000
Subject: [PATCH] fix: only delete first sys.path entry in the stage-2
bootstrap if PYTHONSAFEPATH is unset or unsupported (#2418)
Unnconditionally deleting the first `sys.path` entry on the stage-2
bootstrap incorrecly removes a valid search path on Python 3.11 and
above, since `PYTHONSAFEPATH` is already unconditionally set in stage-1.
It should be deleted only if it is unset or unsupported.
Fixes https://github.com/bazelbuild/rules_python/issues/2318
---------
Co-authored-by: Richard Levasseur <richardlev@gmail.com>
Co-authored-by: Richard Levasseur <rlevasseur@google.com>
---
CHANGELOG.md | 2 +
python/private/stage2_bootstrap_template.py | 16 +++++---
tests/bootstrap_impls/BUILD.bazel | 18 +++------
tests/no_unsafe_paths/BUILD.bazel | 33 ++++++++++++++++
tests/no_unsafe_paths/test.py | 44 +++++++++++++++++++++
tests/support/support.bzl | 7 ++++
6 files changed, 102 insertions(+), 18 deletions(-)
create mode 100644 tests/no_unsafe_paths/BUILD.bazel
create mode 100644 tests/no_unsafe_paths/test.py
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1fa047b47d..590a9c795b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -100,6 +100,8 @@ Other changes:
* (repositories): Add libs/python3.lib and pythonXY.dll to the `libpython` target
defined by a repository template. This enables stable ABI builds of Python extensions
on Windows (by defining Py_LIMITED_API).
+* (rules) `py_test` and `py_binary` targets no longer incorrectly remove the
+ first `sys.path` entry when using {obj}`--bootstrap_impl=script`
{#v0-0-0-added}
### Added
diff --git a/python/private/stage2_bootstrap_template.py b/python/private/stage2_bootstrap_template.py
index d2c7497795..1e19a71b64 100644
--- a/python/private/stage2_bootstrap_template.py
+++ b/python/private/stage2_bootstrap_template.py
@@ -4,13 +4,17 @@
import sys
-# The Python interpreter unconditionally prepends the directory containing this
+# By default the Python interpreter prepends the directory containing this
# script (following symlinks) to the import path. This is the cause of #9239,
-# and is a special case of #7091. We therefore explicitly delete that entry.
-# TODO(#7091): Remove this hack when no longer necessary.
-# TODO: Use sys.flags.safe_path to determine whether this removal should be
-# performed
-del sys.path[0]
+# and is a special case of #7091.
+#
+# Python 3.11 introduced an PYTHONSAFEPATH (-P) option that disables this
+# behaviour, which we set in the stage 1 bootstrap.
+# So the prepended entry needs to be removed only if the above option is either
+# unset or not supported by the interpreter.
+# NOTE: This can be removed when Python 3.10 and below is no longer supported
+if not getattr(sys.flags, "safe_path", False):
+ del sys.path[0]
import contextlib
import os
diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel
index 2fb1f38ff0..8e50f34cfa 100644
--- a/tests/bootstrap_impls/BUILD.bazel
+++ b/tests/bootstrap_impls/BUILD.bazel
@@ -11,16 +11,10 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
-load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test", "sh_py_run_test")
+load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT")
load(":venv_relative_path_tests.bzl", "relative_path_test_suite")
-_SUPPORTS_BOOTSTRAP_SCRIPT = select({
- "@platforms//os:windows": ["@platforms//:incompatible"],
- "//conditions:default": [],
-}) if IS_BAZEL_7_OR_HIGHER else ["@platforms//:incompatible"]
-
sh_py_run_test(
name = "run_binary_zip_no_test",
build_python_zip = "no",
@@ -41,7 +35,7 @@ sh_py_run_test(
build_python_zip = "yes",
py_src = "bin.py",
sh_src = "run_binary_zip_yes_test.sh",
- target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
)
sh_py_run_test(
@@ -50,7 +44,7 @@ sh_py_run_test(
build_python_zip = "no",
py_src = "bin.py",
sh_src = "run_binary_zip_no_test.sh",
- target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
)
py_reconfig_test(
@@ -60,7 +54,7 @@ py_reconfig_test(
env = {"BOOTSTRAP": "script"},
imports = ["./USER_IMPORT/site-packages"],
main = "sys_path_order_test.py",
- target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
)
py_reconfig_test(
@@ -77,7 +71,7 @@ sh_py_run_test(
bootstrap_impl = "script",
py_src = "bin.py",
sh_src = "inherit_pythonsafepath_env_test.sh",
- target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
)
sh_py_run_test(
@@ -86,7 +80,7 @@ sh_py_run_test(
imports = ["./MARKER"],
py_src = "call_sys_exe.py",
sh_src = "sys_executable_inherits_sys_path_test.sh",
- target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
)
relative_path_test_suite(name = "relative_path_tests")
diff --git a/tests/no_unsafe_paths/BUILD.bazel b/tests/no_unsafe_paths/BUILD.bazel
new file mode 100644
index 0000000000..f12d1c9a70
--- /dev/null
+++ b/tests/no_unsafe_paths/BUILD.bazel
@@ -0,0 +1,33 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test")
+load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT")
+
+py_reconfig_test(
+ name = "no_unsafe_paths_3.10_test",
+ srcs = ["test.py"],
+ bootstrap_impl = "script",
+ main = "test.py",
+ python_version = "3.10",
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+)
+
+py_reconfig_test(
+ name = "no_unsafe_paths_3.11_test",
+ srcs = ["test.py"],
+ bootstrap_impl = "script",
+ main = "test.py",
+ python_version = "3.11",
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+)
diff --git a/tests/no_unsafe_paths/test.py b/tests/no_unsafe_paths/test.py
new file mode 100644
index 0000000000..1f6cd4e569
--- /dev/null
+++ b/tests/no_unsafe_paths/test.py
@@ -0,0 +1,44 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+import sys
+import unittest
+
+
+class NoUnsafePathsTest(unittest.TestCase):
+ def test_no_unsafe_paths_in_search_path(self):
+ # Based on sys.path documentation, the first item added is the zip
+ # archive
+ # (see: https://docs.python.org/3/library/sys_path_init.html)
+ #
+ # We can use this as a marker to verify that during bootstrapping,
+ # (1) no unexpected paths were prepended, and (2) no paths were
+ # accidentally dropped.
+ #
+ major, minor, *_ = sys.version_info
+ archive = f"python{major}{minor}.zip"
+
+ # < Python 3.11 behaviour
+ if (major, minor) < (3, 11):
+ # Because of https://github.com/bazelbuild/rules_python/blob/0.39.0/python/private/stage2_bootstrap_template.py#L415-L436
+ self.assertEqual(os.path.dirname(sys.argv[0]), sys.path[0])
+ self.assertEqual(os.path.basename(sys.path[1]), archive)
+ # >= Python 3.11 behaviour
+ else:
+ self.assertEqual(os.path.basename(sys.path[0]), archive)
+
+
+if __name__ == '__main__':
+ unittest.main()
\ No newline at end of file
diff --git a/tests/support/support.bzl b/tests/support/support.bzl
index 7358a6b1ee..2b6703843b 100644
--- a/tests/support/support.bzl
+++ b/tests/support/support.bzl
@@ -19,6 +19,8 @@
# rules_testing or as config_setting values, which don't support Label in some
# places.
+load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
+
MAC = Label("//tests/support:mac")
MAC_X86_64 = Label("//tests/support:mac_x86_64")
LINUX = Label("//tests/support:linux")
@@ -39,3 +41,8 @@ PRECOMPILE_SOURCE_RETENTION = str(Label("//python/config_settings:precompile_sou
PYC_COLLECTION = str(Label("//python/config_settings:pyc_collection"))
PYTHON_VERSION = str(Label("//python/config_settings:python_version"))
VISIBLE_FOR_TESTING = str(Label("//python/private:visible_for_testing"))
+
+SUPPORTS_BOOTSTRAP_SCRIPT = select({
+ "@platforms//os:windows": ["@platforms//:incompatible"],
+ "//conditions:default": [],
+}) if IS_BAZEL_7_OR_HIGHER else ["@platforms//:incompatible"]