Skip to content

refactor/fix: load target_platforms through the hub #2781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/mypy.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
---
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? what's this? stray edit?

name: mypy

on:
Expand Down
Empty file added config.bzl.tmpl.bzlmod
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray file i think?

Empty file.
14 changes: 2 additions & 12 deletions python/private/pypi/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,6 @@ bzl_library(
],
)

bzl_library(
name = "pep508_bzl",
srcs = ["pep508.bzl"],
deps = [
":pep508_env_bzl",
":pep508_evaluate_bzl",
],
)

bzl_library(
name = "pep508_deps_bzl",
srcs = ["pep508_deps.bzl"],
Expand Down Expand Up @@ -378,13 +369,12 @@ bzl_library(
":attrs_bzl",
":deps_bzl",
":generate_whl_library_build_bazel_bzl",
":parse_whl_name_bzl",
":patch_whl_bzl",
":pep508_deps_bzl",
":pep508_requirement_bzl",
":pypi_repo_utils_bzl",
":whl_metadata_bzl",
":whl_target_platforms_bzl",
"//python/private:auth_bzl",
"//python/private:bzlmod_enabled_bzl",
"//python/private:envsubst_bzl",
"//python/private:is_standalone_interpreter_bzl",
"//python/private:repo_utils_bzl",
Expand Down
9 changes: 9 additions & 0 deletions python/private/pypi/config.bzl.tmpl.bzlmod
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""Extra configuration values that are exposed from the hub repository for spoke repositories to access.

NOTE: This is internal `rules_python` API and if you would like to depend on it, please raise an issue
with your usecase. This may change in between rules_python versions without any notice.

@generated by rules_python pip.parse bzlmod extension.
"""

target_platforms = %%TARGET_PLATFORMS%%
41 changes: 18 additions & 23 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ 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")
load(":whl_target_platforms.bzl", "whl_target_platforms")

def _major_minor_version(version):
version = semver(version)
Expand Down Expand Up @@ -68,7 +67,6 @@ def _create_whl_repos(
*,
pip_attr,
whl_overrides,
evaluate_markers = evaluate_markers,
available_interpreters = INTERPRETER_LABELS,
get_index_urls = None):
"""create all of the whl repositories
Expand All @@ -77,7 +75,6 @@ def _create_whl_repos(
module_ctx: {type}`module_ctx`.
pip_attr: {type}`struct` - the struct that comes from the tag class iteration.
whl_overrides: {type}`dict[str, struct]` - per-wheel overrides.
evaluate_markers: the function to use to evaluate markers.
get_index_urls: A function used to get the index URLs
available_interpreters: {type}`dict[str, Label]` The dictionary of available
interpreters that have been registered using the `python` bzlmod extension.
Expand Down Expand Up @@ -162,14 +159,12 @@ def _create_whl_repos(
requirements_osx = pip_attr.requirements_darwin,
requirements_windows = pip_attr.requirements_windows,
extra_pip_args = pip_attr.extra_pip_args,
# TODO @aignas 2025-04-15: pass the full version into here
python_version = major_minor,
logger = logger,
),
extra_pip_args = pip_attr.extra_pip_args,
get_index_urls = get_index_urls,
# NOTE @aignas 2025-02-24: we will use the "cp3xx_os_arch" platform labels
# for converting to the PEP508 environment and will evaluate them in starlark
# without involving the interpreter at all.
evaluate_markers = evaluate_markers,
logger = logger,
)
Expand All @@ -191,7 +186,6 @@ def _create_whl_repos(
enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
environment = pip_attr.environment,
envsubst = pip_attr.envsubst,
experimental_target_platforms = pip_attr.experimental_target_platforms,
group_deps = group_deps,
group_name = group_name,
pip_data_exclude = pip_attr.pip_data_exclude,
Expand Down Expand Up @@ -244,6 +238,12 @@ def _create_whl_repos(
},
extra_aliases = extra_aliases,
whl_libraries = whl_libraries,
target_platforms = {
plat: None
for reqs in requirements_by_platform.values()
for req in reqs
for plat in req.target_platforms
},
)

def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version):
Expand Down Expand Up @@ -274,20 +274,11 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt
args["urls"] = [distribution.url]
args["sha256"] = distribution.sha256
args["filename"] = distribution.filename
args["experimental_target_platforms"] = requirement.target_platforms

# Pure python wheels or sdists may need to have a platform here
target_platforms = None
if distribution.filename.endswith(".whl") and not distribution.filename.endswith("-any.whl"):
parsed_whl = parse_whl_name(distribution.filename)
whl_platforms = whl_target_platforms(
platform_tag = parsed_whl.platform_tag,
)
args["experimental_target_platforms"] = [
p
for p in requirement.target_platforms
if [None for wp in whl_platforms if p.endswith(wp.target_platform)]
]
pass
elif multiple_requirements_for_whl:
target_platforms = requirement.target_platforms

Expand Down Expand Up @@ -416,6 +407,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
hub_group_map = {}
exposed_packages = {}
extra_aliases = {}
target_platforms = {}
whl_libraries = {}

for mod in module_ctx.modules:
Expand Down Expand Up @@ -498,6 +490,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
for whl_name, aliases in out.extra_aliases.items():
extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
exposed_packages.setdefault(hub_name, {}).update(out.exposed_packages)
target_platforms.setdefault(hub_name, {}).update(out.target_platforms)
whl_libraries.update(out.whl_libraries)

# TODO @aignas 2024-04-05: how do we support different requirement
Expand Down Expand Up @@ -535,6 +528,10 @@ You cannot use both the additive_build_content and additive_build_content_file a
}
for hub_name, extra_whl_aliases in extra_aliases.items()
},
target_platforms = {
hub_name: sorted(p)
for hub_name, p in target_platforms.items()
},
whl_libraries = {
k: dict(sorted(args.items()))
for k, args in sorted(whl_libraries.items())
Expand Down Expand Up @@ -626,15 +623,13 @@ def _pip_impl(module_ctx):
},
packages = mods.exposed_packages.get(hub_name, []),
groups = mods.hub_group_map.get(hub_name),
target_platforms = mods.target_platforms.get(hub_name, []),
)

