Skip to content

Commit

Permalink
fix(toolchain): disable exec toolchain by default (#1968)
Browse files Browse the repository at this point in the history
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.

---------

Co-authored-by: Richard Levasseur <rlevasseur@google.com>
aignas and rickeylev authored Jun 19, 2024
1 parent d7e07fc commit cf1f36d
Showing 10 changed files with 83 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -30,6 +30,11 @@ A brief description of the categories of changes:
reduce the total number of targets in the hub repo.

### 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).
* (bzlmod): Targets in `all_requirements` now use the same form as targets returned by the `requirement` macro.
* (rules) Auto exec groups are enabled. This allows actions run by the rules,
such as precompiling, to pick an execution platform separately from what
26 changes: 25 additions & 1 deletion docs/sphinx/api/python/config_settings/index.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion docs/sphinx/toolchains.md
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
load(
"//python/private:flags.bzl",
"BootstrapImplFlag",
"ExecToolsToolchainFlag",
"PrecompileAddToRunfilesFlag",
"PrecompileFlag",
"PrecompileSourceRetentionFlag",
@@ -28,6 +29,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,
1 change: 1 addition & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
3 changes: 2 additions & 1 deletion python/private/autodetecting_toolchain.bzl
Original file line number Diff line number Diff line change
@@ -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"],
)
9 changes: 9 additions & 0 deletions python/private/flags.bzl
Original file line number Diff line number Diff line change
@@ -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
14 changes: 12 additions & 2 deletions python/private/py_toolchain_suite.bzl
Original file line number Diff line number Diff line change
@@ -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"),
)

7 changes: 7 additions & 0 deletions tests/base_rules/precompile/precompile_tests.bzl
Original file line number Diff line number Diff line change
@@ -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",
},
)

1 change: 1 addition & 0 deletions tests/support/support.bzl
Original file line number Diff line number Diff line change
@@ -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"))

0 comments on commit cf1f36d

Please sign in to comment.