Skip to content

Commit 91f86a9

Browse files
committed
poc of accessing deps via the hub repo
1 parent 5dc029a commit 91f86a9

File tree

5 files changed

+53
-35
lines changed

5 files changed

+53
-35
lines changed

python/pip_install/pip_repository.bzl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,7 @@ def _whl_library_impl(rctx):
858858
],
859859
entry_points = entry_points,
860860
annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))),
861+
use_hub = rctx.attr.experimental_use_hub_for_deps,
861862
)
862863
rctx.file("BUILD.bazel", build_file_contents)
863864

@@ -899,6 +900,9 @@ whl_library_attrs = {
899900
),
900901
allow_files = True,
901902
),
903+
"experimental_use_hub_for_deps": attr.string(
904+
default = "",
905+
),
902906
"group_deps": attr.string_list(
903907
doc = "List of dependencies to skip in order to break the cycles within a dependency group.",
904908
default = [],

python/pip_install/private/generate_whl_library_build_bazel.bzl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ def generate_whl_library_build_bazel(
232232
entry_points,
233233
annotation = None,
234234
group_name = None,
235-
group_deps = []):
235+
group_deps = [],
236+
use_hub = False):
236237
"""Generate a BUILD file for an unzipped Wheel
237238
238239
Args:
@@ -250,6 +251,7 @@ def generate_whl_library_build_bazel(
250251
group_deps: List[str]; names of fellow members of the group (if any). These will be excluded
251252
from generated deps lists so as to avoid direct cycles. These dependencies will be provided
252253
at runtime by the group rules which wrap this library and its fellows together.
254+
use_hub: str; the hub to use instead of spoke repos when pointing to dependencies.
253255
254256
Returns:
255257
A complete BUILD file as a string
@@ -338,13 +340,13 @@ def generate_whl_library_build_bazel(
338340
lib_dependencies = _render_list_and_select(
339341
deps = dependencies,
340342
deps_by_platform = dependencies_by_platform,
341-
tmpl = "@{}{{}}//:{}".format(repo_prefix, PY_LIBRARY_PUBLIC_LABEL),
343+
tmpl = "@{}//{{}}".format(use_hub) if use_hub else "@{}{{}}//:{}".format(repo_prefix, PY_LIBRARY_PUBLIC_LABEL),
342344
)
343345

344346
whl_file_deps = _render_list_and_select(
345347
deps = dependencies,
346348
deps_by_platform = dependencies_by_platform,
347-
tmpl = "@{}{{}}//:{}".format(repo_prefix, WHEEL_FILE_PUBLIC_LABEL),
349+
tmpl = "@{}//{{}}:{}".format(use_hub, WHEEL_FILE_PUBLIC_LABEL) if use_hub else "@{}{{}}//:{}".format(repo_prefix, WHEEL_FILE_PUBLIC_LABEL),
348350
)
349351

350352
# If this library is a member of a group, its public label aliases need to

python/private/bzlmod/pip.bzl

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -121,33 +121,27 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
121121
))
122122
python_interpreter_target = INTERPRETER_LABELS[python_name]
123123

124-
if pip_attr.experimental_whl_platform:
125-
pip_name = "{}_{}_{}".format(
126-
hub_name,
127-
pip_attr.experimental_whl_platform,
128-
version_label(pip_attr.python_version),
129-
)
130-
else:
131-
pip_name = "{}_{}".format(
132-
hub_name,
133-
version_label(pip_attr.python_version),
134-
)
124+
pip_name = "{}_{}".format(
125+
hub_name,
126+
version_label(pip_attr.python_version),
127+
)
135128

136129
major_minor = _major_minor_version(pip_attr.python_version)
137130
target_platforms = pip_attr.experimental_target_platforms
138131
platform_config_settings = []
132+
default_version = _major_minor_version(DEFAULT_PYTHON_VERSION)
133+
139134
if pip_attr.experimental_whl_platform:
140135
# NOTE @aignas 2024-02-16: Right now the download is extremely inefficient, because
141136
# we have to download wheels for the target platform from scratch even though some
142137
# of the wheels for the target platform could already be reused because they are
143138
# multi-platform.
144139
#
145-
# In order to improve this we would have to make `whl_library` support accessing its
146-
# deps via the hub_repo aliases (so that it references `@pip//foo` and does not need
147-
# to know if `foo` is target_platform specific or not. One more thing would be to
148-
# somehow know in advance the filename of the `whl` for the `whl_library` that we are
149-
# going to create. That way we could create only a single `whl_library` for all python
150-
# versions/platforms at once, thus simplifying the hub repo layout.
140+
# In order to improve this we would have to somehow know in advance the
141+
# filename of the `whl` for the `whl_library` that we are going to
142+
# create. That way we could create only a single `whl_library` for all
143+
# python versions/platforms at once, thus simplifying the hub repo
144+
# layout.
151145
requirements_lock = pip_attr.requirements_lock
152146
if not requirements_lock or pip_attr.requirements_windows or pip_attr.requirements_darwin or pip_attr.requirements_linux:
153147
fail("only requirements_lock can be specified when platform is used")
@@ -178,7 +172,6 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
178172
# Handle default version matching as well, so that we could use the
179173
# platform specific wheels if they match the right platform without
180174
# transitioning to a specific python version.
181-
default_version = _major_minor_version(DEFAULT_PYTHON_VERSION)
182175
if default_version == major_minor:
183176
platform_config_setting_name = "is_{}_{}".format(
184177
platforms[0].os,
@@ -195,13 +188,7 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
195188
)
196189
platform_config_settings.append("//:" + platform_config_setting_name)
197190

198-
target_platforms = [
199-
"cp{}_{}_{}".format(
200-
version_label(major_minor),
201-
platforms[0].os,
202-
platforms[0].cpu,
203-
),
204-
]
191+
target_platforms = ["cp{}_{}_{}".format(version_label(major_minor), platforms[0].os, platforms[0].cpu)]
205192
else:
206193
platform_config_setting_name = "is_python_{}".format(major_minor)
207194

@@ -214,6 +201,9 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
214201
visibility = ["//:__subpackages__"],
215202
)
216203
platform_config_settings.append("//:" + platform_config_setting_name)
204+
if default_version == major_minor:
205+
platform_config_settings.append("//conditions:default")
206+
config_settings_map[hub_name]["//conditions:default"] = "//conditions:default"
217207
requirements_lock = locked_requirements_label(module_ctx, pip_attr)
218208

219209
# Parse the requirements file directly in starlark to get the information
@@ -261,7 +251,7 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
261251
for whl_name in group_whls
262252
}
263253

264-
group_repo = "%s__groups" % (pip_name,)
254+
group_repo = "%s__%s__groups" % (pip_name, pip_attr.experimental_whl_platform)
265255
group_library(
266256
name = group_repo,
267257
repo_prefix = pip_name + "_",
@@ -279,17 +269,24 @@ def _create_whl_repos(*, module_ctx, pip_attr, whl_map, whl_overrides, config_se
279269
group_deps = requirement_cycles.get(group_name, [])
280270

281271
repo_name = "{}_{}".format(pip_name, whl_name)
272+
if pip_attr.experimental_whl_platform:
273+
repo_name = "{}__{}".format(repo_name, pip_attr.experimental_whl_platform)
274+
whl_pip_name = "{}_{}".format(pip_name, pip_attr.experimental_whl_platform)
275+
else:
276+
whl_pip_name = pip_name
277+
282278
whl_library(
283279
name = repo_name,
284280
requirement = requirement_line,
285-
repo = pip_name,
286-
repo_prefix = pip_name + "_",
281+
repo = whl_pip_name,
282+
repo_prefix = whl_pip_name + "_",
287283
annotation = annotation,
288284
whl_patches = {
289285
p: json.encode(args)
290286
for p, args in whl_overrides.get(whl_name, {}).items()
291287
},
292288
experimental_target_platforms = target_platforms,
289+
experimental_use_hub_for_deps = hub_name,
293290
python_interpreter = pip_attr.python_interpreter,
294291
python_interpreter_target = python_interpreter_target,
295292
quiet = pip_attr.quiet,
@@ -481,7 +478,14 @@ def _pip_impl(module_ctx):
481478

482479
for hub_name, whl_map in hub_whl_map.items():
483480
build_file_contents = BUILD_FILE_CONTENTS
484-
config_settings = config_settings_map.get(hub_name, {}).values()
481+
has_default = False
482+
config_settings = []
483+
for value in config_settings_map.get(hub_name, {}).values():
484+
if value == "//conditions:default":
485+
has_default = True
486+
continue
487+
config_settings.append(value)
488+
485489
if config_settings:
486490
self, _, _ = str(Label("//:MODULE.bazel")).partition("//")
487491
build_file_contents = "\n".join([
@@ -501,6 +505,7 @@ def _pip_impl(module_ctx):
501505
for key, value in whl_map.items()
502506
},
503507
default_version = _major_minor_version(DEFAULT_PYTHON_VERSION),
508+
render_default_version = not has_default,
504509
)
505510

506511
def _pip_parse_ext_attrs():

python/private/bzlmod/pip_repository.bzl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def _pip_repository_impl(rctx):
3131
key: [whl_alias(**v) for v in json.decode(values)]
3232
for key, values in rctx.attr.whl_map.items()
3333
},
34-
default_version = rctx.attr.default_version,
34+
default_version = rctx.attr.default_version if rctx.attr.render_default_version else None,
3535
)
3636
for path, contents in aliases.items():
3737
rctx.file(path, contents)
@@ -72,6 +72,9 @@ This is the default python version in the format of X.Y. This should match
7272
what is setup by the 'python' extension using the 'is_default = True'
7373
setting.""",
7474
),
75+
"render_default_version": attr.bool(
76+
default = True,
77+
),
7578
"repo_name": attr.string(
7679
mandatory = True,
7780
doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.",

python/private/render_pkg_aliases.bzl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def _render_whl_library_alias(
5757
selects = {}
5858
no_match_error = "_NO_MATCH_ERROR"
5959
default = None
60-
for alias in sorted(aliases, key = lambda x: x.version):
60+
for alias in sorted(aliases, key = lambda x: x.config_setting):
6161
actual = "@{repo}//:{name}".format(repo = alias.repo, name = name)
6262
selects[alias.config_setting] = actual
6363
if alias.version == default_version:
@@ -81,8 +81,12 @@ def _render_common_aliases(*, name, aliases, default_version = None):
8181
]
8282

8383
versions = None
84+
has_default = False
8485
if aliases:
85-
versions = sorted([v.version for v in aliases if v.version])
86+
versions = sorted([a.version for a in aliases if a.version])
87+
has_default = len([a.config_setting for a in aliases if a.config_setting == "//conditions:default"]) == 1
88+
if has_default:
89+
default_version = None
8690

8791
if not versions or default_version in versions:
8892
pass

0 commit comments

Comments
 (0)