fix: fixes to prepare for making bootstrap=script the default for Linux

.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,
     }