fix: only delete first sys.path entry in the stage-2 bootstrap if PYT…

…HONSAFEPATH 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 #2318 --------- Co-authored-by: Richard Levasseur <[email protected]> Co-authored-by: Richard Levasseur <[email protected]>
bazelbuild · Dec 4, 2024 · 5eb139f · 5eb139f
1 parent 096a04f
commit 5eb139f
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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()
diff --git 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"]