Skip to content

Commit ae80da5

Browse files
committed
refactor(pypi): move config setting processing to the macro
Before hand we would pass around `whl_alias` struct instances and it would go through rounds of starlark functions which would end up rendered as BUILD.bazel files in the hub repository. This means that every time the output of the said functions would change, the `MODULE.bazel.lock` would also possibly change. Since we now have much better unit testing framework, we start relying on these functions being evaluated at loading phase instead of repo rule phase, which means that we can make the `BUILD.bazel` files not change a lot when the underlying code changes, this should make the code more maintainable in the long run and unblocks the code to accept extra things, like env marker values or similar. Summary: - Remove the `repo` field from `whl_alias`. - Rename `whl_alias` to `whl_config_setting` - Move the tests around - Simplify the `pkg_aliases` tests to use `contains` instead of exact equality to make things less brittle. - Simplify the `pkg_aliases` tests to use different mocks to make expectations easier to understand. - Make `whl_config_setting` hashable by using tuples instead of lists. This is definitely needed for bazel-contrib#2319 and bazel-contrib#2423.
1 parent 0e9f97d commit ae80da5

File tree

11 files changed

+1006
-842
lines changed

11 files changed

+1006
-842
lines changed

examples/bzlmod/MODULE.bazel.lock

+84-84
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

python/private/pypi/BUILD.bazel

