8000 cleanup: Set toolchain target_setting directly instead of via inline … · bazel-contrib/rules_python@4c365e7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4c365e7

Browse files
rickeylevrickeylev
authored
cleanup: Set toolchain target_setting directly instead of via inline ternary (#1246)
The generated toolchain BUILD file is confusing to read because it relies on a ternary expression in the BUILD file to set the `target_settings` attribute. This makes debugging harder because, upon first look, all the toolchains appear to have the version constraint set. It's only upon closer inspection that you can see the 1-character difference of "False" vs "True" embedded into the middle of a line amongst other similar looking lines. Also: * Adds a bit of validation logic for the `set_python_version_constraint` argument because it's conceptually a boolean, but is passed as a string, so is prone to having an incorrect value passed. * Documents the `set_python_version_constraint` arg, since it has a particular range of values it accepts.
1 parent 7f6de72 commit 4c365e7

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

‎python/private/toolchains_repo.bzl

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,30 @@ def python_toolchain_build_file_content(
4646
Args:
4747
prefix: Python toolchain name prefixes
4848
python_version: Python versions for the toolchains
49-
set_python_version_constraint: string "True" or "False"
49+
set_python_version_constraint: string, "True" if the toolchain should
50+
have the Python version constraint added as a requirement for
51+
matching the toolchain, "False" if not.
5052
user_repository_name: names for the user repos
5153
rules_python: rules_python label
5254
5355
Returns:
5456
build_content: Text containing toolchain definitions
5557
"""
56-
57-
python_version_constraint = "{rules_python}//python/config_settings:is_python_{python_version}".format(
58-
rules_python = rules_python,
59-
python_version = python_version,
60-
)
58+
if set_python_version_constraint == "True":
59+
constraint = "{rules_python}//python/config_settings:is_python_{python_version}".format(
60+
rules_python = rules_python,
61+
python_version = python_version,
62+
)
63+
target_settings = '["{}"]'.format(constraint)
64+
elif set_python_version_constraint == "False":
65+
target_settings = "[]"
66+
else:
67+
fail(("Invalid set_python_version_constraint value: got {} {}, wanted " +
68+
"either the string 'True' or the string 'False'; " +
69+
"(did you convert bool to string?)").format(
70+
type(set_python_version_constraint),
71+
repr(set_python_version_constraint),
72+
))
6173

6274
# We create a list of toolchain content from iterating over
6375
# the enumeration of PLATFORMS. We enumerate PLATFORMS in
@@ -67,19 +79,18 @@ def python_toolchain_build_file_content(
6779
toolchain(
6880
name = "{prefix}{platform}_toolchain",
6981
target_compatible_with = {compatible_with},
70-
target_settings = ["{python_version_constraint}"] if {set_python_version_constraint} else [],
82+
target_settings = {target_settings},
7183
toolchain = "@{user_repository_name}_{platform}//:python_runtimes",
7284
toolchain_type = "@bazel_tools//tools/python:toolchain_type",
7385
)
7486
""".format(
7587
compatible_with = meta.compatible_with,
7688
platform = platform,
77-
python_version_constraint = python_version_constraint,
7889
# We have to use a String value here because bzlmod is passing in a
7990
# string as we cannot have list of bools in build rule attribues.
8091
# This if statement does not appear to work unless it is in the
8192
# toolchain file.
82-
set_python_version_constraint = True if set_python_version_constraint == "True" else False,
93+
target_settings = target_settings,
8394
user_repository_name = user_repository_name,
8495
prefix = prefix,
8596
)

0 commit comments

Comments
 (0)
0