Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 42b8510

Browse filesBrowse files
aignasaignas
committed
refactor(pypi): split out more utils and start passing 'abi_os_arch' around
This is extra preparation needed for #2059. Summary: - Create `pypi_repo_utils` for more ergonomic handling of Python in repo context. - Split the resolution of requirements files to platforms into a separate function to make the testing easier. This also allows more validation that was realized that there is a need for in the WIP feature PR. - Make the code more robust about the assumption of the target platform label. Work towards #260, #1105, #1868.
1 parent 87fd3f5 commit 42b8510
Copy full SHA for 42b8510

File tree

Expand file treeCollapse file tree

12 files changed

+674
-508
lines changed
Filter options
Expand file treeCollapse file tree

12 files changed

+674
-508
lines changed

‎python/private/pypi/BUILD.bazel

Copy file name to clipboardExpand all lines: python/private/pypi/BUILD.bazel
+20-2Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ bzl_library(
161161
deps = [
162162
":index_sources_bzl",
163163
":parse_requirements_txt_bzl",
164+
":pypi_repo_utils_bzl",
165+
":requirements_files_by_platform_bzl",
164166
":whl_target_platforms_bzl",
165167
"//python/private:normalize_name_bzl",
166168
],
@@ -227,6 +229,15 @@ bzl_library(
227229
srcs = ["pip_repository_attrs.bzl"],
228230
)
229231

232+
bzl_library(
233+
name = "pypi_repo_utils_bzl",
234+
srcs = ["pypi_repo_utils.bzl"],
235+
deps = [
236+
"//python:versions_bzl",
237+
"//python/private:toolchains_repo_bzl",
238+
],
239+
)
240+
230241
bzl_library(
231242
name = "render_pkg_aliases_bzl",
232243
srcs = ["render_pkg_aliases.bzl"],
@@ -240,6 +251,14 @@ bzl_library(
240251
],
241252
)
242253

254+
bzl_library(
255+
name = "requirements_files_by_platform_bzl",
256+
srcs = ["requirements_files_by_platform.bzl"],
257+
deps = [
258+
":whl_target_platforms_bzl",
259+
],
260+
)
261+
243262
bzl_library(
244263
name = "simpleapi_download_bzl",
245264
srcs = ["simpleapi_download.bzl"],
@@ -270,13 +289,12 @@ bzl_library(
270289
":generate_whl_library_build_bazel_bzl",
271290
":parse_whl_name_bzl",
272291
":patch_whl_bzl",
292+
":pypi_repo_utils_bzl",
273293
":whl_target_platforms_bzl",
274294
"//python:repositories_bzl",
275-
"//python:versions_bzl",
276295
"//python/private:auth_bzl",
277296
"//python/private:envsubst_bzl",
278297
"//python/private:repo_utils_bzl",
279-
"//python/private:toolchains_repo_bzl",
280298
],
281299
)
282300

‎python/private/pypi/extension.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/extension.bzl
+12-7Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_r
2626
load(":parse_whl_name.bzl", "parse_whl_name")
2727
load(":pip_repository_attrs.bzl", "ATTRS")
2828
load(":render_pkg_aliases.bzl", "whl_alias")
29+
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
2930
load(":simpleapi_download.bzl", "simpleapi_download")
3031
load(":whl_library.bzl", "whl_library")
3132
load(":whl_repo_name.bzl", "whl_repo_name")
@@ -183,12 +184,16 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
183184

184185
requirements_by_platform = parse_requirements(
185186
module_ctx,
186-
requirements_by_platform = pip_attr.requirements_by_platform,
187-
requirements_linux = pip_attr.requirements_linux,
188-
requirements_lock = pip_attr.requirements_lock,
189-
requirements_osx = pip_attr.requirements_darwin,
190-
requirements_windows = pip_attr.requirements_windows,
191-
extra_pip_args = pip_attr.extra_pip_args,
187+
requirements_by_platform = requirements_files_by_platform(
188+
requirements_by_platform = pip_attr.requirements_by_platform,
189+
requirements_linux = pip_attr.requirements_linux,
190+
requirements_lock = pip_attr.requirements_lock,
191+
requirements_osx = pip_attr.requirements_darwin,
192+
requirements_windows = pip_attr.requirements_windows,
193+
extra_pip_args = pip_attr.extra_pip_args,
194+
python_version = major_minor,
195+
logger = logger,
196+
),
192197
get_index_urls = get_index_urls,
193198
python_version = major_minor,
194199
logger = logger,
@@ -298,7 +303,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
298303

299304
requirement = select_requirement(
300305
requirements,
301-
platform = repository_platform,
306+
platform = None if pip_attr.download_only else repository_platform,
302307
)
303308
if not requirement:
304309
# Sometimes the package is not present for host platform if there

‎python/private/pypi/parse_requirements.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/parse_requirements.bzl
+17-161Lines changed: 17 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ behavior.
2929
load("//python/private:normalize_name.bzl", "normalize_name")
3030
load(":index_sources.bzl", "index_sources")
3131
load(":parse_requirements_txt.bzl", "parse_requirements_txt")
32-
load(":whl_target_platforms.bzl", "select_whls", "whl_target_platforms")
32+
load(":whl_target_platforms.bzl", "select_whls")
3333

3434
# This includes the vendored _translate_cpu and _translate_os from
3535
# @platforms//host:extension.bzl at version 0.0.9 so that we don't
@@ -80,72 +80,10 @@ DEFAULT_PLATFORMS = [
8080
"windows_x86_64",
8181
]
8282

83-
def _default_platforms(*, filter):
84-
if not filter:
85-
fail("Must specific a filter string, got: {}".format(filter))
86-
87-
if filter.startswith("cp3"):
88-
# TODO @aignas 2024-05-23: properly handle python versions in the filter.
89-
# For now we are just dropping it to ensure that we don't fail.
90-
_, _, filter = filter.partition("_")
91-
92-
sanitized = filter.replace("*", "").replace("_", "")
93-
if sanitized and not sanitized.isalnum():
94-
fail("The platform filter can only contain '*', '_' and alphanumerics")
95-
96-
if "*" in filter:
97-
prefix = filter.rstrip("*")
98-
if "*" in prefix:
99-
fail("The filter can only contain '*' at the end of it")
100-
101-
if not prefix:
102-
return DEFAULT_PLATFORMS
103-
104-
return [p for p in DEFAULT_PLATFORMS if p.startswith(prefix)]
105-
else:
106-
return [p for p in DEFAULT_PLATFORMS if filter in p]
107-
108-
def _platforms_from_args(extra_pip_args):
109-
platform_values = []
110-
111-
for arg in extra_pip_args:
112-
if platform_values and platform_values[-1] == "":
113-
platform_values[-1] = arg
114-
continue
115-
116-
if arg == "--platform":
117-
platform_values.append("")
118-
continue
119-
120-
if not arg.startswith("--platform"):
121-
continue
122-
123-
_, _, plat = arg.partition("=")
124-
if not plat:
125-
_, _, plat = arg.partition(" ")
126-
if plat:
127-
platform_values.append(plat)
128-
else:
129-
platform_values.append("")
130-
131-
if not platform_values:
132-
return []
133-
134-
platforms = {
135-
p.target_platform: None
136-
for arg in platform_values
137-
for p in whl_target_platforms(arg)
138-
}
139-
return list(platforms.keys())
140-
14183
def parse_requirements(
14284
ctx,
14385
*,
14486
requirements_by_platform = {},
145-
requirements_osx = None,
146-
requirements_linux = None,
147-
requirements_lock = None,
148-
requirements_windows = None,
14987
extra_pip_args = [],
15088
get_index_urls = None,
15189
python_version = None,
@@ -158,10 +96,6 @@ def parse_requirements(
15896
requirements_by_platform (label_keyed_string_dict): a way to have
15997
different package versions (or different packages) for different
16098
os, arch combinations.
161-
requirements_osx (label): The requirements file for the osx OS.
162-
requirements_linux (label): The requirements file for the linux OS.
163-
requirements_lock (label): The requirements file for all OSes, or used as a fallback.
164-
requirements_windows (label): The requirements file for windows OS.
16599
extra_pip_args (string list): Extra pip arguments to perform extra validations and to
166100
be joined with args fined in files.
167101
get_index_urls: Callable[[ctx, list[str]], dict], a callable to get all
@@ -186,91 +120,11 @@ def parse_requirements(
186120
187121
The second element is extra_pip_args should be passed to `whl_library`.
188122
"""
189-
if not (
190-
requirements_lock or
191-
requirements_linux or
192-
requirements_osx or
193-
requirements_windows or
194-
requirements_by_platform
195-
):
196-
fail_fn(
197-
"A 'requirements_lock' attribute must be specified, a platform-specific lockfiles " +
198-
"via 'requirements_by_platform' or an os-specific lockfiles must be specified " +
199-
"via 'requirements_*' attributes",
200-
)
201-
return None
202-
203-
platforms = _platforms_from_args(extra_pip_args)
204-
205-
if platforms:
206-
lock_files = [
207-
f
208-
for f in [
209-
requirements_lock,
210-
requirements_linux,
211-
requirements_osx,
212-
requirements_windows,
213-
] + list(requirements_by_platform.keys())
214-
if f
215-
]
216-
217-
if len(lock_files) > 1:
218-
# If the --platform argument is used, check that we are using
219-
# a single `requirements_lock` file instead of the OS specific ones as that is
220-
# the only correct way to use the API.
221-
fail_fn("only a single 'requirements_lock' file can be used when using '--platform' pip argument, consider specifying it via 'requirements_lock' attribute")
222-
return None
223-
224-
files_by_platform = [
225-
(lock_files[0], platforms),
226-
]
227-
else:
228-
files_by_platform = {
229-
file: [
230-
platform
231-
for filter_or_platform in specifier.split(",")
232-
for platform in (_default_platforms(filter = filter_or_platform) if filter_or_platform.endswith("*") else [filter_or_platform])
233-
]
234-
for file, specifier in requirements_by_platform.items()
235-
}.items()
236-
237-
for f in [
238-
# If the users need a greater span of the platforms, they should consider
239-
# using the 'requirements_by_platform' attribute.
240-
(requirements_linux, _default_platforms(filter = "linux_*")),
241-
(requirements_osx, _default_platforms(filter = "osx_*")),
242-
(requirements_windows, _default_platforms(filter = "windows_*")),
243-
(requirements_lock, None),
244-
]:
245-
if f[0]:
246-
files_by_platform.append(f)
247-
248-
configured_platforms = {}
249-
250123
options = {}
251124
requirements = {}
252-
for file, plats in files_by_platform:
253-
if plats:
254-
for p in plats:
255-
if p in configured_platforms:
256-
fail_fn(
257-
"Expected the platform '{}' to be map only to a single requirements file, but got multiple: '{}', '{}'".format(
258-
p,
259-
configured_platforms[p],
260-
file,
261-
),
262-
)
263-
return None
264-
configured_platforms[p] = file
265-
else:
266-
plats = [
267-
p
268-
for p in DEFAULT_PLATFORMS
269-
if p not in configured_platforms
270-
]
271-
for p in plats:
272-
configured_platforms[p] = file
273-
125+
for file, plats in requirements_by_platform.items():
126+
if logger:
127+
logger.debug(lambda: "Using {} for {}".format(file, plats))
274128
contents = ctx.read(file)
275129

276130
# Parse the requirements file directly in starlark to get the information
@@ -303,9 +157,9 @@ def parse_requirements(
303157
tokenized_options.append(p)
304158

305159
pip_args = tokenized_options + extra_pip_args
306-
for p in plats:
307-
requirements[p] = requirements_dict
308-
options[p] = pip_args
160+
for plat in plats:
161+
requirements[plat] = requirements_dict
162+
options[plat] = pip_args
309163

310164
requirements_by_platform = {}
311165
for target_platform, reqs_ in requirements.items():
@@ -325,7 +179,6 @@ def parse_requirements(
325179
requirement_line = requirement_line,
326180
target_platforms = [],
327181
extra_pip_args = extra_pip_args,
328-
download = len(platforms) > 0,
329182
),
330183
)
331184
for_req.target_platforms.append(target_platform)
@@ -353,12 +206,12 @@ def parse_requirements(
353206
for p in r.target_platforms:
354207
requirement_target_platforms[p] = None
355208

356-
is_exposed = len(requirement_target_platforms) == len(configured_platforms)
209+
is_exposed = len(requirement_target_platforms) == len(requirements)
357210
if not is_exposed and logger:
358-
logger.debug(lambda: "Package {} will not be exposed because it is only present on a subset of platforms: {} out of {}".format(
211+
logger.debug(lambda: "Package '{}' will not be exposed because it is only present on a subset of platforms: {} out of {}".format(
359212
whl_name,
360213
sorted(requirement_target_platforms),
361-
sorted(configured_platforms),
214+
sorted(requirements),
362215
))
363216

364217
for r in sorted(reqs.values(), key = lambda r: r.requirement_line):
@@ -376,13 +229,15 @@ def parse_requirements(
376229
requirement_line = r.requirement_line,
377230
target_platforms = sorted(r.target_platforms),
378231
extra_pip_args = r.extra_pip_args,
379-
download = r.download,
380232
whls = whls,
381233
sdist = sdist,
382234
is_exposed = is_exposed,
383235
),
384236
)
385237

238+
if logger:
239+
logger.debug(lambda: "Will configure whl repos: {}".format(ret.keys()))
240+
386241
return ret
387242

388243
def select_requirement(requirements, *, platform):
@@ -391,8 +246,9 @@ def select_requirement(requirements, *, platform):
391246
Args:
392247
requirements (list[struct]): The list of requirements as returned by
393248
the `parse_requirements` function above.
394-
platform (str): The host platform. Usually an output of the
395-
`host_platform` function.
249+
platform (str or None): The host platform. Usually an output of the
250+
`host_platform` function. If None, then this function will return
251+
the first requirement it finds.
396252
397253
Returns:
398254
None if not found or a struct returned as one of the values in the
@@ -402,7 +258,7 @@ def select_requirement(requirements, *, platform):
402258
maybe_requirement = [
403259
req
404260
for req in requirements
405-
if platform in req.target_platforms or req.download
261+
if not platform or [p for p in req.target_platforms if p.endswith(platform)]
406262
]
407263
if not maybe_requirement:
408264
# Sometimes the package is not present for host platform if there

‎python/private/pypi/pip_repository.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/pip_repository.bzl
+10-6Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ load("//python/private:text_util.bzl", "render")
2121
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
2222
load(":pip_repository_attrs.bzl", "ATTRS")
2323
load(":render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
24+
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
2425

2526
def _get_python_interpreter_attr(rctx):
2627
"""A helper function for getting the `python_interpreter` attribute or it's default
@@ -71,11 +72,14 @@ exports_files(["requirements.bzl"])
7172
def _pip_repository_impl(rctx):
7273
requirements_by_platform = parse_requirements(
7374
rctx,
74-
requirements_by_platform = rctx.attr.requirements_by_platform,
75-
requirements_linux = rctx.attr.requirements_linux,
76-
requirements_lock = rctx.attr.requirements_lock,
77-
requirements_osx = rctx.attr.requirements_darwin,
78-
requirements_windows = rctx.attr.requirements_windows,
75+
requirements_by_platform = requirements_files_by_platform(
76+
requirements_by_platform = rctx.attr.requirements_by_platform,
77+
requirements_linux = rctx.attr.requirements_linux,
78+
requirements_lock = rctx.attr.requirements_lock,
79+
requirements_osx = rctx.attr.requirements_darwin,
80+
requirements_windows = rctx.attr.requirements_windows,
81+
extra_pip_args = rctx.attr.extra_pip_args,
82+
),
7983
extra_pip_args = rctx.attr.extra_pip_args,
8084
)
8185
selected_requirements = {}
@@ -84,7 +88,7 @@ def _pip_repository_impl(rctx):
8488
for name, requirements in requirements_by_platform.items():
8589
r = select_requirement(
8690
requirements,
87-
platform = repository_platform,
91+
platform = None if rctx.attr.download_only else repository_platform,
8892
)
8993
if not r:
9094
continue

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.