Skip to content

Commit 8a10382

Browse files
committed
feat: reuse extracted whl srcs within a hub repository
This is the common denominator and for now no warnings are printed, but there are opportunities to do this. This approach is way more surgical than the previous one. Fixes #2948
1 parent 0c23701 commit 8a10382

3 files changed

Lines changed: 165 additions & 17 deletions

File tree

python/private/pypi/extension.bzl

Lines changed: 148 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ load(":pypi_cache.bzl", "pypi_cache")
3232
load(":simpleapi_download.bzl", "simpleapi_download")
3333
load(":unified_hub_repo.bzl", "unified_hub_repo")
3434
load(":whl_library.bzl", "whl_library")
35+
load(":whl_archive.bzl", "whl_archive")
36+
load(":whl_deps_repo.bzl", "whl_deps_repo")
37+
load(":whl_repo_name.bzl", "whl_repo_name")
38+
load(":pep508_requirement.bzl", "requirement")
3539

3640
def _whl_mods_impl(whl_mods_dict):
3741
"""Implementation of the pip.whl_mods tag class.
@@ -537,6 +541,149 @@ def _create_unified_hub_repo(mods):
537541
packages = packages,
538542
)
539543

544+
def register_whl_libraries(
545+
registrations,
546+
rules = struct(
547+
whl_library=whl_library,
548+
whl_deps_repo=whl_deps_repo,
549+
whl_archive=whl_archive,
550+
),
551+
):
552+
"""Register all of the whl libraries that where produced by the extension.
553+
554+
Args:
555+
registrations: {type}`dict[str, dict[str, Any]]` The args to pass to
556+
the repository rules when processing everything.
557+
rules: {type}`struct` used to inject different implementations used to
558+
create instances. Used for testing.
559+
"""
560+
# we first check which wheels are ambiguous - belong to the same hub repository, indicated
561+
# by the `dep_template`, but have different (URL, index_url) pairs for each whl.
562+
urls = {}
563+
fallback = {}
564+
separate_srcs = {}
565+
for name, args in registrations.items():
566+
if "urls" not in args or "filename" not in args:
567+
# only enabled with bazel downloader
568+
continue
569+
570+
if "annotation" in args or "whl_patches" in args:
571+
# No support for patched repos yet
572+
continue
573+
574+
# For each each integrity, filename, we should have a unique url and index_url
575+
# used across all of the hub repos, if not, print a warning and fallback to the
576+
# previous behaviour of not using the source repository.
577+
#
578+
# If the filename is the same but it has a different sha256, then it means that
579+
# there could be one of the following situations:
580+
# 1. dependency confusion.
581+
# 2. somebody patched the wheel.
582+
# 3. it is an unlucky coincidence.
583+
#
584+
# Any of these cases are a bit tricky to get right, the repo name is derived
585+
# filename together. The repo name is derived from the filename and a hash for
586+
# this reason.
587+
integrity = args["integrity"]
588+
filename = args["filename"]
589+
590+
url = args["urls"][0]
591+
index_url = args["index_url"]
592+
593+
hub_name, _, _ = args["dep_template"].partition("//")
594+
hub_name = hub_name.strip("@")
595+
596+
# for now we include the hub_name in the key since the reuse is done for each
597+
# hub_name separately.
598+
key = (hub_name, filename, integrity)
599+
value = (url, index_url)
600+
601+
if (filename, integrity) in fallback:
602+
continue
603+
604+
got = urls.setdefault(key, value)
605+
if got != value:
606+
urls.pop(key)
607+
fallback.setdefault((filename, integrity), None)
608+
609+
separate_srcs[name] = key
610+
611+
for name, args in registrations.items():
612+
if name in separate_srcs:
613+
# first handle the regular case
614+
continue
615+
616+
rules.whl_library(name = name, **args)
617+
618+
# Then handle the reusable case
619+
registered_srcs = {}
620+
for name, key in separate_srcs.items():
621+
args = registrations[name]
622+
hub_name, _, integrity = key
623+
624+
filename = args["filename"]
625+
extract_args = {
626+
k: v
627+
for k, v in args.items()
628+
if k not in {
629+
"config_load": None,
630+
"dep_template": None,
631+
}
632+
}
633+
634+
# The extras do not affect the extraction, so normalize the requirement
635+
# to allow the same wheel to be extracted only once.
636+
extract_args["requirement"] = _without_extras(extract_args["requirement"])
637+
extract_repo_name = "{}_{}".format(
638+
hub_name,
639+
whl_repo_name(filename, ""),
640+
)
641+
if key not in registered_srcs:
642+
registered_srcs[key] = extract_repo_name
643+
rules.whl_archive(
644+
name = extract_repo_name,
645+
**extract_args,
646+
)
647+
648+
deps_args = {
649+
k: args.get(k)
650+
for k in [
651+
"config_load",
652+
"requirement",
653+
"dep_template",
654+
"group_deps",
655+
"group_name",
656+
"repo_prefix",
657+
]
658+
if args.get(k) != None
659+
} | {
660+
"metadata_file": "@{}//:metadata.json".format(extract_repo_name),
661+
}
662+
rules.whl_deps_repo(
663+
name = name,
664+
**deps_args
665+
)
666+
667+
def _without_extras(requirement_line):
668+
"""Remove the extras from a requirement line.
669+
670+
The extras do not affect which wheel is downloaded and extracted, so they
671+
can be removed to allow the same wheel to be extracted only once even when
672+
it is referenced with different extras.
673+
674+
Args:
675+
requirement_line: {type}`str` the requirement line, e.g.
676+
`foo[bar]==1.0 --hash=sha256:...`.
677+
678+
Returns:
679+
The requirement line without the extras, e.g. `foo==1.0 --hash=sha256:...`.
680+
"""
681+
name_and_version, _, extras = requirement_line.partition("[")
682+
if not extras:
683+
return requirement_line
684+
_, _, rest = extras.partition("]")
685+
return name_and_version + rest
686+
540687
def _pip_impl(module_ctx):
541688
"""Implementation of a class tag that creates the pip hub and corresponding pip spoke whl repositories.
542689
@@ -611,8 +758,7 @@ def _pip_impl(module_ctx):
611758
# Build all of the wheel modifications if the tag class is called.
612759
_whl_mods_impl(mods.whl_mods)
613760