if bazel_features.external_deps.extension_metadata_has_reproducible:
# If we are not using the `experimental_index_url feature, the extension is fully
# deterministic and we don't need to create a lock entry for it.
#
# In order to be able to dogfood the `experimental_index_url` feature before it gets
# stabilized, we have created the `_pip_non_reproducible` function, that will result
# in extra entries in the lock file.
# NOTE @aignas 2025-04-15: this is set to be reproducible, because the
# results after calling the PyPI index should be reproducible on each
# machine.
return module_ctx.extension_metadata(reproducible = True)
else:
return None
Expand Down
27 changes: 22 additions & 5 deletions python/private/pypi/generate_whl_library_build_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,56 @@ _RENDER = {
"copy_files": render.dict,
"data": render.list,
"data_exclude": render.list,
"dependencies": render.list,
"dependencies_by_platform": lambda x: render.dict(x, value_repr = render.list),
"entry_points": render.dict,
"extras": render.list,
"group_deps": render.list,
"requires_dist": render.list,
"srcs_exclude": render.list,
"tags": render.list,
"target_platforms": lambda x: render.list(x) if x else "target_platforms",
}

# NOTE @aignas 2024-10-25: We have to keep this so that files in
# this repository can be publicly visible without the need for
# export_files
_TEMPLATE = """\
load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets")
{loads}

package(default_visibility = ["//visibility:public"])

whl_library_targets(
whl_library_targets_from_requires(
{kwargs}
)
"""

def generate_whl_library_build_bazel(
*,
annotation = None,
default_python_version = None,
**kwargs):
"""Generate a BUILD file for an unzipped Wheel

Args:
annotation: The annotation for the build file.
default_python_version: The python version to use to parse the METADATA.
**kwargs: Extra args serialized to be passed to the
{obj}`whl_library_targets`.

Returns:
A complete BUILD file as a string
"""

loads = [
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""",
]
if not kwargs.setdefault("target_platforms", None):
dep_template = kwargs["dep_template"]
loads.append(
"load(\"{}\", \"{}\")".format(
dep_template.format(name = "", target = "config.bzl"),
"target_platforms",
),
)

additional_content = []
if annotation:
kwargs["data"] = annotation.data
Expand All @@ -66,10 +80,13 @@ def generate_whl_library_build_bazel(
kwargs["srcs_exclude"] = annotation.srcs_exclude_glob
if annotation.additive_build_content:
additional_content.append(annotation.additive_build_content)
if default_python_version:
kwargs["default_python_version"] = default_python_version

contents = "\n".join(
[
_TEMPLATE.format(
loads = "\n".join(loads),
kwargs = render.indent("\n".join([
"{} = {},".format(k, _RENDER.get(k, repr)(v))
for k, v in sorted(kwargs.items())
Expand Down
18 changes: 16 additions & 2 deletions python/private/pypi/hub_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ def _impl(rctx):
macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name)

rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS)
rctx.template("requirements.bzl", rctx.attr._template, substitutions = {
rctx.template(
"config.bzl",
rctx.attr._config_template,
substitutions = {
"%%TARGET_PLATFORMS%%": render.list(rctx.attr.target_platforms),
},
)
rctx.template("requirements.bzl", rctx.attr._requirements_bzl_template, substitutions = {
"%%ALL_DATA_REQUIREMENTS%%": render.list([
macro_tmpl.format(p, "data")
for p in bzl_packages
Expand Down Expand Up @@ -80,14 +87,21 @@ The list of packages that will be exposed via all_*requirements macros. Defaults
mandatory = True,
doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.",
),
"target_platforms": attr.string_list(
mandatory = True,
doc = "All of the target platforms for the hub repo",
),
"whl_map": attr.string_dict(
mandatory = True,
doc = """\
The wheel map where values are json.encoded strings of the whl_map constructed
in the pip.parse tag class.
""",
),
"_template": attr.label(
"_config_template": attr.label(
default = ":config.bzl.tmpl.bzlmod",
),
"_requirements_bzl_template": attr.label(
default = ":requirements.bzl.tmpl.bzlmod",
),
},
Expand Down
23 changes: 0 additions & 23 deletions python/private/pypi/pep508.bzl

This file was deleted.

Loading