Skip to content

Commit

Permalink
fix(toolchain): do not force users to depend on optional toolchains
Browse files Browse the repository at this point in the history
With this change we set the `target_settings` to something that is not
going to be ever matched if the `precompile` flag is not enabled. Users
wanting to use the `precompile` feature will need to set the
config_setting value to `enabled` instead of `auto`.

Fixes #1967
  • Loading branch information
aignas committed Jun 14, 2024
1 parent 7a437cc commit 2aa7528
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 4 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ A brief description of the categories of changes:
* `protobuf`/`com_google_protobuf` dependency bumped to `v24.4`

### Fixed
* Nothing yet
* The targets would depend on the `exec_tools_toolchain_type` toolchain if it
was registered even though the pre-compilation was not enabled. This has been
fixed to not be the case anymore. Fixes [#1967](https://github.com/bazelbuild/rules_python/issues/1967).

### Removed
* Nothing yet
Expand Down
43 changes: 43 additions & 0 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//lib:selects.bzl", "selects")
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
load(
"//python/private:flags.bzl",
Expand Down Expand Up @@ -128,3 +129,45 @@ string_flag(
)
for flag in INTERNAL_FLAGS
]

config_setting(
name = "_precompile_enabled",
flag_values = {
":precompile": PrecompileFlag.ENABLED,
},
# Only public because it is used in toolchain definition
visibility = ["//visibility:public"],
)

config_setting(
name = "_precompile_enabled_generated_source",
flag_values = {
":precompile": PrecompileFlag.IF_GENERATED_SOURCE,
},
# Only public because it is used in toolchain definition
visibility = ["//visibility:public"],
)

config_setting(
name = "_precompile_enabled_force",
flag_values = {
":precompile": PrecompileFlag.FORCE_ENABLED,
},
# Only public because it is used in toolchain definition
visibility = ["//visibility:public"],
)

# FIXME @aignas 2024-06-14: this currently does not handle the `auto` value of
# the flag correctly. Right now the behaviour is to disable the pre-compilation
# via the _precompile_flag_get_effective_value function default to disabled if
# it the flag is "AUTO", however we may need to revisit this.
selects.config_setting_group(
name = "is_precompile_enabled",
match_any = [
":_precompile_enabled",
":_precompile_enabled_force",
":_precompile_enabled_generated_source",
],
# Only public because it is used in toolchain definition
visibility = ["//visibility:public"],
)
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("//python/private: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"],
)
2 changes: 1 addition & 1 deletion python/private/flags.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ BootstrapImplFlag = enum(
def _precompile_flag_get_effective_value(ctx):
value = ctx.attr._precompile_flag[BuildSettingInfo].value
if value == PrecompileFlag.AUTO:
value = PrecompileFlag.DISABLED
value = PrecompileFlag.IF_GENERATED_SOURCE
return value

# Determines if Python source files should be compiled at build time.
Expand Down
5 changes: 4 additions & 1 deletion python/private/py_toolchain_suite.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ def py_toolchain_suite(*, prefix, user_repository_name, python_version, set_pyth
),
toolchain_type = EXEC_TOOLS_TOOLCHAIN_TYPE,
# The target settings capture the Python version
target_settings = target_settings,
target_settings = select({
Label("//python/config_settings:is_precompile_enabled"): target_settings,
"//conditions:default": [Label("//python/config_settings:is_precompile_enabled")],
}),
exec_compatible_with = kwargs.get("target_compatible_with"),
)

Expand Down

0 comments on commit 2aa7528

Please sign in to comment.