Skip to content

Commit d5075c3

Browse files
committed
feat(pypi): incrementally build platform configuration
Before this PR the configuration for platforms would be built non-incrementally, making it harder for users to override particular attributes of the already configured ones. With this PR the new features introduced in #3058 will be easier to override. Work towards #2747
1 parent 9792058 commit d5075c3

File tree

2 files changed

+126
-37
lines changed

2 files changed

+126
-37
lines changed

python/private/pypi/extension.bzl

Lines changed: 75 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -380,24 +380,88 @@ def _whl_repo(*, src, whl_library_args, is_multiple_versions, download_only, net
380380
def _configure(config, *, platform, os_name, arch_name, config_settings, env = {}, override = False):
381381
"""Set the value in the config if the value is provided"""
382382
config.setdefault("platforms", {})
383-
if platform:
383+
if platform and (os_name or arch_name or config_settings or env):
384384
if not override and config.get("platforms", {}).get(platform):
385385
return
386386

387387
for key in env:
388388
if key not in _SUPPORTED_PEP508_KEYS:
389389
fail("Unsupported key in the PEP508 environment: {}".format(key))
390390

391-
config["platforms"][platform] = struct(
392-
name = platform.replace("-", "_").lower(),
393-
os_name = os_name,
394-
arch_name = arch_name,
395-
config_settings = config_settings,
396-
env = env,
397-
)
398-
else:
391+
config["platforms"].setdefault(platform, {})
392+
for key, value in {
393+
"arch_name": arch_name,
394+
"config_settings": config_settings,
395+
"env": env,
396+
"name": platform.replace("-", "_").lower(),
397+
"os_name": os_name,
398+
}.items():
399+
if not value:
400+
continue
401+
402+
if not override and config.get(key):
403+
continue
404+
405+
config["platforms"][platform][key] = value
406+
elif platform and override:
399407
config["platforms"].pop(platform)
400408

409+
def _plat(*, name, arch_name, os_name, config_settings = [], env = {}):
410+
return struct(
411+
name = name,
412+
arch_name = arch_name,
413+
os_name = os_name,
414+
config_settings = config_settings,
415+
env = env,
416+
)
417+
418+
def build_config(
419+
*,
420+
module_ctx,
421+
enable_pipstar):
422+
"""Parse 'configure' and 'default' extension tags
423+
424+
Args:
425+
module_ctx: {type}`module_ctx` module context.
426+
enable_pipstar: {type}`bool` a flag to enable dropping Python dependency for
427+
evaluation of the extension.
428+
429+
Returns:
430+
A struct with the configuration.
431+
"""
432+
defaults = {
433+
"platforms": {},
434+
}
435+
for mod in module_ctx.modules:
436+
if not (mod.is_root or mod.name == "rules_python"):
437+
continue
438+
439+
for tag in mod.tags.default:
440+
_configure(
441+
defaults,
442+
arch_name = tag.arch_name,
443+
config_settings = tag.config_settings,
444+
env = tag.env,
445+
os_name = tag.os_name,
446+
platform = tag.platform,
447+
override = mod.is_root,
448+
# TODO @aignas 2025-05-19: add more attr groups:
449+
# * for AUTH - the default `netrc` usage could be configured through a common
450+
# attribute.
451+
# * for index/downloader config. This includes all of those attributes for
452+
# overrides, etc. Index overrides per platform could be also used here.
453+
# * for whl selection - selecting preferences of which `platform_tag`s we should use
454+
# for what. We could also model the `cp313t` freethreaded as separate platforms.
455+
)
456+
457+
return struct(
458+
platforms = {
459+
name: _plat(**values)
460+
for name, values in defaults["platforms"].items()
461+
},
462+
enable_pipstar = enable_pipstar,
463+
)
464+
401465
def parse_modules(
402466
module_ctx,
403467
_fail = fail,
@@ -448,33 +512,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
448512
srcs_exclude_glob = whl_mod.srcs_exclude_glob,
449513
)
450514

451-
defaults = {
452-
"enable_pipstar": enable_pipstar,
453-
"platforms": {},
454-
}
455-
for mod in module_ctx.modules:
456-
if not (mod.is_root or mod.name == "rules_python"):
457-
continue
458-
459-
for tag in mod.tags.default:
460-
_configure(
461-
defaults,
462-
arch_name = tag.arch_name,
463-
config_settings = tag.config_settings,
464-
env = tag.env,
465-
os_name = tag.os_name,
466-
platform = tag.platform,
467-
override = mod.is_root,
468-
# TODO @aignas 2025-05-19: add more attr groups:
469-
# * for AUTH - the default `netrc` usage could be configured through a common
470-
# attribute.
471-
# * for index/downloader config. This includes all of those attributes for
472-
# overrides, etc. Index overrides per platform could be also used here.
473-
# * for whl selection - selecting preferences of which `platform_tag`s we should use
474-
# for what. We could also model the `cp313t` freethreaded as separate platforms.
475-
)
476-
477-
config = struct(**defaults)
515+
config = build_config(module_ctx = module_ctx, enable_pipstar = enable_pipstar)
478516

479517
# TODO @aignas 2025-06-03: Merge override API with the builder?
480518
_overriden_whl_set = {}
@@ -659,6 +697,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
659697
k: dict(sorted(args.items()))
660698
for k, args in sorted(whl_libraries.items())
661699
},
700+
config = config,
662701
)
663702

