fix: fixes to prepare for making bootstrap=script the default for Linux (#2760)

Various cleanup and prep work to switch bootstrap=script to be the
default.

* Change `bootstrap_impl` to always be disabled for windows. This allows
setting it to
true in a bazelrc without worrying about the target platform. This is
done by using
FeatureFlagInfo to force the value to disabled for windows. This allows
any downstream
usages of the flag to Just Work and not have to add selects() for
windows themselves.
* Switch pip_repository_annotations test to `import python.runfiles`.
The script bootstrap
doesn't add the runfiles root to sys.path, so `import rules_python`
stops working.

* Switch gazelle workspace to using the runtime-env toolchain. It was
previously
implicitly using the deprecated one built into bazel, which doesn't
provide various
  necessary provider fields.
* Make the local toolchain use `sys._base_executable` instead of
`sys.executable`
when finding the interpreter. Otherwise, it might find a venv
interpreter or not
  properly handle wrapper scripts like pyenv.
* Adds a toolchain attribute/field to indicate if the toolchain supports
a build-time
created venv. This is due to the runtime_env toolchain. See PR comments
for details,
but in short: if we don't know the python interpreter path and version
at
  build time, the venv may not properly activate or find site-packages.

If it isn't supported, then the stage1 bootstrap creates a temporary
venv, similar
to how the zip case is handled. Unfortunately, this requires invoking
Python itself
as part of program startup, but I don't see a way around that -- note
this is
  only triggered by the runtime-env toolchain.

* Make the runtime-env toolchain better support virtualenvs. Because
it's a wrapper
that re-invokes Python, Python can't automatically detect its in a venv.
Two
tricks are used (`exec -a` and PYTHONEXECUTABLE) to help address this
(but they
  aren't guaranteed to work, hence the "recreate at runtime" logic).
* Fix a subtle issue where `sys._base_executable` isn't set correctly
due to `home`
missing in the pyvenv.cfg file. This mostly only affected the creation
of venvs
  from within the bazel-created venv.
* Change the bazel site init to always add the build-time created
site-packages
(if it exists) as a site directory. This matches the system_python
bootstrap
behavior a bit better, which just shoved everything onto sys.path using
  PYTHONPATH.
* Skip running runtime_env_toolchains tests on RBE. RBE's system python
is 3.6,
but the script bootstrap uses 3.9 features. (Running it on RBE is
questionable
  anyways).

Along the way...

* Ignore gazelle convenience symlinks
* Switch pip_repository_annotations test to use
non-legacy_external_runfiles based
  paths. The legacy behavior is disabled in Bazel 8+ by default.
* Also document why the script bootstrap doesn't add the runfiles root
to sys.path.

Work towards #2521

---------

Co-authored-by: Ignas Anikevicius <[email protected]>

Author: rickeylev

.bazelignore

@@ -25,6 +25,7 @@ examples/pip_parse/bazel-pip_parse
 examples/pip_parse_vendored/bazel-pip_parse_vendored
 examples/pip_repository_annotations/bazel-pip_repository_annotations
 examples/py_proto_library/bazel-py_proto_library
+gazelle/bazel-gazelle
 tests/integration/compile_pip_requirements/bazel-compile_pip_requirements
 tests/integration/ignore_root_user_error/bazel-ignore_root_user_error
 tests/integration/local_toolchains/bazel-local_toolchains

CHANGELOG.md

@@ -54,13 +54,21 @@ END_UNRELEASED_TEMPLATE
 
 {#v0-0-0-changed}
 ### Changed
-* Nothing changed.
+* (rules) On Windows, {obj}`--bootstrap_impl=system_python` is forced. This
+  allows setting `--bootstrap_impl=script` in bazelrc for mixed-platform
+  environments.
 
 {#v0-0-0-fixed}
 ### Fixed
+
 * (rules) PyInfo provider is now advertised by py_test, py_binary, and py_library;
   this allows aspects using required_providers to function correctly.
   ([#2506](https://github.com/bazel-contrib/rules_python/issues/2506)).
+* Fixes when using {obj}`--bootstrap_impl=script`:
+  * `compile_pip_requirements` now works with it
+  * The `sys._base_executable` value will reflect the underlying interpreter,
+    not venv interpreter.
+  * The {obj}`//python/runtime_env_toolchains:all` toolchain now works with it.
 
 {#v0-0-0-added}
 ### Added

examples/pip_repository_annotations/.bazelrc

@@ -5,4 +5,5 @@ try-import %workspace%/user.bazelrc
 # is in examples/bzlmod as the `whl_mods` feature.
 common --noenable_bzlmod
 common --enable_workspace
+common --legacy_external_runfiles=false
 common --incompatible_python_disallow_native_rules

examples/pip_repository_annotations/pip_repository_annotations_test.py

@@ -21,7 +21,7 @@
 import unittest
 from pathlib import Path
 
-from rules_python.python.runfiles import runfiles
+from python.runfiles import runfiles
 
 
 class PipRepositoryAnnotationsTest(unittest.TestCase):
@@ -34,11 +34,7 @@ def wheel_pkg_dir(self) -> str:
 
     def test_build_content_and_data(self):
         r = runfiles.Create()
-        rpath = r.Rlocation(
-            "pip_repository_annotations_example/external/{}/generated_file.txt".format(
-                self.wheel_pkg_dir()
-            )
-        )
+        rpath = r.Rlocation("{}/generated_file.txt".format(self.wheel_pkg_dir()))
         generated_file = Path(rpath)
         self.assertTrue(generated_file.exists())
 
@@ -47,11 +43,7 @@ def test_build_content_and_data(self):
 
     def test_copy_files(self):
         r = runfiles.Create()
-        rpath = r.Rlocation(
-            "pip_repository_annotations_example/external/{}/copied_content/file.txt".format(
-                self.wheel_pkg_dir()
-            )
-        )
+        rpath = r.Rlocation("{}/copied_content/file.txt".format(self.wheel_pkg_dir()))
         copied_file = Path(rpath)
         self.assertTrue(copied_file.exists())
 
@@ -61,7 +53,7 @@ def test_copy_files(self):
     def test_copy_executables(self):
         r = runfiles.Create()
         rpath = r.Rlocation(
-            "pip_repository_annotations_example/external/{}/copied_content/executable{}".format(
+            "{}/copied_content/executable{}".format(
                 self.wheel_pkg_dir(),
                 ".exe" if platform.system() == "windows" else ".py",
             )
@@ -82,7 +74,7 @@ def test_data_exclude_glob(self):
         current_wheel_version = "0.38.4"
 
         r = runfiles.Create()
-        dist_info_dir = "pip_repository_annotations_example/external/{}/site-packages/wheel-{}.dist-info".format(
+        dist_info_dir = "{}/site-packages/wheel-{}.dist-info".format(
             self.wheel_pkg_dir(),
             current_wheel_version,
         )
@@ -113,11 +105,8 @@ def test_extra(self):
         # This test verifies that annotations work correctly for pip packages with extras
         # specified, in this case requests[security].
         r = runfiles.Create()
-        rpath = r.Rlocation(
-            "pip_repository_annotations_example/external/{}/generated_file.txt".format(
-                self.requests_pkg_dir()
-            )
-        )
+        path = "{}/generated_file.txt".format(self.requests_pkg_dir())
+        rpath = r.Rlocation(path)
         generated_file = Path(rpath)
         self.assertTrue(generated_file.exists())
 

gazelle/WORKSPACE

@@ -42,6 +42,8 @@ load("//:internal_dev_deps.bzl", "internal_dev_deps")
 
 internal_dev_deps()
 
+register_toolchains("@rules_python//python/runtime_env_toolchains:all")
+
 load("//:deps.bzl", _py_gazelle_deps = "gazelle_deps")
 
 # gazelle:repository_macro deps.bzl%go_deps

python/config_settings/BUILD.bazel

@@ -11,6 +11,7 @@ load(
     "PrecompileSourceRetentionFlag",
     "VenvsSitePackages",
     "VenvsUseDeclareSymlinkFlag",
+    rp_string_flag = "string_flag",
 )
 load(
     "//python/private/pypi:flags.bzl",
@@ -87,14 +88,27 @@ string_flag(
     visibility = ["//visibility:public"],
 )
 
-string_flag(
+rp_string_flag(
     name = "bootstrap_impl",
     build_setting_default = BootstrapImplFlag.SYSTEM_PYTHON,
+    override = select({
+        # Windows doesn't yet support bootstrap=script, so force disable it
+        ":_is_windows": BootstrapImplFlag.SYSTEM_PYTHON,
+        "//conditions:default": "",
+    }),
     values = sorted(BootstrapImplFlag.__members__.values()),
     # NOTE: Only public because it's an implicit dependency
     visibility = ["//visibility:public"],
 )
 
+# For some reason, @platforms//os:windows can't be directly used
+# in the select() for the flag. But it can be used when put behind
+# a config_setting().
+config_setting(
+    name = "_is_windows",
+    constraint_values = ["@platforms//os:windows"],
+)
+
 # This is used for pip and hermetic toolchain resolution.
 string_flag(
     name = "py_linux_libc",

python/private/BUILD.bazel

@@ -86,6 +86,7 @@ bzl_library(
     name = "runtime_env_toolchain_bzl",
     srcs = ["runtime_env_toolchain.bzl"],
     deps = [
+        ":config_settings_bzl",
         ":py_exec_tools_toolchain_bzl",
         ":toolchain_types_bzl",
         "//python:py_runtime_bzl",

python/private/config_settings.bzl

@@ -209,3 +209,33 @@ _current_config = rule(
         "_template": attr.string(default = _DEBUG_ENV_MESSAGE_TEMPLATE),
     },
 )
+
+def is_python_version_at_least(name, **kwargs):
+    flag_name = "_{}_flag".format(name)
+    native.config_setting(
+        name = name,
+        flag_values = {
+            flag_name: "yes",
+        },
+    )
+    _python_version_at_least(
+        name = flag_name,
+        visibility = ["//visibility:private"],
+        **kwargs
+    )
+
+def _python_version_at_least_impl(ctx):
+    at_least = tuple(ctx.attr.at_least.split("."))
+    current = tuple(
+        ctx.attr._major_minor[config_common.FeatureFlagInfo].value.split("."),
+    )
+    value = "yes" if current >= at_least else "no"
+    return [config_common.FeatureFlagInfo(value = value)]
+
+_python_version_at_least = rule(
+    implementation = _python_version_at_least_impl,
+    attrs = {
+        "at_least": attr.string(mandatory = True),
+        "_major_minor": attr.label(default = _PYTHON_VERSION_MAJOR_MINOR_FLAG),
+    },
+)

python/private/flags.bzl

@@ -35,8 +35,38 @@ AddSrcsToRunfilesFlag = FlagEnum(
     is_enabled = _AddSrcsToRunfilesFlag_is_enabled,
 )
 
+def _string_flag_impl(ctx):
+    if ctx.attr.override:
+        value = ctx.attr.override
+    else:
+        value = ctx.build_setting_value
+
+    if value not in ctx.attr.values:
+        fail((
+            "Invalid value for {name}: got {value}, must " +
+            "be one of {allowed}"
+        ).format(
+            name = ctx.label,
+            value = value,
+            allowed = ctx.attr.values,
+        ))
+
+    return [
+        BuildSettingInfo(value = value),
+        config_common.FeatureFlagInfo(value = value),
+    ]
+
+string_flag = rule(
+    implementation = _string_flag_impl,
+    build_setting = config.string(flag = True),
+    attrs = {
+        "override": attr.string(),
+        "values": attr.string_list(),
+    },
+)
+
 def _bootstrap_impl_flag_get_value(ctx):
-    return ctx.attr._bootstrap_impl_flag[BuildSettingInfo].value
+    return ctx.attr._bootstrap_impl_flag[config_common.FeatureFlagInfo].value
 
 # buildifier: disable=name-conventions
 BootstrapImplFlag = enum(

python/private/get_local_runtime_info.py

@@ -22,6 +22,7 @@
     "micro": sys.version_info.micro,
     "include": sysconfig.get_path("include"),
     "implementation_name": sys.implementation.name,
+    "base_executable": sys._base_executable,
 }
 
 config_vars = [

python/private/local_runtime_repo.bzl

@@ -84,6 +84,20 @@ def _local_runtime_repo_impl(rctx):
     info = json.decode(exec_result.stdout)
     logger.info(lambda: _format_get_info_result(info))
 
+    # We use base_executable because we want the path within a Python
+    # installation directory ("PYTHONHOME"). The problems with sys.executable
+    # are:
+    # * If we're in an activated venv, then we don't want the venv's
+    #   `bin/python3` path to be used -- it isn't an actual Python installation.
+    # * If sys.executable is a wrapper (e.g. pyenv), then (1) it may not be
+    #   located within an actual Python installation directory, and (2) it
+    #   can interfer with Python recognizing when it's within a venv.
+    #
+    # In some cases, it may be a symlink (usually e.g. `python3->python3.12`),
+    # but we don't realpath() it to respect what it has decided is the
+    # appropriate path.
+    interpreter_path = info["base_executable"]
+
     # NOTE: Keep in sync with recursive glob in define_local_runtime_toolchain_impl
     repo_utils.watch_tree(rctx, rctx.path(info["include"]))
 

python/private/py_executable.bzl

@@ -350,6 +350,7 @@ def _create_executable(
             main_py = main_py,
             imports = imports,
             runtime_details = runtime_details,
+            venv = venv,
         )
         extra_runfiles = ctx.runfiles([stage2_bootstrap] + venv.files_without_interpreter)
         zip_main = _create_zip_main(
@@ -538,11 +539,14 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
     ctx.actions.write(pyvenv_cfg, "")
 
     runtime = runtime_details.effective_runtime
+
     venvs_use_declare_symlink_enabled = (
         VenvsUseDeclareSymlinkFlag.get_value(ctx) == VenvsUseDeclareSymlinkFlag.YES
     )
+    recreate_venv_at_runtime = False
 
-    if not venvs_use_declare_symlink_enabled:
+    if not venvs_use_declare_symlink_enabled or not runtime.supports_build_time_venv:
+        recreate_venv_at_runtime = True
         if runtime.interpreter:
             interpreter_actual_path = runfiles_root_path(ctx, runtime.interpreter.short_path)
         else:
@@ -557,6 +561,8 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
         ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))
 
     elif runtime.interpreter:
+        # Some wrappers around the interpreter (e.g. pyenv) use the program
+        # name to decide what to do, so preserve the name.
         py_exe_basename = paths.basename(runtime.interpreter.short_path)
 
         # Even though ctx.actions.symlink() is used, using
@@ -594,7 +600,8 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
     if "t" in runtime.abi_flags:
         version += "t"
 
-    site_packages = "{}/lib/python{}/site-packages".format(venv, version)
+    venv_site_packages = "lib/python{}/site-packages".format(version)
+    site_packages = "{}/{}".format(venv, venv_site_packages)
     pth = ctx.actions.declare_file("{}/bazel.pth".format(site_packages))
     ctx.actions.write(pth, "import _bazel_site_init\n")
 
@@ -616,10 +623,12 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
 
     return struct(
         interpreter = interpreter,
-        recreate_venv_at_runtime = not venvs_use_declare_symlink_enabled,
+        recreate_venv_at_runtime = recreate_venv_at_runtime,
         # Runfiles root relative path or absolute path
         interpreter_actual_path = interpreter_actual_path,
         files_without_interpreter = [pyvenv_cfg, pth, site_init] + site_packages_symlinks,
+        # string; venv-relative path to the site-packages directory.
+        venv_site_packages = venv_site_packages,
     )
 
 def _create_site_packages_symlinks(ctx, site_packages):
@@ -716,7 +725,8 @@ def _create_stage2_bootstrap(
         output_sibling,
         main_py,
         imports,
-        runtime_details):
+        runtime_details,
+        venv = None):
     output = ctx.actions.declare_file(
         # Prepend with underscore to prevent pytest from trying to
         # process the bootstrap for files starting with `test_`
@@ -731,6 +741,14 @@ def _create_stage2_bootstrap(
         main_py_path = "{}/{}".format(ctx.workspace_name, main_py.short_path)
     else:
         main_py_path = ""
+
+    # The stage2 bootstrap uses the venv site-packages location to fix up issues
+    # that occur when the toolchain doesn't support the build-time venv.
+    if venv and not runtime.supports_build_time_venv:
+        venv_rel_site_packages = venv.venv_site_packages
+    else:
+        venv_rel_site_packages = ""
+
     ctx.actions.expand_template(
         template = template,
         output = output,
@@ -741,6 +759,7 @@ def _create_stage2_bootstrap(
             "%main%": main_py_path,
             "%main_module%": ctx.attr.main_module,
             "%target%": str(ctx.label),
+            "%venv_rel_site_packages%": venv_rel_site_packages,
             "%workspace_name%": ctx.workspace_name,
         },
         is_executable = True,
@@ -766,6 +785,12 @@ def _create_stage1_bootstrap(
 
     python_binary_actual = venv.interpreter_actual_path if venv else ""
 
+    # Runtime may be None on Windows due to the --python_path flag.
+    if runtime and runtime.supports_build_time_venv:
+        resolve_python_binary_at_runtime = "0"
+    else:
+        resolve_python_binary_at_runtime = "1"
+
     subs = {
         "%interpreter_args%": "\n".join([
             '"{}"'.format(v)
@@ -775,7 +800,9 @@ def _create_stage1_bootstrap(
         "%python_binary%": python_binary_path,
         "%python_binary_actual%": python_binary_actual,
         "%recreate_venv_at_runtime%": str(int(venv.recreate_venv_at_runtime)) if venv else "0",
+        "%resolve_python_binary_at_runtime%": resolve_python_binary_at_runtime,
         "%target%": str(ctx.label),
+        "%venv_rel_site_packages%": venv.venv_site_packages if venv else "",
         "%workspace_name%": ctx.workspace_name,
     }