Skip to content

Commit cc3b182

Browse files
committed
refactor(bzlmod): Reduce the number of pip config setting targets
This essentially removes the `python_version` from `flag_values` and reduces the number of targets we create by a lot because we no longer need to create all of the micro version combinations when we want to match e.g. `3.10` by using a trick I've learned in bazel-contrib#1968: ```starlark select({ "is_python_3.x": "my_config_setting" "//conditions:default": "is_python_3.x" }) ``` The config settings targets in `//tests/private/pip_config_settings/...` where we have multiple python versions is changing: * ":is_*" - 995 to 996 (the extra addition being `is_python_default`) * ":_*is_*" - 28272 to 2480 CHANGELOG: - (bzlmod): optimize the creation of config settings used in pip to reduce the total number of targets.
1 parent 7a437cc commit cc3b182

File tree

2 files changed

+33
-39
lines changed

2 files changed

+33
-39
lines changed

python/private/config_settings.bzl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,14 @@ def construct_config_settings(name = None): # buildifier: disable=function-docs
181181
visibility = ["//visibility:public"],
182182
)
183183

184+
native.config_setting(
185+
name = "is_python_default",
186+
flag_values = {
187+
Label("//python/config_settings:python_version"): "",
188+
},
189+
visibility = ["//visibility:public"],
190+
)
191+
184192
for version, matching_versions in VERSION_FLAG_VALUES.items():
185193
is_python_config_setting(
186194
name = "is_python_{}".format(version),

python/private/pip_config_settings.bzl

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ Note, that here the specialization of musl vs manylinux wheels is the same in
3939
order to ensure that the matching fails if the user requests for `musl` and we don't have it or vice versa.
4040
"""
4141

42-
load(":config_settings.bzl", "is_python_config_setting")
4342
load(
4443
":pip_flags.bzl",
4544
"INTERNAL_FLAGS",
@@ -116,7 +115,7 @@ def pip_config_settings(
116115

117116
alias_rule = alias_rule or native.alias
118117

119-
for version in python_versions:
118+
for version in ["default"] + python_versions:
120119
is_python = "is_python_{}".format(version)
121120
alias_rule(
122121
name = is_python,
@@ -174,16 +173,18 @@ def _whl_config_settings(*, suffix, plat_flag_values, **kwargs):
174173
flag_values = {}
175174

176175
for n, f in {
177-
"{}_none_any{}".format(py, suffix): None,
176+
"{}_none_any{}".format(py, suffix): _flags.whl_py2_py3,
178177
"{}3_none_any{}".format(py, suffix): _flags.whl_py3,
179178
"{}3_abi3_any{}".format(py, suffix): _flags.whl_py3_abi3,
180179
"{}_none_any{}".format(pycp, suffix): _flags.whl_pycp3x,
181180
"{}_abi3_any{}".format(pycp, suffix): _flags.whl_pycp3x_abi3,
182181
"{}_cp_any{}".format(pycp, suffix): _flags.whl_pycp3x_abicp,
183182
}.items():
184-
if f and f in flag_values:
185-
fail("BUG")
186-
elif f:
183+
if f in flag_values:
184+
# This should never happen as all of the different whls should have
185+
# unique flag values.
186+
fail("BUG: the flag {} is attempted to be added twice to the list".format(f))
187+
else:
187188
flag_values[f] = ""
188189

189190
_whl_config_setting(
@@ -205,9 +206,11 @@ def _whl_config_settings(*, suffix, plat_flag_values, **kwargs):
205206
"{}_abi3_{}".format(pycp, suffix): _flags.whl_plat_pycp3x_abi3,
206207
"{}_cp_{}".format(pycp, suffix): _flags.whl_plat_pycp3x_abicp,
207208
}.items():
208-
if f and f in flag_values:
209-
fail("BUG")
210-
elif f:
209+
if f in flag_values:
210+
# This should never happen as all of the different whls should have
211+
# unique flag values.
212+
fail("BUG: the flag {} is attempted to be added twice to the list".format(f))
213+
else:
211214
flag_values[f] = ""
212215

213216
_whl_config_setting(
@@ -290,7 +293,6 @@ def _whl_config_setting(*, name, flag_values, visibility, config_setting_rule =
290293
FLAGS.pip_whl: UseWhlFlag.ONLY,
291294
},
292295
default = flag_values | {
293-
_flags.whl_py2_py3: "",
294296
FLAGS.pip_whl: UseWhlFlag.AUTO,
295297
},
296298
visibility = visibility,
@@ -333,34 +335,18 @@ def _config_setting_or(*, name, flag_values, default, visibility, **kwargs):
333335
**kwargs
334336
)
335337

336-
def _config_setting(python_version = "", **kwargs):
338+
def _config_setting(*, name, python_version = "", **kwargs):
337339
if python_version:
338-
# NOTE @aignas 2024-05-26: with this we are getting about 24k internal
339-
# config_setting targets in our unit tests. Whilst the number of the
340-
# external dependencies does not dictate this number, it does mean that
341-
# bazel will take longer to parse stuff. This would be especially
342-
# noticeable in repos, which use multiple hub repos within a single
343-
# workspace.
344-
#
345-
# A way to reduce the number of targets would be:
346-
# * put them to a central location and teach this code to just alias them,
347-
# maybe we should create a pip_config_settings repo within the pip
348-
# extension, which would collect config settings for all hub_repos.
349-
# * put them in rules_python - this has the drawback of exposing things like
350-
# is_cp3.10_linux and users may start depending upon the naming
351-
# convention and this API is very unstable.
352-
is_python_config_setting(
353-
python_version = python_version,
354-
**kwargs
355-
)
340+
actual = name.replace("is_cp{}".format(python_version), "_is")
356341
else:
357-
# We need this to ensure that there are no ambiguous matches when python_version
358-
# is unset, which usually happens when we are not using the python version aware
359-
# rules.
360-
flag_values = kwargs.pop("flag_values", {}) | {
361-
FLAGS.python_version: "",
362-
}
363-
native.config_setting(
364-
flag_values = flag_values,
365-
**kwargs
366-
)
342+
native.config_setting(name = "_" + name, **kwargs)
343+
actual = "_" + name
344+
345+
is_python = ":is_python_" + (python_version or "default")
346+
native.alias(
347+
name = name,
348+
actual = select({
349+
is_python: ":" + actual,
350+
"//conditions:default": is_python,
351+
}),
352+
)

0 commit comments

Comments
 (0)