Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(toolchain): disable exec toolchain by default #1968

Merged
merged 17 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7db60b1
fix(toolchain): do not force users to depend on optional toolchains
aignas Jun 14, 2024
c914a9e
fixup! fix(toolchain): do not force users to depend on optional toolc…
aignas Jun 14, 2024
2fd24d7
fixup! fixup! fix(toolchain): do not force users to depend on optiona…
aignas Jun 14, 2024
f6a37f9
include auto in the precompile, that way users at least have a way to…
aignas Jun 14, 2024
6d11c19
fixup! include auto in the precompile, that way users at least have a…
aignas Jun 14, 2024
34add9c
Revert "fixup! include auto in the precompile, that way users at leas…
aignas Jun 15, 2024
465eac4
Revert "include auto in the precompile, that way users at least have …
aignas Jun 15, 2024
f9a6596
Revert "fixup! fixup! fix(toolchain): do not force users to depend on…
aignas Jun 15, 2024
8c5503c
Revert "fixup! fix(toolchain): do not force users to depend on option…
aignas Jun 15, 2024
35cbc55
Revert "fix(toolchain): do not force users to depend on optional tool…
aignas Jun 15, 2024
878d3f7
exp: move the precompilation toolchain disabling shortcircuit down th…
aignas Jun 15, 2024
3a307da
fix(precompile): autodetecting toolchain and feature flags
aignas Jun 15, 2024
20b70d9
fixup! fix(precompile): autodetecting toolchain and feature flags
aignas Jun 15, 2024
653d209
tests: enable precompile toolchain
aignas Jun 15, 2024
cce48cc
cleanup and paring down for code review
rickeylev Jun 19, 2024
8e40458
Merge branch 'main' of ssh://ssh.github.com:443/bazelbuild/rules_pyth…
rickeylev Jun 19, 2024
1faedf6
remove defunct comment
rickeylev Jun 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 25 additions & 1 deletion docs/sphinx/api/python/config_settings/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/sphinx/toolchains.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
load(
"//python/private:flags.bzl",
"BootstrapImplFlag",
"ExecToolsToolchainFlag",
"PrecompileAddToRunfilesFlag",
"PrecompileFlag",
"PrecompileSourceRetentionFlag",
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
3 changes: 2 additions & 1 deletion python/private/autodetecting_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions python/private/py_toolchain_suite.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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"),
)

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

Expand Down Expand Up @@ -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",
)
Expand Down Expand Up @@ -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",
},
)

Expand Down Expand Up @@ -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",
},
)

Expand Down Expand Up @@ -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",
},
)

Expand Down Expand Up @@ -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",
},
)

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