Skip to content

Commit

Permalink
refactor(pypi): move config setting processing to the macro
Browse files Browse the repository at this point in the history
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 #2319 and #2423.
  • Loading branch information
aignas committed Nov 18, 2024
1 parent 9766cb6 commit 16cf803
Show file tree
Hide file tree
Showing 11 changed files with 1,006 additions and 842 deletions.
168 changes: 84 additions & 84 deletions examples/bzlmod/MODULE.bazel.lock

Large diffs are not rendered by default.

10 changes: 9 additions & 1 deletion python/private/pypi/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ bzl_library(
":parse_whl_name_bzl",
":pip_repository_attrs_bzl",
":simpleapi_download_bzl",
":whl_alias_bzl",
":whl_library_bzl",
":whl_repo_name_bzl",
"//python/private:full_version_bzl",
Expand Down Expand Up @@ -156,7 +157,7 @@ bzl_library(
name = "multi_pip_parse_bzl",
srcs = ["multi_pip_parse.bzl"],
deps = [
"pip_repository_bzl",
":pip_repository_bzl",
"//python/private:text_util_bzl",
],
)
Expand Down Expand Up @@ -230,6 +231,7 @@ bzl_library(
":parse_requirements_bzl",
":pip_repository_attrs_bzl",
":render_pkg_aliases_bzl",
":whl_alias_bzl",
"//python/private:normalize_name_bzl",
"//python/private:repo_utils_bzl",
"//python/private:text_util_bzl",
Expand Down Expand Up @@ -257,6 +259,7 @@ bzl_library(
deps = [
":generate_group_library_build_bazel_bzl",
":parse_whl_name_bzl",
":whl_alias_bzl",
":whl_target_platforms_bzl",
"//python/private:normalize_name_bzl",
"//python/private:text_util_bzl",
Expand All @@ -283,6 +286,11 @@ bzl_library(
],
)

bzl_library(
name = "whl_alias_bzl",
srcs = ["whl_config_setting.bzl"],
)

bzl_library(
name = "whl_library_alias_bzl",
srcs = ["whl_library_alias.bzl"],
Expand Down
58 changes: 17 additions & 41 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ load("//python/private:semver.bzl", "semver")
load("//python/private:version_label.bzl", "version_label")
load(":attrs.bzl", "use_isolated")
load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
load(":hub_repository.bzl", "hub_repository")
load(":hub_repository.bzl", "hub_repository", "whl_aliases")
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
load(":parse_whl_name.bzl", "parse_whl_name")
load(":pip_repository_attrs.bzl", "ATTRS")
load(":render_pkg_aliases.bzl", "whl_alias")
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
load(":simpleapi_download.bzl", "simpleapi_download")
load(":whl_config_setting.bzl", "whl_config_setting")
load(":whl_library.bzl", "whl_library")
load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name")

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

whl_libraries[repo_name] = args

whl_map.setdefault(whl_name, []).append(
whl_alias(
repo = repo_name,
version = major_minor,
filename = distribution.filename,
target_platforms = target_platforms,
),
)
whl_map.setdefault(whl_name, {})[whl_config_setting(
version = major_minor,
filename = distribution.filename,
target_platforms = target_platforms,
)] = repo_name

if found_something:
if is_exposed:
Expand Down Expand Up @@ -343,12 +340,7 @@ def _create_whl_repos(
# args are manipulated in the code going before.
repo_name = "{}_{}".format(pip_name, whl_name)
whl_libraries[repo_name] = dict(whl_library_args.items())
whl_map.setdefault(whl_name, []).append(
whl_alias(
repo = repo_name,
version = major_minor,
),
)
whl_map.setdefault(whl_name, {})[whl_config_setting(version = major_minor)] = repo_name
continue

is_exposed = False
Expand All @@ -372,13 +364,10 @@ def _create_whl_repos(
*target_platforms
)
whl_libraries[repo_name] = args
whl_map.setdefault(whl_name, []).append(
whl_alias(
repo = repo_name,
version = major_minor,
target_platforms = target_platforms or None,
),
)
whl_map.setdefault(whl_name, {})[whl_config_setting(
version = major_minor,
target_platforms = target_platforms or None,
)] = repo_name

if is_exposed:
exposed_packages[whl_name] = None
Expand Down Expand Up @@ -521,7 +510,8 @@ You cannot use both the additive_build_content and additive_build_content_file a
)
hub_whl_map.setdefault(hub_name, {})
for key, settings in out.whl_map.items():
hub_whl_map[hub_name].setdefault(key, []).extend(settings)
for setting, repo in settings.items():
hub_whl_map[hub_name].setdefault(key, {}).setdefault(repo, []).append(setting)
extra_aliases.setdefault(hub_name, {})
for whl_name, aliases in out.extra_aliases.items():
extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
Expand All @@ -541,7 +531,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
whl_mods = dict(sorted(whl_mods.items())),
hub_whl_map = {
hub_name: {
whl_name: sorted(settings, key = lambda x: (x.version, x.filename))
whl_name: dict(settings)
for whl_name, settings in sorted(whl_map.items())
}
for hub_name, whl_map in sorted(hub_whl_map.items())
Expand Down Expand Up @@ -571,20 +561,6 @@ You cannot use both the additive_build_content and additive_build_content_file a
is_reproducible = is_reproducible,
)

def _alias_dict(a):
ret = {
"repo": a.repo,
}
if a.config_setting:
ret["config_setting"] = a.config_setting
if a.filename:
ret["filename"] = a.filename
if a.target_platforms:
ret["target_platforms"] = a.target_platforms
if a.version:
ret["version"] = a.version
return ret

def _pip_impl(module_ctx):
"""Implementation of a class tag that creates the pip hub and corresponding pip spoke whl repositories.
Expand Down Expand Up @@ -665,7 +641,7 @@ def _pip_impl(module_ctx):
repo_name = hub_name,
extra_hub_aliases = mods.extra_aliases.get(hub_name, {}),
whl_map = {
key: json.encode([_alias_dict(a) for a in aliases])
key: whl_aliases(aliases)
for key, aliases in whl_map.items()
},
packages = mods.exposed_packages.get(hub_name, []),
Expand Down
51 changes: 45 additions & 6 deletions python/private/pypi/hub_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
""

load("//python/private:text_util.bzl", "render")
load(
":render_pkg_aliases.bzl",
"render_multiplatform_pkg_aliases",
"whl_alias",
)
load(":render_pkg_aliases.bzl", "render_multiplatform_pkg_aliases")
load(":whl_config_setting.bzl", "whl_config_setting")

_BUILD_FILE_CONTENTS = """\
package(default_visibility = ["//visibility:public"])
Expand All @@ -32,7 +29,7 @@ def _impl(rctx):
bzl_packages = rctx.attr.packages or rctx.attr.whl_map.keys()
aliases = render_multiplatform_pkg_aliases(
aliases = {
key: [whl_alias(**v) for v in json.decode(values)]
key: _whl_aliases(values)
for key, values in rctx.attr.whl_map.items()
},
extra_hub_aliases = rctx.attr.extra_hub_aliases,
Expand Down Expand Up @@ -97,3 +94,45 @@ in the pip.parse tag class.
doc = """A rule for bzlmod mulitple pip repository creation. PRIVATE USE ONLY.""",
implementation = _impl,
)

def _whl_aliases(repo_mapping_json):
"""Inverse of whl_aliases
Args:
repo_mapping_json: {type}`str`
Returns:
What `whl_aliases` accepts.
"""
return {
whl_config_setting(**v): repo
for repo, values in json.decode(repo_mapping_json).items()
for v in values
}

def whl_aliases(repo_mapping):
"""A function to serialize the aliases so that `hub_repository` can accept them.
Args:
repo_mapping: {type}`dict[str, list[struct]]` repo to
{obj}`whl_config_setting` mapping.
Returns:
A deserializable JSON string
"""
return json.encode({
repo: [_whl_config_setting_dict(s) for s in settings]
for repo, settings in repo_mapping.items()
})

def _whl_config_setting_dict(a):
ret = {}
if a.config_setting:
ret["config_setting"] = a.config_setting
if a.filename:
ret["filename"] = a.filename
if a.target_platforms:
ret["target_platforms"] = a.target_platforms
if a.version:
ret["version"] = a.version
return ret
4 changes: 2 additions & 2 deletions python/private/pypi/pip_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ load("//python/private:text_util.bzl", "render")
load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
load(":pip_repository_attrs.bzl", "ATTRS")
load(":render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
load(":render_pkg_aliases.bzl", "render_pkg_aliases")
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")

def _get_python_interpreter_attr(rctx):
Expand Down Expand Up @@ -174,7 +174,7 @@ def _pip_repository_impl(rctx):

aliases = render_pkg_aliases(
aliases = {
pkg: [whl_alias(repo = rctx.attr.name + "_" + pkg)]
pkg: rctx.attr.name + "_" + pkg
for pkg in bzl_packages or []
},
)
Expand Down
Loading

0 comments on commit 16cf803

Please sign in to comment.