From 5cd59eb9b9644a5ca797aebfa08ba6e4ef94d5cc Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 19 Jun 2024 13:08:58 +0900 Subject: [PATCH] fix(toolchain): disable exec toolchain by default (#1968) This makes the exec tools toolchain disabled by default to prevent toolchain resolution from matching it and inadvertently pulling in a dependency on the hermetic runtimes. While the hermetic runtime wouldn't actually be used (precompiling is disabled by default), the dependency triggered downloading of the runtimes, which breaks environments which forbid remote downloads they haven't vetted (such a case is Bazel's own build process). To fix this, a flag is added to control if the exec tools toolchain is enabled or not. When disabled (the default), the toolchain won't match, and the remote dependency isn't triggered. Fixes #1967. Cherry-pick of cf1f36dd. --------- Co-authored-by: Richard Levasseur --- CHANGELOG.md | 11 ++++++++ .../api/python/config_settings/index.md | 26 ++++++++++++++++++- docs/sphinx/toolchains.md | 2 +- python/config_settings/BUILD.bazel | 20 ++++++++++++++ python/private/BUILD.bazel | 1 + python/private/autodetecting_toolchain.bzl | 3 ++- python/private/flags.bzl | 9 +++++++ python/private/py_toolchain_suite.bzl | 14 ++++++++-- .../precompile/precompile_tests.bzl | 7 +++++ tests/support/support.bzl | 1 + 10 files changed, 89 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9abbd44de0..28ab14dae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,17 @@ A brief description of the categories of changes: ### Removed * Nothing yet +## [0.33.2] - 2024-06-13 + +[0.33.2]: https://github.com/bazelbuild/rules_python/releases/tag/0.33.2 + +### Fixed +* (toolchains) The {obj}`exec_tools_toolchain_type` is disabled by default. + To enable it, set {obj}`--//python/config_settings:exec_tools_toolchain=enabled`. + This toolchain must be enabled for precompilation to work. This toolchain will + be enabled by default in a future release. + Fixes [1967](https://github.com/bazelbuild/rules_python/issues/1967). + ## [0.33.1] - 2024-06-13 [0.33.1]: https://github.com/bazelbuild/rules_python/releases/tag/0.33.1 diff --git a/docs/sphinx/api/python/config_settings/index.md b/docs/sphinx/api/python/config_settings/index.md index 8c23bf6855..50647abb8d 100644 --- a/docs/sphinx/api/python/config_settings/index.md +++ b/docs/sphinx/api/python/config_settings/index.md @@ -10,6 +10,22 @@ Determines the default hermetic Python toolchain version. This can be set to one of the values that `rules_python` maintains. ::: +::::{bzl:flag} exec_tools_toolchain +Determines if the {obj}`exec_tools_toolchain_type` toolchain is enabled. + +:::{note} +* Note that this only affects the rules_python generated toolchains. +::: + +Values: + +* `enabled`: Allow matching of the registered toolchains at build time. +* `disabled`: Prevent the toolchain from being matched at build time. + +:::{versionadded} 0.33.2 +::: +:::: + ::::{bzl:flag} precompile Determines if Python source files should be compiled at build time. @@ -34,6 +50,8 @@ Values: * `force_disabled`: Like `disabled`, except overrides target-level setting. This is useful useful for development, testing enabling precompilation more broadly, or as an escape hatch if build-time compiling is not available. +:::{versionadded} 0.33.0 +::: :::: ::::{bzl:flag} precompile_source_retention @@ -51,9 +69,11 @@ Values: * `omit_source`: Don't include the orignal py source. * `omit_if_generated_source`: Keep the original source if it's a regular source file, but omit it if it's a generated file. +:::{versionadded} 0.33.0 +::: :::: -:::{bzl:flag} precompile_add_to_runfiles +::::{bzl:flag} precompile_add_to_runfiles Determines if a target adds its compiled files to its runfiles. When a target compiles its files, but doesn't add them to its own runfiles, it @@ -66,7 +86,9 @@ Values: runfiles; they are still added to {bzl:obj}`PyInfo.transitive_pyc_files`. See also: {bzl:obj}`py_binary.pyc_collection` attribute. This is useful for allowing incrementally enabling precompilation on a per-binary basis. +:::{versionadded} 0.33.0 ::: +:::: ::::{bzl:flag} pyc_collection Determine if `py_binary` collects transitive pyc files. @@ -78,6 +100,8 @@ This flag is overridden by the target level `pyc_collection` attribute. Values: * `include_pyc`: Include `PyInfo.transitive_pyc_files` as part of the binary. * `disabled`: Don't include `PyInfo.transitive_pyc_files` as part of the binary. +:::{versionadded} 0.33.0 +::: :::: ::::{bzl:flag} py_linux_libc diff --git a/docs/sphinx/toolchains.md b/docs/sphinx/toolchains.md index e3be22f97b..26557cabed 100644 --- a/docs/sphinx/toolchains.md +++ b/docs/sphinx/toolchains.md @@ -233,7 +233,7 @@ use `python3` from the environment a binary runs within. This provides extremely limited functionality to the rules (at build time, nothing is knowable about the Python runtime). -Bazel itself automatically registers `@bazel_tools//python:autodetecting_toolchain` +Bazel itself automatically registers `@bazel_tools//tools/python:autodetecting_toolchain` as the lowest priority toolchain. For WORKSPACE builds, if no other toolchain is registered, that toolchain will be used. For bzlmod builds, rules_python automatically registers a higher-priority toolchain; it won't be used unless diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index e2d2608c7c..cacfb818e1 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -2,6 +2,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load( "//python/private:flags.bzl", "BootstrapImplFlag", + "ExecToolsToolchainFlag", "PrecompileAddToRunfilesFlag", "PrecompileFlag", "PrecompileSourceRetentionFlag", @@ -29,6 +30,25 @@ construct_config_settings( name = "construct_config_settings", ) +string_flag( + name = "exec_tools_toolchain", + build_setting_default = ExecToolsToolchainFlag.DISABLED, + values = sorted(ExecToolsToolchainFlag.__members__.values()), + # NOTE: Only public because it is used in py_toolchain_suite from toolchain + # repositories + visibility = ["//visibility:private"], +) + +config_setting( + name = "is_exec_tools_toolchain_enabled", + flag_values = { + "exec_tools_toolchain": ExecToolsToolchainFlag.ENABLED, + }, + # NOTE: Only public because it is used in py_toolchain_suite from toolchain + # repositories + visibility = ["//visibility:public"], +) + string_flag( name = "precompile", build_setting_default = PrecompileFlag.AUTO, diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 422ed9c7c2..e2a2bc01a2 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -59,6 +59,7 @@ bzl_library( name = "autodetecting_toolchain_bzl", srcs = ["autodetecting_toolchain.bzl"], deps = [ + ":toolchain_types_bzl", "//python:py_runtime_bzl", "//python:py_runtime_pair_bzl", ], diff --git a/python/private/autodetecting_toolchain.bzl b/python/private/autodetecting_toolchain.bzl index 55c95699c9..174136e870 100644 --- a/python/private/autodetecting_toolchain.bzl +++ b/python/private/autodetecting_toolchain.bzl @@ -16,6 +16,7 @@ load("//python:py_runtime.bzl", "py_runtime") load("//python:py_runtime_pair.bzl", "py_runtime_pair") +load(":toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def define_autodetecting_toolchain(name): """Defines the autodetecting Python toolchain. @@ -65,6 +66,6 @@ def define_autodetecting_toolchain(name): native.toolchain( name = name, toolchain = ":_autodetecting_py_runtime_pair", - toolchain_type = ":toolchain_type", + toolchain_type = TARGET_TOOLCHAIN_TYPE, visibility = ["//visibility:public"], ) diff --git a/python/private/flags.bzl b/python/private/flags.bzl index d141f72eee..fa31262c94 100644 --- a/python/private/flags.bzl +++ b/python/private/flags.bzl @@ -37,6 +37,15 @@ def _precompile_flag_get_effective_value(ctx): value = PrecompileFlag.DISABLED return value +# Determines if the Python exec tools toolchain should be registered. +# buildifier: disable=name-conventions +ExecToolsToolchainFlag = enum( + # Enable registering the exec tools toolchain using the hermetic toolchain. + ENABLED = "enabled", + # Disable registering the exec tools toolchain using the hermetic toolchain. + DISABLED = "disabled", +) + # Determines if Python source files should be compiled at build time. # # NOTE: The flag value is overridden by the target-level attribute, except diff --git a/python/private/py_toolchain_suite.bzl b/python/private/py_toolchain_suite.bzl index 174c36f782..ed562c97b4 100644 --- a/python/private/py_toolchain_suite.bzl +++ b/python/private/py_toolchain_suite.bzl @@ -22,6 +22,8 @@ load( "TARGET_TOOLCHAIN_TYPE", ) +_IS_EXEC_TOOLCHAIN_ENABLED = Label("//python/config_settings:is_exec_tools_toolchain_enabled") + def py_toolchain_suite(*, prefix, user_repository_name, python_version, set_python_version_constraint, flag_values, **kwargs): """For internal use only. @@ -106,8 +108,16 @@ def py_toolchain_suite(*, prefix, user_repository_name, python_version, set_pyth user_repository_name = user_repository_name, ), toolchain_type = EXEC_TOOLS_TOOLCHAIN_TYPE, - # The target settings capture the Python version - target_settings = target_settings, + target_settings = select({ + _IS_EXEC_TOOLCHAIN_ENABLED: target_settings, + # Whatever the default is, it has to map to a `config_setting` + # that will never match. Since the default branch is only taken if + # _IS_EXEC_TOOLCHAIN_ENABLED is false, then it will never match + # when later evaluated during toolchain resolution. + # Note that @platforms//:incompatible can't be used here because + # the RHS must be a `config_setting`. + "//conditions:default": [_IS_EXEC_TOOLCHAIN_ENABLED], + }), exec_compatible_with = kwargs.get("target_compatible_with"), ) diff --git a/tests/base_rules/precompile/precompile_tests.bzl b/tests/base_rules/precompile/precompile_tests.bzl index 02ab4ab19c..58bdafe39c 100644 --- a/tests/base_rules/precompile/precompile_tests.bzl +++ b/tests/base_rules/precompile/precompile_tests.bzl @@ -27,6 +27,7 @@ load("//tests/base_rules:py_info_subject.bzl", "py_info_subject") load( "//tests/support:support.bzl", "CC_TOOLCHAIN", + "EXEC_TOOLS_TOOLCHAIN", "PLATFORM_TOOLCHAIN", "PRECOMPILE", "PRECOMPILE_ADD_TO_RUNFILES", @@ -61,6 +62,7 @@ def _test_precompile_enabled_setup(name, py_rule, **kwargs): target = name + "_subject", config_settings = { "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, + EXEC_TOOLS_TOOLCHAIN: "enabled", }, ) @@ -119,6 +121,7 @@ def _test_pyc_only(name): config_settings = { "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, ##PRECOMPILE_SOURCE_RETENTION: "omit_source", + EXEC_TOOLS_TOOLCHAIN: "enabled", }, target = name + "_subject", ) @@ -161,6 +164,7 @@ def _test_precompile_if_generated(name): target = name + "_subject", config_settings = { "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, + EXEC_TOOLS_TOOLCHAIN: "enabled", }, ) @@ -203,6 +207,7 @@ def _test_omit_source_if_generated_source(name): config_settings = { "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, PRECOMPILE_SOURCE_RETENTION: "omit_if_generated_source", + EXEC_TOOLS_TOOLCHAIN: "enabled", }, ) @@ -252,6 +257,7 @@ def _test_precompile_add_to_runfiles_decided_elsewhere(name): "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, PRECOMPILE_ADD_TO_RUNFILES: "decided_elsewhere", PRECOMPILE: "enabled", + EXEC_TOOLS_TOOLCHAIN: "enabled", }, ) @@ -288,6 +294,7 @@ def _test_precompiler_action(name): target = name + "_subject", config_settings = { "//command_line_option:extra_toolchains": _TEST_TOOLCHAINS, + EXEC_TOOLS_TOOLCHAIN: "enabled", }, ) diff --git a/tests/support/support.bzl b/tests/support/support.bzl index 4bcc554854..efcc43a54f 100644 --- a/tests/support/support.bzl +++ b/tests/support/support.bzl @@ -31,6 +31,7 @@ CC_TOOLCHAIN = str(Label("//tests/cc:all")) # str() around Label() is necessary because rules_testing's config_settings # doesn't accept yet Label objects. +EXEC_TOOLS_TOOLCHAIN = str(Label("//python/config_settings:exec_tools_toolchain")) PRECOMPILE = str(Label("//python/config_settings:precompile")) PYC_COLLECTION = str(Label("//python/config_settings:pyc_collection")) PRECOMPILE_SOURCE_RETENTION = str(Label("//python/config_settings:precompile_source_retention"))