664703
def _pip_impl(module_ctx):

tests/pypi/extension/extension_tests.bzl

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
load("@rules_testing//lib:test_suite.bzl", "test_suite")
1818
load("@rules_testing//lib:truth.bzl", "subjects")
19-
load("//python/private/pypi:extension.bzl", "parse_modules") # buildifier: disable=bzl-visibility
19+
load("//python/private/pypi:extension.bzl", "build_config", "parse_modules") # buildifier: disable=bzl-visibility
2020
load("//python/private/pypi:parse_simpleapi_html.bzl", "parse_simpleapi_html") # buildifier: disable=bzl-visibility
2121
load("//python/private/pypi:whl_config_setting.bzl", "whl_config_setting") # buildifier: disable=bzl-visibility
2222

@@ -92,6 +92,18 @@ def _parse_modules(env, enable_pipstar = 0, **kwargs):
9292
),
9393
)
9494

95+
def _build_config(env, enable_pipstar = 0, **kwargs):
96+
return env.expect.that_struct(
97+
build_config(
98+
enable_pipstar = enable_pipstar,
99+
**kwargs
100+
),
101+
attrs = dict(
102+
platforms = subjects.dict,
103+
enable_pipstar = subjects.bool,
104+
),
105+
)
106+
95107
def _default(
96108
arch_name = None,
97109
config_settings = None,
@@ -1206,6 +1218,44 @@ optimum[onnxruntime-gpu]==1.17.1 ; sys_platform == 'linux'
12061218

12071219
_tests.append(_test_pipstar_platforms)
12081220

1221+
def _test_build_pipstar_platform(env):
1222+
config = _build_config(
1223+
env,
1224+
module_ctx = _mock_mctx(
1225+
_mod(
1226+
name = "rules_python",
1227+
default = [
1228+
_default(
1229+
platform = "myplat",
1230+
os_name = "linux",
1231+
arch_name = "x86_64",
1232+
config_settings = [
1233+
"@platforms//os:linux",
1234+
"@platforms//cpu:x86_64",
1235+
],
1236+
),
1237+
_default(),
1238+
],
1239+
),
1240+
),
1241+
enable_pipstar = True,
1242+
)
1243+
config.enable_pipstar().equals(True)
1244+
config.platforms().contains_exactly({
1245+
"myplat": struct(
1246+
name = "myplat",
1247+
os_name = "linux",
1248+
arch_name = "x86_64",
1249+
config_settings = [
1250+
"@platforms//os:linux",
1251+
"@platforms//cpu:x86_64",
1252+
],
1253+
env = {},
1254+
),
1255+
})
1256+
1257+
_tests.append(_test_build_pipstar_platform)
1258+
12091259
def extension_test_suite(name):
12101260
"""Create the test suite.
12111261

0 commit comments

Comments
 (0)