+9-1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ bzl_library(
6464
":parse_whl_name_bzl",
6565
":pip_repository_attrs_bzl",
6666
":simpleapi_download_bzl",
67+
":whl_alias_bzl",
6768
":whl_library_bzl",
6869
":whl_repo_name_bzl",
6970
"//python/private:full_version_bzl",
@@ -156,7 +157,7 @@ bzl_library(
156157
name = "multi_pip_parse_bzl",
157158
srcs = ["multi_pip_parse.bzl"],
158159
deps = [
159-
"pip_repository_bzl",
160+
":pip_repository_bzl",
160161
"//python/private:text_util_bzl",
161162
],
162163
)
@@ -230,6 +231,7 @@ bzl_library(
230231
":parse_requirements_bzl",
231232
":pip_repository_attrs_bzl",
232233
":render_pkg_aliases_bzl",
234+
":whl_alias_bzl",
233235
"//python/private:normalize_name_bzl",
234236
"//python/private:repo_utils_bzl",
235237
"//python/private:text_util_bzl",
@@ -257,6 +259,7 @@ bzl_library(
257259
deps = [
258260
":generate_group_library_build_bazel_bzl",
259261
":parse_whl_name_bzl",
262+
":whl_alias_bzl",
260263
":whl_target_platforms_bzl",
261264
"//python/private:normalize_name_bzl",
262265
"//python/private:text_util_bzl",
@@ -283,6 +286,11 @@ bzl_library(
283286
],
284287
)
285288

289+
bzl_library(
290+
name = "whl_alias_bzl",
291+
srcs = ["whl_config_setting.bzl"],
292+
)
293+
286294
bzl_library(
287295
name = "whl_library_alias_bzl",
288296
srcs = ["whl_library_alias.bzl"],

python/private/pypi/extension.bzl

+17-41
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ load("//python/private:semver.bzl", "semver")
2323
load("//python/private:version_label.bzl", "version_label")
2424
load(":attrs.bzl", "use_isolated")
2525
load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
26-
load(":hub_repository.bzl", "hub_repository")
26+
load(":hub_repository.bzl", "hub_repository", "whl_aliases")
2727
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
2828
load(":parse_whl_name.bzl", "parse_whl_name")
2929
load(":pip_repository_attrs.bzl", "ATTRS")
30-
load(":render_pkg_aliases.bzl", "whl_alias")
3130
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
3231
load(":simpleapi_download.bzl", "simpleapi_download")
32+
load(":whl_config_setting.bzl", "whl_config_setting")
3333
load(":whl_library.bzl", "whl_library")
3434
load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name")
3535

@@ -87,7 +87,7 @@ def _create_whl_repos(
8787
Returns a {type}`struct` with the following attributes:
8888
whl_map: {type}`dict[str, list[struct]]` the output is keyed by the
8989
normalized package name and the values are the instances of the
90-
{bzl:obj}`whl_alias` return values.
90+
{bzl:obj}`whl_config_setting` return values.
9191
exposed_packages: {type}`dict[str, Any]` this is just a way to
9292
represent a set of string values.
9393
whl_libraries: {type}`dict[str, dict[str, Any]]` the keys are the
@@ -305,14 +305,11 @@ def _create_whl_repos(
305305

306306
whl_libraries[repo_name] = args
307307

308-
whl_map.setdefault(whl_name, []).append(
309-
whl_alias(
310-
repo = repo_name,
311-
version = major_minor,
312-
filename = distribution.filename,
313-
target_platforms = target_platforms,
314-
),
315-
)
308+
whl_map.setdefault(whl_name, {})[whl_config_setting(
309+
version = major_minor,
310+
filename = distribution.filename,
311+
target_platforms = target_platforms,
312+
)] = repo_name
316313

317314
if found_something:
318315
if is_exposed:
@@ -343,12 +340,7 @@ def _create_whl_repos(
343340
# args are manipulated in the code going before.
344341
repo_name = "{}_{}".format(pip_name, whl_name)
345342
whl_libraries[repo_name] = dict(whl_library_args.items())
346-
whl_map.setdefault(whl_name, []).append(
347-
whl_alias(
348-
repo = repo_name,
349-
version = major_minor,
350-
),
351-
)
343+
whl_map.setdefault(whl_name, {})[whl_config_setting(version = major_minor)] = repo_name
352344
continue
353345

354346
is_exposed = False
@@ -372,13 +364,10 @@ def _create_whl_repos(
372364
*target_platforms
373365
)
374366
whl_libraries[repo_name] = args
375-
whl_map.setdefault(whl_name, []).append(
376-
whl_alias(
377-
repo = repo_name,
378-
version = major_minor,
379-
target_platforms = target_platforms or None,
380-
),
381-
)
367+
whl_map.setdefault(whl_name, {})[whl_config_setting(
368+
version = major_minor,
369+
target_platforms = target_platforms or None,
370+
)] = repo_name
382371

383372
if is_exposed:
384373
exposed_packages[whl_name] = None
@@ -521,7 +510,8 @@ You cannot use both the additive_build_content and additive_build_content_file a
521510
)
522511
hub_whl_map.setdefault(hub_name, {})
523512
for key, settings in out.whl_map.items():
524-
hub_whl_map[hub_name].setdefault(key, []).extend(settings)
513+
for setting, repo in settings.items():
514+
hub_whl_map[hub_name].setdefault(key, {}).setdefault(repo, []).append(setting)
525515
extra_aliases.setdefault(hub_name, {})
526516
for whl_name, aliases in out.extra_aliases.items():
527517
extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
@@ -541,7 +531,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
541531
whl_mods = dict(sorted(whl_mods.items())),
542532
hub_whl_map = {
543533
hub_name: {
544-
whl_name: sorted(settings, key = lambda x: (x.version, x.filename))
534+
whl_name: dict(settings)
545535
for whl_name, settings in sorted(whl_map.items())
546536
}
547537
for hub_name, whl_map in sorted(hub_whl_map.items())
@@ -571,20 +561,6 @@ You cannot use both the additive_build_content and additive_build_content_file a
571561
is_reproducible = is_reproducible,
572562
)
573563

574-
def _alias_dict(a):
575-
ret = {
576-
"repo": a.repo,
577-
}
578-
if a.config_setting:
579-
ret["config_setting"] = a.config_setting
580-
if a.filename:
581-
ret["filename"] = a.filename
582-
if a.target_platforms:
583-
ret["target_platforms"] = a.target_platforms
584-
if a.version:
585-
ret["version"] = a.version
586-
return ret
587-
588564
def _pip_impl(module_ctx):
589565
"""Implementation of a class tag that creates the pip hub and corresponding pip spoke whl repositories.
590566
@@ -665,7 +641,7 @@ def _pip_impl(module_ctx):
665641
repo_name = hub_name,
666642
extra_hub_aliases = mods.extra_aliases.get(hub_name, {}),
667643
whl_map = {
668-
key: json.encode([_alias_dict(a) for a in aliases])
644+
key: whl_aliases(aliases)
669645
for key, aliases in whl_map.items()
670646
},
671647
packages = mods.exposed_packages.get(hub_name, []),

python/private/pypi/hub_repository.bzl

+45-6
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,8 @@
1515
""
1616

1717
load("//python/private:text_util.bzl", "render")
18-
load(
19-
":render_pkg_aliases.bzl",
20-
"render_multiplatform_pkg_aliases",
21-
"whl_alias",
22-
)
18+
load(":render_pkg_aliases.bzl", "render_multiplatform_pkg_aliases")
19+
load(":whl_config_setting.bzl", "whl_config_setting")
2320

2421
_BUILD_FILE_CONTENTS = """\
2522
package(default_visibility = ["//visibility:public"])
@@ -32,7 +29,7 @@ def _impl(rctx):
3229
bzl_packages = rctx.attr.packages or rctx.attr.whl_map.keys()
3330
aliases = render_multiplatform_pkg_aliases(
3431
aliases = {
35-
key: [whl_alias(**v) for v in json.decode(values)]
32+
key: _whl_aliases(values)
3633
for key, values in rctx.attr.whl_map.items()
3734
},
3835
extra_hub_aliases = rctx.attr.extra_hub_aliases,
@@ -97,3 +94,45 @@ in the pip.parse tag class.
9794
doc = """A rule for bzlmod mulitple pip repository creation. PRIVATE USE ONLY.""",
9895
implementation = _impl,
9996
)
97+
98+
def _whl_aliases(repo_mapping_json):
99+
"""Inverse of whl_aliases
100+
101+
Args:
102+
repo_mapping_json: {type}`str`
103+
104+
Returns:
105+
What `whl_aliases` accepts.
106+
"""
107+
return {
108+
whl_config_setting(**v): repo
109+
for repo, values in json.decode(repo_mapping_json).items()
110+
for v in values
111+
}
112+
113+
def whl_aliases(repo_mapping):
114+
"""A function to serialize the aliases so that `hub_repository` can accept them.
115+
116+
Args:
117+
repo_mapping: {type}`dict[str, list[struct]]` repo to
118+
{obj}`whl_config_setting` mapping.
119+
120+
Returns:
121+
A deserializable JSON string
122+
"""
123+
return json.encode({
124+
repo: [_whl_config_setting_dict(s) for s in settings]
125+
for repo, settings in repo_mapping.items()
126+
})
127+
128+
def _whl_config_setting_dict(a):
129+
ret = {}
130+
if a.config_setting:
131+
ret["config_setting"] = a.config_setting
132+
if a.filename:
133+
ret["filename"] = a.filename
134+
if a.target_platforms:
135+
ret["target_platforms"] = a.target_platforms
136+
if a.version:
137+
ret["version"] = a.version
138+
return ret

python/private/pypi/pip_repository.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ load("//python/private:text_util.bzl", "render")
2121
load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
2222
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
2323
load(":pip_repository_attrs.bzl", "ATTRS")
24-
load(":render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
24+
load(":render_pkg_aliases.bzl", "render_pkg_aliases")
2525
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
2626

2727
def _get_python_interpreter_attr(rctx):
@@ -174,7 +174,7 @@ def _pip_repository_impl(rctx):
174174

175175
aliases = render_pkg_aliases(
176176
aliases = {
177-
pkg: [whl_alias(repo = rctx.attr.name + "_" + pkg)]
177+
pkg: rctx.attr.name + "_" + pkg
178178
for pkg in bzl_packages or []
179179
},
180180
)

0 commit comments

Comments
 (0)