From e99bc2dad0e4a5f5e3302ee845afc3b7249a183e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 4 May 2025 18:02:54 -0700 Subject: [PATCH 01/12] feat: default to bootstrap script for non-windows --- CHANGELOG.md | 1 + docs/api/rules_python/python/config_settings/index.md | 9 +++++++++ python/config_settings/BUILD.bazel | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9cb14459d..25320d1050 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ END_UNRELEASED_TEMPLATE {#v0-0-0-changed} ### Changed +* (rules) {obj}`--bootstrap_impl=script` is the default for non-Windows. * (rules) On Windows, {obj}`--bootstrap_impl=system_python` is forced. This allows setting `--bootstrap_impl=script` in bazelrc for mixed-platform environments. diff --git a/docs/api/rules_python/python/config_settings/index.md b/docs/api/rules_python/python/config_settings/index.md index ed6444298e..0784786c8a 100644 --- a/docs/api/rules_python/python/config_settings/index.md +++ b/docs/api/rules_python/python/config_settings/index.md @@ -233,6 +233,10 @@ Values: ::::{bzl:flag} bootstrap_impl Determine how programs implement their startup process. +The default for this depends on the platform: +* Windows: `system_python` (**always** used) +* Other: `script` + Values: * `system_python`: Use a bootstrap that requires a system Python available in order to start programs. This requires @@ -257,6 +261,11 @@ instead. :::{versionadded} 0.33.0 ::: +:::{versionchanged} VERSION_NEXT_FEATURE +* The default for non-Windows changed from `system_python` to `script`. +* On Windows, the value is forced to `system_python`. +::: + :::: ::::{bzl:flag} current_config diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 872d7d1bda..32d125543d 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -90,7 +90,7 @@ string_flag( rp_string_flag( name = "bootstrap_impl", - build_setting_default = BootstrapImplFlag.SYSTEM_PYTHON, + build_setting_default = BootstrapImplFlag.SCRIPT, override = select({ # Windows doesn't yet support bootstrap=script, so force disable it ":_is_windows": BootstrapImplFlag.SYSTEM_PYTHON, From 4f81501dfd86f2d86d0a0773ce2c658e1b4a2c01 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 5 May 2025 16:24:57 -0700 Subject: [PATCH 02/12] convert strs to ints for comparison --- python/private/config_settings.bzl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 2cf7968061..e87f3d035e 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -225,10 +225,12 @@ def is_python_version_at_least(name, **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("."), - ) + at_least = tuple([int(x) for x in ctx.attr.at_least.split(".")]) + current = tuple([ + int(x) + for x in ctx.attr._major_minor[config_common.FeatureFlagInfo].value.split(".") + ]) + print(current, at_least) value = "yes" if current >= at_least else "no" return [config_common.FeatureFlagInfo(value = value)] From 4cf24e0f91fd780b16d7d3f35a348015b7771930 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 5 May 2025 16:30:24 -0700 Subject: [PATCH 03/12] remove debug code --- python/private/config_settings.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index e87f3d035e..0af8e6f902 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -230,7 +230,6 @@ def _python_version_at_least_impl(ctx): int(x) for x in ctx.attr._major_minor[config_common.FeatureFlagInfo].value.split(".") ]) - print(current, at_least) value = "yes" if current >= at_least else "no" return [config_common.FeatureFlagInfo(value = value)] From 2d021be0d4fdb92baa455adc39e6a4b9d585f489 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 5 May 2025 16:40:42 -0700 Subject: [PATCH 04/12] add some debug code --- python/private/runtime_env_toolchain_interpreter.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/private/runtime_env_toolchain_interpreter.sh b/python/private/runtime_env_toolchain_interpreter.sh index 6159d4f38c..7b3ec598b2 100755 --- a/python/private/runtime_env_toolchain_interpreter.sh +++ b/python/private/runtime_env_toolchain_interpreter.sh @@ -68,6 +68,9 @@ if [ -e "$self_dir/pyvenv.cfg" ] || [ -e "$self_dir/../pyvenv.cfg" ]; then ;; esac + if [ ! -e "$PYTHON_BIN" ]; then + die "ERROR: Python interpreter does not exist: $PYTHON_BIN" + fi # PYTHONEXECUTABLE is also used because `exec -a` doesn't fully trick the # pyenv wrappers. # NOTE: The PYTHONEXECUTABLE envvar only works for non-Mac starting in Python 3.11 From 50b37025215b49961869fece0a5b3cf9d87aaf4b Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 5 May 2025 21:35:19 -0700 Subject: [PATCH 05/12] wire in version to runtime env test --- MODULE.bazel | 7 ++++- python/private/internal_dev_deps.bzl | 2 ++ python/private/runtime_env_repo.bzl | 35 +++++++++++++++++++++++++ tests/runtime_env_toolchain/BUILD.bazel | 4 +++ 4 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 python/private/runtime_env_repo.bzl diff --git a/MODULE.bazel b/MODULE.bazel index c649896344..289ce9dfbc 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -98,7 +98,12 @@ internal_dev_deps = use_extension( "internal_dev_deps", dev_dependency = True, ) -use_repo(internal_dev_deps, "buildkite_config", "wheel_for_testing") +use_repo( + internal_dev_deps, + "buildkite_config", + "rules_python_runtime_env", + "wheel_for_testing", +) # Add gazelle plugin so that we can run the gazelle example as an e2e integration # test and include the distribution files. diff --git a/python/private/internal_dev_deps.bzl b/python/private/internal_dev_deps.bzl index 2a3b84e7df..648e096ef8 100644 --- a/python/private/internal_dev_deps.bzl +++ b/python/private/internal_dev_deps.bzl @@ -15,6 +15,7 @@ load("@bazel_ci_rules//:rbe_repo.bzl", "rbe_preconfig") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file") +load(":runtime_env_repo.bzl", "runtime_env_repo") def _internal_dev_deps_impl(mctx): _ = mctx # @unused @@ -37,6 +38,7 @@ def _internal_dev_deps_impl(mctx): name = "buildkite_config", toolchain = "ubuntu1804-bazel-java11", ) + runtime_env_repo(name = "rules_python_runtime_env") internal_dev_deps = module_extension( implementation = _internal_dev_deps_impl, diff --git a/python/private/runtime_env_repo.bzl b/python/private/runtime_env_repo.bzl new file mode 100644 index 0000000000..209a019d13 --- /dev/null +++ b/python/private/runtime_env_repo.bzl @@ -0,0 +1,35 @@ +"""Internal setup to help the runtime_env toolchain.""" + +load("//python/private:repo_utils.bzl", "repo_utils") + +def _runtime_env_repo_impl(rctx): + pyenv = repo_utils.which_unchecked(rctx, "pyenv").binary + if pyenv != None: + pyenv_version_file = repo_utils.execute_checked( + rctx, + op = "GetPyenvVersionFile", + arguments = [pyenv, "version-file"], + ).stdout.strip() + rctx.watch(pyenv_version_file) + + version = repo_utils.execute_checked( + rctx, + op = "GetPythonVersion", + arguments = [ + "python3", + "-I", + "-c", + """import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")""", + ], + environment = { + # Prevent the user's current shell from influencing the result. + # This envvar won't be present when a test is run. + "PYENV_VERSION": None, + }, + ).stdout.strip() + rctx.file("info.bzl", "PYTHON_VERSION = '{}'\n".format(version)) + rctx.file("BUILD.bazel", "") + +runtime_env_repo = repository_rule( + implementation = _runtime_env_repo_impl, +) diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel index 59ca93ba49..0cf2c91774 100644 --- a/tests/runtime_env_toolchain/BUILD.bazel +++ b/tests/runtime_env_toolchain/BUILD.bazel @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@rules_python_runtime_env//:info.bzl", "PYTHON_VERSION") load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test") load("//tests/support:support.bzl", "CC_TOOLCHAIN") load(":runtime_env_toolchain_tests.bzl", "runtime_env_toolchain_test_suite") @@ -30,6 +31,9 @@ py_reconfig_test( CC_TOOLCHAIN, ], main = "toolchain_runs_test.py", + # With bootstrap=script, the build version must match the runtime version + # because the venv has the version in the lib/site-packages dir name. + python_version = PYTHON_VERSION, # Our RBE has Python 3.6, which is too old for the language features # we use now. Using the runtime-env toolchain on RBE is pretty # questionable anyways. From ddd1c21a3f779153537b49a88bd574e4aa70996f Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 09:33:09 -0700 Subject: [PATCH 06/12] make workspace happy; rename internal repo --- MODULE.bazel | 2 +- internal_dev_setup.bzl | 3 +++ python/private/internal_dev_deps.bzl | 2 +- tests/runtime_env_toolchain/BUILD.bazel | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 289ce9dfbc..d0f7cc4afa 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -101,7 +101,7 @@ internal_dev_deps = use_extension( use_repo( internal_dev_deps, "buildkite_config", - "rules_python_runtime_env", + "rules_python_runtime_env_tc_info", "wheel_for_testing", ) diff --git a/internal_dev_setup.bzl b/internal_dev_setup.bzl index fc38e3f9c5..6c5eeee037 100644 --- a/internal_dev_setup.bzl +++ b/internal_dev_setup.bzl @@ -24,6 +24,7 @@ load("@rules_shell//shell:repositories.bzl", "rules_shell_dependencies", "rules_ load("//:version.bzl", "SUPPORTED_BAZEL_VERSIONS") load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") load("//python/private:pythons_hub.bzl", "hub_repo") # buildifier: disable=bzl-visibility +load("//python/private:runtime_env_repo.bzl", "runtime_env_repo") load("//python/private/pypi:deps.bzl", "pypi_deps") # buildifier: disable=bzl-visibility def rules_python_internal_setup(): @@ -40,6 +41,8 @@ def rules_python_internal_setup(): python_versions = sorted(TOOL_VERSIONS.keys()), ) + runtime_env_repo(name = "rules_python_runtime_env_tc_info") + pypi_deps() bazel_skylib_workspace() diff --git a/python/private/internal_dev_deps.bzl b/python/private/internal_dev_deps.bzl index 648e096ef8..4f2cca0b42 100644 --- a/python/private/internal_dev_deps.bzl +++ b/python/private/internal_dev_deps.bzl @@ -38,7 +38,7 @@ def _internal_dev_deps_impl(mctx): name = "buildkite_config", toolchain = "ubuntu1804-bazel-java11", ) - runtime_env_repo(name = "rules_python_runtime_env") + runtime_env_repo(name = "rules_python_runtime_env_tc_info") internal_dev_deps = module_extension( implementation = _internal_dev_deps_impl, diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel index 0cf2c91774..ad2bd4eeb5 100644 --- a/tests/runtime_env_toolchain/BUILD.bazel +++ b/tests/runtime_env_toolchain/BUILD.bazel @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_python_runtime_env//:info.bzl", "PYTHON_VERSION") +load("@rules_python_runtime_env_tc_info//:info.bzl", "PYTHON_VERSION") load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test") load("//tests/support:support.bzl", "CC_TOOLCHAIN") load(":runtime_env_toolchain_tests.bzl", "runtime_env_toolchain_test_suite") From c81a0203234fd05b0a93a3a997b08cddfa025340 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 09:39:23 -0700 Subject: [PATCH 07/12] fix buildifier lint --- internal_dev_setup.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal_dev_setup.bzl b/internal_dev_setup.bzl index 6c5eeee037..f33908049f 100644 --- a/internal_dev_setup.bzl +++ b/internal_dev_setup.bzl @@ -24,7 +24,7 @@ load("@rules_shell//shell:repositories.bzl", "rules_shell_dependencies", "rules_ load("//:version.bzl", "SUPPORTED_BAZEL_VERSIONS") load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") load("//python/private:pythons_hub.bzl", "hub_repo") # buildifier: disable=bzl-visibility -load("//python/private:runtime_env_repo.bzl", "runtime_env_repo") +load("//python/private:runtime_env_repo.bzl", "runtime_env_repo") # buildifier: disable=bzl-visibility load("//python/private/pypi:deps.bzl", "pypi_deps") # buildifier: disable=bzl-visibility def rules_python_internal_setup(): From 7982332cd3a9bd49aa60b357d94cd90169376051 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 10:23:19 -0700 Subject: [PATCH 08/12] use empty string for repo env due to bazel 7 --- python/private/runtime_env_repo.bzl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/python/private/runtime_env_repo.bzl b/python/private/runtime_env_repo.bzl index 209a019d13..cade1968bb 100644 --- a/python/private/runtime_env_repo.bzl +++ b/python/private/runtime_env_repo.bzl @@ -10,6 +10,10 @@ def _runtime_env_repo_impl(rctx): op = "GetPyenvVersionFile", arguments = [pyenv, "version-file"], ).stdout.strip() + + # When pyenv is used, the version file is what decided the + # version used. Watch it so we compute the correct value if the + # user changes it. rctx.watch(pyenv_version_file) version = repo_utils.execute_checked( @@ -24,7 +28,9 @@ def _runtime_env_repo_impl(rctx): environment = { # Prevent the user's current shell from influencing the result. # This envvar won't be present when a test is run. - "PYENV_VERSION": None, + # NOTE: This should be None, but Bazel 7 doesn't support None + # values. Thankfully, pyenv treats empty string the same as missing. + "PYENV_VERSION": "", }, ).stdout.strip() rctx.file("info.bzl", "PYTHON_VERSION = '{}'\n".format(version)) From b889e16619bbfef5b72e7a4ce746e1330914d9eb Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 10:45:45 -0700 Subject: [PATCH 09/12] ignore empty flag value --- python/private/config_settings.bzl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 0af8e6f902..c85a1a8660 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -225,11 +225,19 @@ def is_python_version_at_least(name, **kwargs): ) def _python_version_at_least_impl(ctx): - at_least = tuple([int(x) for x in ctx.attr.at_least.split(".")]) + flag_value = ctx.attr._major_minor[config_common.FeatureFlagInfo].value + + # CI is, somehow, getting an empty string for the current flag value. + # How isn't clear. + if not flag_value: + return "no" + current = tuple([ int(x) - for x in ctx.attr._major_minor[config_common.FeatureFlagInfo].value.split(".") + for x in flag_value.split(".") ]) + at_least = tuple([int(x) for x in ctx.attr.at_least.split(".")]) + value = "yes" if current >= at_least else "no" return [config_common.FeatureFlagInfo(value = value)] From 297f76ed695e05377b932e495d7221d56fbc4218 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 10:47:46 -0700 Subject: [PATCH 10/12] fix return value --- python/private/config_settings.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index c85a1a8660..1685195b78 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -230,7 +230,7 @@ def _python_version_at_least_impl(ctx): # CI is, somehow, getting an empty string for the current flag value. # How isn't clear. if not flag_value: - return "no" + return [config_common.FeatureFlagInfo(value = "no")] current = tuple([ int(x) From 8ce45f14b942bdd4603f8b42a1102b74f3f3b169 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 11:21:03 -0700 Subject: [PATCH 11/12] add helpful note in changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6ca2c8fd..63f8599747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,11 @@ END_UNRELEASED_TEMPLATE {#v0-0-0-changed} ### Changed +* If using the autodetecting/runtime_env toolchain, then the Python version + specified at build-time *must* match the Python version used at runtime (the + {obj}`--@rules_python//python/config_settings:python_version` flag controls the + build-time version). (Such a misconfiguration was unlikely to work to begin + with; this is called out as an FYI). * (rules) {obj}`--bootstrap_impl=script` is the default for non-Windows. * (rules) On Windows, {obj}`--bootstrap_impl=system_python` is forced. This allows setting `--bootstrap_impl=script` in bazelrc for mixed-platform From 098a39a7a6bcdff7b63784107b240be088a9b276 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 11:26:17 -0700 Subject: [PATCH 12/12] clarify changelog --- CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63f8599747..8fdb7edd6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,11 +55,13 @@ END_UNRELEASED_TEMPLATE {#v0-0-0-changed} ### Changed -* If using the autodetecting/runtime_env toolchain, then the Python version - specified at build-time *must* match the Python version used at runtime (the - {obj}`--@rules_python//python/config_settings:python_version` flag controls the - build-time version). (Such a misconfiguration was unlikely to work to begin - with; this is called out as an FYI). +* If using the (deprecated) autodetecting/runtime_env toolchain, then the Python + version specified at build-time *must* match the Python version used at + runtime (the {obj}`--@rules_python//python/config_settings:python_version` + flag and the {attr}`python_version` attribute control the build-time version + for a target). If they don't match, dependencies won't be importable. (Such a + misconfiguration was unlikely to work to begin with; this is called out as an + FYI). * (rules) {obj}`--bootstrap_impl=script` is the default for non-Windows. * (rules) On Windows, {obj}`--bootstrap_impl=system_python` is forced. This allows setting `--bootstrap_impl=script` in bazelrc for mixed-platform