614-
for name, args in mods.whl_libraries.items():
615-
whl_library(name = name, **args)
761+
register_whl_libraries(registrations = mods.whl_libraries)
616762

617763
for hub_name, whl_map in mods.hub_whl_map.items():
618764
hub_repository(

python/private/pypi/whl_library_deps_targets.bzl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ def whl_library_deps_targets(
2323
repo,
2424
aliases = None,
2525
metadata_name,
26-
requires_dist,
27-
extras,
26+
requires_dist = [],
27+
extras = [],
2828
include = [],
2929
group_deps = [],
3030
group_name = None,
@@ -168,6 +168,7 @@ def whl_library_deps_targets(
168168
package_deps = package_deps,
169169
tmpl = dep_template.format(name = "{}", target = PY_LIBRARY_PUBLIC_LABEL),
170170
),
171+
precompile = "disabled",
171172
tags = tags,
172173
visibility = impl_vis,
173174
)

python/private/pypi/whl_library_targets.bzl

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,17 @@ def whl_library_targets(
8686
**kwargs
8787
)
8888

89-
whl_library_deps_targets(
90-
name = name,
91-
metadata_name = metadata_name,
92-
requires_dist = requires_dist,
93-
dep_template = dep_template, # only needed if requires_dist or group_name is present
94-
group_deps = group_deps, # only needed if group_name is present
95-
group_name = group_name, # must specify group_deps together
96-
extras = extras, # only needed if requires_dist is present
97-
include = include, # only needed if requires_dist is present
98-
repo = None, # set aliases in the same repo
99-
aliases = {},
100-
**kwargs
101-
)
89+
if dep_template:
90+
whl_library_deps_targets(
91+
name = name,
92+
metadata_name = metadata_name,
93+
requires_dist = requires_dist,
94+
dep_template = dep_template, # only needed if requires_dist or group_name is present
95+
group_deps = group_deps, # only needed if group_name is present
96+
group_name = group_name, # must specify group_deps together
97+
extras = extras, # only needed if requires_dist is present
98+
include = include, # only needed if requires_dist is present
99+
repo = None, # set aliases in the same repo
100+
aliases = {},
101+
**kwargs
102+
)

0 commit comments

Comments
 (0)