From efcc4988be0ab9ca829223c2a6ab3a1ce3622a17 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Fri, 19 Jul 2024 13:37:40 +0900 Subject: [PATCH 1/9] fix the logger to work with module_ctx --- python/private/pypi/extension.bzl | 2 +- python/private/repo_utils.bzl | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 95526e4252..48ae1c51c3 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -99,7 +99,7 @@ You cannot use both the additive_build_content and additive_build_content_file a ) def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, simpleapi_cache, exposed_packages): - logger = repo_utils.logger(module_ctx) + logger = repo_utils.logger(module_ctx, "pypi:create_whl_repos") python_interpreter_target = pip_attr.python_interpreter_target is_hub_reproducible = True diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index 18937895f5..cdfb322f1c 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -42,22 +42,23 @@ def _debug_print(rctx, message_cb): if _is_repo_debug_enabled(rctx): print(message_cb()) # buildifier: disable=print -def _logger(rctx): +def _logger(ctx, name = None): """Creates a logger instance for printing messages. Args: - rctx: repository_ctx object. If the attribute `_rule_name` is - present, it will be included in log messages. + ctx: repository_ctx or module_ctx object. If the attribute + `_rule_name` is present, it will be included in log messages. + name: name for the logger. Optional for repository_ctx usage. Returns: A struct with attributes logging: trace, debug, info, warn, fail. """ - if _is_repo_debug_enabled(rctx): + if _is_repo_debug_enabled(ctx): verbosity_level = "DEBUG" else: verbosity_level = "WARN" - env_var_verbosity = rctx.os.environ.get(REPO_VERBOSITY_ENV_VAR) + env_var_verbosity = ctx.os.environ.get(REPO_VERBOSITY_ENV_VAR) verbosity_level = env_var_verbosity or verbosity_level verbosity = { @@ -66,18 +67,25 @@ def _logger(rctx): "TRACE": 3, }.get(verbosity_level, 0) + if hasattr(ctx, "attr"): + # This is `repository_ctx`. + name = name or "{}(@@{})".format(getattr(ctx.attr, "_rule_name", "?"), ctx.name) + elif not name: + fail("The name has to be specified when using the logger with `module_ctx`") + + name = getattr(ctx, "name", "extension") + def _log(enabled_on_verbosity, level, message_cb_or_str): if verbosity < enabled_on_verbosity: return - rule_name = getattr(rctx.attr, "_rule_name", "?") + if type(message_cb_or_str) == "string": message = message_cb_or_str else: message = message_cb_or_str() - print("\nrules_python:{}(@@{}) {}:".format( - rule_name, - rctx.name, + print("\nrules_python:{} {}:".format( + name, level.upper(), ), message) # buildifier: disable=print From b1d1277501e5bf481866b681870eda94ae4ccd17 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:01:53 +0900 Subject: [PATCH 2/9] fix the bug --- python/private/pypi/whl_target_platforms.bzl | 28 ++++++-- .../whl_target_platforms/select_whl_tests.bzl | 72 +++++++++---------- 2 files changed, 60 insertions(+), 40 deletions(-) diff --git a/python/private/pypi/whl_target_platforms.bzl b/python/private/pypi/whl_target_platforms.bzl index bee795732a..68c73d7c36 100644 --- a/python/private/pypi/whl_target_platforms.bzl +++ b/python/private/pypi/whl_target_platforms.bzl @@ -46,15 +46,14 @@ _OS_PREFIXES = { "win": "windows", } # buildifier: disable=unsorted-dict-items -def select_whls(*, whls, want_python_version = "3.0", want_abis = [], want_platforms = [], logger = None): +def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logger = None): """Select a subset of wheels suitable for target platforms from a list. Args: whls(list[struct]): A list of candidates which have a `filename` attribute containing the `whl` filename. want_python_version(str): An optional parameter to filter whls by python version. Defaults to '3.0'. - want_abis(list[str]): A list of ABIs that are supported. - want_platforms(str): The platforms + want_platforms(str): The platforms in "{abi}_{os}_{cpu}" or "{os}_{cpu}" format. logger: A logger for printing diagnostic messages. Returns: @@ -64,6 +63,27 @@ def select_whls(*, whls, want_python_version = "3.0", want_abis = [], want_platf if not whls: return [] + want_abis = { + "abi3": None, + "none": None, + } + + _want_platforms = {} + for p in want_platforms: + if not p.startswith("cp3"): + fail("expected all platforms to start with ABI, but got: {}".format(p)) + + abi, _, os_cpu = p.partition("_") + _want_platforms[os_cpu] = None + _want_platforms[p] = None + + # For some legacy implementations the wheels may target the `cp3xm` ABI + _want_platforms["{}m_{}".format(abi, os_cpu)] = None + want_abis[abi] = None + want_abis[abi + "m"] = None + + want_platforms = sorted(_want_platforms) + version_limit = -1 if want_python_version: version_limit = int(want_python_version.split(".")[1]) @@ -110,7 +130,7 @@ def select_whls(*, whls, want_python_version = "3.0", want_abis = [], want_platf if parsed.platform_tag == "any": compatible = True else: - for p in whl_target_platforms(parsed.platform_tag): + for p in whl_target_platforms(parsed.platform_tag, abi_tag = parsed.abi_tag.strip("m") if parsed.abi_tag.startswith("cp") else None): if p.target_platform in want_platforms: compatible = True break diff --git a/tests/pypi/whl_target_platforms/select_whl_tests.bzl b/tests/pypi/whl_target_platforms/select_whl_tests.bzl index 3fd80e3ee3..a848767d92 100644 --- a/tests/pypi/whl_target_platforms/select_whl_tests.bzl +++ b/tests/pypi/whl_target_platforms/select_whl_tests.bzl @@ -15,6 +15,7 @@ "" load("@rules_testing//lib:test_suite.bzl", "test_suite") +load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "REPO_VERBOSITY_ENV_VAR", "repo_utils") # buildifier: disable=bzl-visibility load("//python/private/pypi:whl_target_platforms.bzl", "select_whls") # buildifier: disable=bzl-visibility WHL_LIST = [ @@ -79,7 +80,7 @@ def _match(env, got, *want_filenames): # Check that we pass the original structs env.expect.that_str(got[0].other).equals("dummy") -def _select_whls(whls, **kwargs): +def _select_whls(whls, debug = False, **kwargs): return select_whls( whls = [ struct( @@ -88,6 +89,14 @@ def _select_whls(whls, **kwargs): ) for f in whls ], + logger = repo_utils.logger(struct( + os = struct( + environ = { + REPO_DEBUG_ENV_VAR: "1", + REPO_VERBOSITY_ENV_VAR: "TRACE" if debug else "INFO", + }, + ), + ), "unit-test"), **kwargs ) @@ -100,35 +109,17 @@ def _test_simplest(env): "pkg-0.0.1-py3-abi3-any.whl", "pkg-0.0.1-py3-none-any.whl", ], - want_abis = ["none"], - want_platforms = ["ignored"], + want_platforms = ["cp3x_ignored"], ) _match( env, got, + "pkg-0.0.1-py3-abi3-any.whl", "pkg-0.0.1-py3-none-any.whl", ) _tests.append(_test_simplest) -def _test_select_abi3(env): - got = _select_whls( - whls = [ - "pkg-0.0.1-py2.py3-abi3-any.whl", - "pkg-0.0.1-py3-abi3-any.whl", - "pkg-0.0.1-py3-none-any.whl", - ], - want_abis = ["abi3"], - want_platforms = ["ignored"], - ) - _match( - env, - got, - "pkg-0.0.1-py3-abi3-any.whl", - ) - -_tests.append(_test_select_abi3) - def _test_select_by_supported_py_version(env): for want_python_version, match in { "3.11": "pkg-0.0.1-py311-abi3-any.whl", @@ -140,8 +131,7 @@ def _test_select_by_supported_py_version(env): "pkg-0.0.1-py3-abi3-any.whl", "pkg-0.0.1-py311-abi3-any.whl", ], - want_abis = ["abi3"], - want_platforms = ["ignored"], + want_platforms = ["cp3x_ignored"], want_python_version = want_python_version, ) _match(env, got, match) @@ -160,8 +150,7 @@ def _test_select_by_supported_cp_version(env): "pkg-0.0.1-py311-abi3-any.whl", "pkg-0.0.1-cp311-abi3-any.whl", ], - want_abis = ["abi3"], - want_platforms = ["ignored"], + want_platforms = ["cp3x_ignored"], want_python_version = want_python_version, ) _match(env, got, match) @@ -180,8 +169,7 @@ def _test_supported_cp_version_manylinux(env): "pkg-0.0.1-py311-none-manylinux_x86_64.whl", "pkg-0.0.1-cp311-none-manylinux_x86_64.whl", ], - want_abis = ["none"], - want_platforms = ["linux_x86_64"], + want_platforms = ["cp{}_linux_x86_64".format(want_python_version.replace(".", ""))], want_python_version = want_python_version, ) _match(env, got, match) @@ -193,8 +181,7 @@ def _test_ignore_unsupported(env): whls = [ "pkg-0.0.1-xx3-abi3-any.whl", ], - want_abis = ["abi3"], - want_platforms = ["ignored"], + want_platforms = ["cp3x_ignored"], ) _match(env, got) @@ -202,24 +189,28 @@ _tests.append(_test_ignore_unsupported) def _test_match_abi_and_not_py_version(env): # Check we match the ABI and not the py version - got = _select_whls(whls = WHL_LIST, want_abis = ["cp37m"], want_platforms = ["linux_x86_64"], want_python_version = "3.7") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp37_linux_x86_64"], want_python_version = "3.7") _match( env, got, "pkg-0.0.1-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", "pkg-0.0.1-cp37-cp37m-musllinux_1_1_x86_64.whl", + "pkg-0.0.1-py3-abi3-any.whl", + "pkg-0.0.1-py3-none-any.whl", ) _tests.append(_test_match_abi_and_not_py_version) def _test_select_filename_with_many_tags(env): # Check we can select a filename with many platform tags - got = _select_whls(whls = WHL_LIST, want_abis = ["cp39"], want_platforms = ["linux_x86_32"], want_python_version = "3.9") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_32"], want_python_version = "3.9") _match( env, got, "pkg-0.0.1-cp39-cp39-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", "pkg-0.0.1-cp39-cp39-musllinux_1_1_i686.whl", + "pkg-0.0.1-cp39-abi3-any.whl", + "pkg-0.0.1-py3-none-any.whl", ) _tests.append(_test_select_filename_with_many_tags) @@ -228,8 +219,7 @@ def _test_osx_prefer_arch_specific(env): # Check that we prefer the specific wheel got = _select_whls( whls = WHL_LIST, - want_abis = ["cp311"], - want_platforms = ["osx_x86_64", "osx_x86_32"], + want_platforms = ["cp311_osx_x86_64", "cp311_osx_x86_32"], want_python_version = "3.11", ) _match( @@ -237,32 +227,42 @@ def _test_osx_prefer_arch_specific(env): got, "pkg-0.0.1-cp311-cp311-macosx_10_9_universal2.whl", "pkg-0.0.1-cp311-cp311-macosx_10_9_x86_64.whl", + "pkg-0.0.1-cp39-abi3-any.whl", + "pkg-0.0.1-py3-none-any.whl", ) - got = _select_whls(whls = WHL_LIST, want_abis = ["cp311"], want_platforms = ["osx_aarch64"], want_python_version = "3.11") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp311_osx_aarch64"], want_python_version = "3.11") _match( env, got, "pkg-0.0.1-cp311-cp311-macosx_10_9_universal2.whl", "pkg-0.0.1-cp311-cp311-macosx_11_0_arm64.whl", + "pkg-0.0.1-cp39-abi3-any.whl", + "pkg-0.0.1-py3-none-any.whl", ) _tests.append(_test_osx_prefer_arch_specific) def _test_osx_fallback_to_universal2(env): # Check that we can use the universal2 if the arm wheel is not available - got = _select_whls(whls = [w for w in WHL_LIST if "arm64" not in w], want_abis = ["cp311"], want_platforms = ["osx_aarch64"], want_python_version = "3.11") + got = _select_whls( + whls = [w for w in WHL_LIST if "arm64" not in w], + want_platforms = ["cp311_osx_aarch64"], + want_python_version = "3.11", + ) _match( env, got, "pkg-0.0.1-cp311-cp311-macosx_10_9_universal2.whl", + "pkg-0.0.1-cp39-abi3-any.whl", + "pkg-0.0.1-py3-none-any.whl", ) _tests.append(_test_osx_fallback_to_universal2) def _test_prefer_manylinux_wheels(env): # Check we prefer platform specific wheels - got = _select_whls(whls = WHL_LIST, want_abis = ["none", "abi3", "cp39"], want_platforms = ["linux_x86_64"], want_python_version = "3.9") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_64"], want_python_version = "3.9") _match( env, got, From 2d799a4d6081bee22c5113bc27b9b21f318c0698 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:24:06 +0900 Subject: [PATCH 3/9] drop the need to have the version --- python/private/pypi/whl_target_platforms.bzl | 17 ++++---- .../whl_target_platforms/select_whl_tests.bzl | 41 ++++++++----------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/python/private/pypi/whl_target_platforms.bzl b/python/private/pypi/whl_target_platforms.bzl index 68c73d7c36..bdc44c697a 100644 --- a/python/private/pypi/whl_target_platforms.bzl +++ b/python/private/pypi/whl_target_platforms.bzl @@ -46,13 +46,12 @@ _OS_PREFIXES = { "win": "windows", } # buildifier: disable=unsorted-dict-items -def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logger = None): +def select_whls(*, whls, want_platforms = [], logger = None): """Select a subset of wheels suitable for target platforms from a list. Args: whls(list[struct]): A list of candidates which have a `filename` attribute containing the `whl` filename. - want_python_version(str): An optional parameter to filter whls by python version. Defaults to '3.0'. want_platforms(str): The platforms in "{abi}_{os}_{cpu}" or "{os}_{cpu}" format. logger: A logger for printing diagnostic messages. @@ -69,6 +68,8 @@ def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logge } _want_platforms = {} + version_limit = None + for p in want_platforms: if not p.startswith("cp3"): fail("expected all platforms to start with ABI, but got: {}".format(p)) @@ -77,6 +78,12 @@ def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logge _want_platforms[os_cpu] = None _want_platforms[p] = None + version_limit_candidate = int(abi[3:]) + if not version_limit: + version_limit = version_limit_candidate + if version_limit and version_limit != version_limit_candidate: + fail("Only a single python version is supported for now") + # For some legacy implementations the wheels may target the `cp3xm` ABI _want_platforms["{}m_{}".format(abi, os_cpu)] = None want_abis[abi] = None @@ -84,10 +91,6 @@ def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logge want_platforms = sorted(_want_platforms) - version_limit = -1 - if want_python_version: - version_limit = int(want_python_version.split(".")[1]) - candidates = {} for whl in whls: parsed = parse_whl_name(whl.filename) @@ -121,7 +124,7 @@ def select_whls(*, whls, want_python_version = "3.0", want_platforms = [], logge logger.trace(lambda: "Discarding the whl because the whl abi did not match") continue - if version_limit != -1 and whl_version_min > version_limit: + if whl_version_min > version_limit: if logger: logger.trace(lambda: "Discarding the whl because the whl supported python version is too high") continue diff --git a/tests/pypi/whl_target_platforms/select_whl_tests.bzl b/tests/pypi/whl_target_platforms/select_whl_tests.bzl index a848767d92..2994bd513f 100644 --- a/tests/pypi/whl_target_platforms/select_whl_tests.bzl +++ b/tests/pypi/whl_target_platforms/select_whl_tests.bzl @@ -109,7 +109,7 @@ def _test_simplest(env): "pkg-0.0.1-py3-abi3-any.whl", "pkg-0.0.1-py3-none-any.whl", ], - want_platforms = ["cp3x_ignored"], + want_platforms = ["cp30_ignored"], ) _match( env, @@ -121,9 +121,9 @@ def _test_simplest(env): _tests.append(_test_simplest) def _test_select_by_supported_py_version(env): - for want_python_version, match in { - "3.11": "pkg-0.0.1-py311-abi3-any.whl", - "3.8": "pkg-0.0.1-py3-abi3-any.whl", + for minor_version, match in { + 8: "pkg-0.0.1-py3-abi3-any.whl", + 11: "pkg-0.0.1-py311-abi3-any.whl", }.items(): got = _select_whls( whls = [ @@ -131,17 +131,16 @@ def _test_select_by_supported_py_version(env): "pkg-0.0.1-py3-abi3-any.whl", "pkg-0.0.1-py311-abi3-any.whl", ], - want_platforms = ["cp3x_ignored"], - want_python_version = want_python_version, + want_platforms = ["cp3{}_ignored".format(minor_version)], ) _match(env, got, match) _tests.append(_test_select_by_supported_py_version) def _test_select_by_supported_cp_version(env): - for want_python_version, match in { - "3.11": "pkg-0.0.1-cp311-abi3-any.whl", - "3.8": "pkg-0.0.1-py3-abi3-any.whl", + for minor_version, match in { + 11: "pkg-0.0.1-cp311-abi3-any.whl", + 8: "pkg-0.0.1-py3-abi3-any.whl", }.items(): got = _select_whls( whls = [ @@ -150,17 +149,16 @@ def _test_select_by_supported_cp_version(env): "pkg-0.0.1-py311-abi3-any.whl", "pkg-0.0.1-cp311-abi3-any.whl", ], - want_platforms = ["cp3x_ignored"], - want_python_version = want_python_version, + want_platforms = ["cp3{}_ignored".format(minor_version)], ) _match(env, got, match) _tests.append(_test_select_by_supported_cp_version) def _test_supported_cp_version_manylinux(env): - for want_python_version, match in { - "3.11": "pkg-0.0.1-cp311-none-manylinux_x86_64.whl", - "3.8": "pkg-0.0.1-py3-none-manylinux_x86_64.whl", + for minor_version, match in { + 8: "pkg-0.0.1-py3-none-manylinux_x86_64.whl", + 11: "pkg-0.0.1-cp311-none-manylinux_x86_64.whl", }.items(): got = _select_whls( whls = [ @@ -169,8 +167,7 @@ def _test_supported_cp_version_manylinux(env): "pkg-0.0.1-py311-none-manylinux_x86_64.whl", "pkg-0.0.1-cp311-none-manylinux_x86_64.whl", ], - want_platforms = ["cp{}_linux_x86_64".format(want_python_version.replace(".", ""))], - want_python_version = want_python_version, + want_platforms = ["cp3{}_linux_x86_64".format(minor_version)], ) _match(env, got, match) @@ -181,7 +178,7 @@ def _test_ignore_unsupported(env): whls = [ "pkg-0.0.1-xx3-abi3-any.whl", ], - want_platforms = ["cp3x_ignored"], + want_platforms = ["cp30_ignored"], ) _match(env, got) @@ -189,7 +186,7 @@ _tests.append(_test_ignore_unsupported) def _test_match_abi_and_not_py_version(env): # Check we match the ABI and not the py version - got = _select_whls(whls = WHL_LIST, want_platforms = ["cp37_linux_x86_64"], want_python_version = "3.7") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp37_linux_x86_64"]) _match( env, got, @@ -203,7 +200,7 @@ _tests.append(_test_match_abi_and_not_py_version) def _test_select_filename_with_many_tags(env): # Check we can select a filename with many platform tags - got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_32"], want_python_version = "3.9") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_32"]) _match( env, got, @@ -220,7 +217,6 @@ def _test_osx_prefer_arch_specific(env): got = _select_whls( whls = WHL_LIST, want_platforms = ["cp311_osx_x86_64", "cp311_osx_x86_32"], - want_python_version = "3.11", ) _match( env, @@ -231,7 +227,7 @@ def _test_osx_prefer_arch_specific(env): "pkg-0.0.1-py3-none-any.whl", ) - got = _select_whls(whls = WHL_LIST, want_platforms = ["cp311_osx_aarch64"], want_python_version = "3.11") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp311_osx_aarch64"]) _match( env, got, @@ -248,7 +244,6 @@ def _test_osx_fallback_to_universal2(env): got = _select_whls( whls = [w for w in WHL_LIST if "arm64" not in w], want_platforms = ["cp311_osx_aarch64"], - want_python_version = "3.11", ) _match( env, @@ -262,7 +257,7 @@ _tests.append(_test_osx_fallback_to_universal2) def _test_prefer_manylinux_wheels(env): # Check we prefer platform specific wheels - got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_64"], want_python_version = "3.9") + got = _select_whls(whls = WHL_LIST, want_platforms = ["cp39_linux_x86_64"]) _match( env, got, From b7f21120c6f245fec97c51a41b33fc47294b0544 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:26:17 +0900 Subject: [PATCH 4/9] update the prod code --- python/private/pypi/parse_requirements.bzl | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index 968c486bfb..d6eabfc619 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -168,9 +168,8 @@ def parse_requirements( for r in sorted(reqs.values(), key = lambda r: r.requirement_line): whls, sdist = _add_dists( - r, - index_urls.get(whl_name), - python_version = python_version, + requirement = r, + index_urls = index_urls.get(whl_name), logger = logger, ) @@ -238,7 +237,7 @@ def host_platform(ctx): repo_utils.get_platforms_cpu_name(ctx), ) -def _add_dists(requirement, index_urls, python_version, logger = None): +def _add_dists(*, requirement, index_urls, logger = None): """Populate dists based on the information from the PyPI index. This function will modify the given requirements_by_platform data structure. @@ -246,7 +245,6 @@ def _add_dists(requirement, index_urls, python_version, logger = None): Args: requirement: The result of parse_requirements function. index_urls: The result of simpleapi_download. - python_version: The version of the python interpreter. logger: A logger for printing diagnostic info. """ if not index_urls: @@ -289,18 +287,6 @@ def _add_dists(requirement, index_urls, python_version, logger = None): ])) # Filter out the wheels that are incompatible with the target_platforms. - whls = select_whls( - whls = whls, - want_abis = [ - "none", - "abi3", - "cp" + python_version.replace(".", ""), - # Older python versions have wheels for the `*m` ABI. - "cp" + python_version.replace(".", "") + "m", - ], - want_platforms = requirement.target_platforms, - want_python_version = python_version, - logger = logger, - ) + whls = select_whls(whls = whls, want_platforms = requirement.target_platforms, logger = logger) return whls, sdist From 746f74f5da137e56632658b7ce33f90458d62f17 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:57:44 +0900 Subject: [PATCH 5/9] remove unused code --- python/private/pypi/extension.bzl | 1 - python/private/pypi/parse_requirements.bzl | 11 +---------- .../parse_requirements_tests.bzl | 14 -------------- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 48ae1c51c3..82e580d3a2 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -195,7 +195,6 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s logger = logger, ), get_index_urls = get_index_urls, - python_version = major_minor, logger = logger, ) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index d6eabfc619..0cab1d708a 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -38,9 +38,7 @@ def parse_requirements( requirements_by_platform = {}, extra_pip_args = [], get_index_urls = None, - python_version = None, - logger = None, - fail_fn = fail): + logger = None): """Get the requirements with platforms that the requirements apply to. Args: @@ -53,10 +51,7 @@ def parse_requirements( get_index_urls: Callable[[ctx, list[str]], dict], a callable to get all of the distribution URLs from a PyPI index. Accepts ctx and distribution names to query. - python_version: str or None. This is needed when the get_index_urls is - specified. It should be of the form "3.x.x", logger: repo_utils.logger or None, a simple struct to log diagnostic messages. - fail_fn (Callable[[str], None]): A failure function used in testing failure cases. Returns: A tuple where the first element a dict of dicts where the first key is @@ -137,10 +132,6 @@ def parse_requirements( index_urls = {} if get_index_urls: - if not python_version: - fail_fn("'python_version' must be provided") - return None - index_urls = get_index_urls( ctx, # Use list({}) as a way to have a set diff --git a/tests/pypi/parse_requirements/parse_requirements_tests.bzl b/tests/pypi/parse_requirements/parse_requirements_tests.bzl index 1a7143b747..280dbd1a6c 100644 --- a/tests/pypi/parse_requirements/parse_requirements_tests.bzl +++ b/tests/pypi/parse_requirements/parse_requirements_tests.bzl @@ -197,20 +197,6 @@ def _test_select_requirement_none_platform(env): _tests.append(_test_select_requirement_none_platform) -def _test_fail_no_python_version(env): - errors = [] - parse_requirements( - ctx = _mock_ctx(), - requirements_by_platform = { - "requirements_lock": [""], - }, - get_index_urls = lambda _, __: {}, - fail_fn = errors.append, - ) - env.expect.that_str(errors[0]).equals("'python_version' must be provided") - -_tests.append(_test_fail_no_python_version) - def parse_requirements_test_suite(name): """Create the test suite. From 3add69c9b4742bc1140a01c23e0d946ab438c215 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 20 Jul 2024 13:15:38 +0900 Subject: [PATCH 6/9] comment: remove leftover code --- python/private/repo_utils.bzl | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index cdfb322f1c..e99cbc7aad 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -73,8 +73,6 @@ def _logger(ctx, name = None): elif not name: fail("The name has to be specified when using the logger with `module_ctx`") - name = getattr(ctx, "name", "extension") - def _log(enabled_on_verbosity, level, message_cb_or_str): if verbosity < enabled_on_verbosity: return From def94ed291c7fcaf806d3db3ee97e37abe749db7 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 20 Jul 2024 13:20:42 +0900 Subject: [PATCH 7/9] fix: repo_utils.getenv --- python/private/repo_utils.bzl | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index e99cbc7aad..3c07027663 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -58,7 +58,7 @@ def _logger(ctx, name = None): else: verbosity_level = "WARN" - env_var_verbosity = ctx.os.environ.get(REPO_VERBOSITY_ENV_VAR) + env_var_verbosity = _getenv(ctx, REPO_VERBOSITY_ENV_VAR) verbosity_level = env_var_verbosity or verbosity_level verbosity = { @@ -284,12 +284,9 @@ def _which_describe_failure(binary_name, path): path = path, ) -def _getenv(rctx, name, default = None): - # Bazel 7+ API - if hasattr(rctx, "getenv"): - return rctx.getenv(name, default) - else: - return rctx.os.environ.get("PATH", default) +def _getenv(ctx, name, default = None): + # Bazel 7+ API has ctx.getenv + return getattr(ctx, "getenv", ctx.os.environ.get)(name, default) def _args_to_str(arguments): return " ".join([_arg_repr(a) for a in arguments]) From d74b8c9cb14ecd192b2bf8db9526d57c50b8ccb3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 20 Jul 2024 13:30:39 +0900 Subject: [PATCH 8/9] comment: better log message --- python/private/pypi/whl_library.bzl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index f453f92ccf..0419926c7a 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -231,9 +231,16 @@ def _whl_library_impl(rctx): args = _parse_optional_attrs(rctx, args, extra_pip_args) if not whl_path: + if rctx.attr.urls: + op_tmpl = "whl_library.BuildWheelFromSource({name}, {requirement})" + elif rctx.attr.download_only: + op_tmpl = "whl_library.DownloadWheel({name}, {requirement})" + else: + op_tmpl = "whl_library.ResolveRequirement({name}, {requirement})" + repo_utils.execute_checked( rctx, - op = "whl_library.ResolveRequirement({}, {})".format(rctx.attr.name, rctx.attr.requirement), + op = op_tmpl.format(name = rctx.attr.name, requirement = rctx.attr.requirement), arguments = args, environment = environment, quiet = rctx.attr.quiet, From b9caae47f1e54e9194cee0525b291459d4c362bd Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 20 Jul 2024 13:38:18 +0900 Subject: [PATCH 9/9] doc: add a note in changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a0792d3a5..8c03a79061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,8 @@ A brief description of the categories of changes: [x.x.x]: https://github.com/bazelbuild/rules_python/releases/tag/x.x.x ### Changed -* Nothing yet +* (whl_library) A better log message when the wheel is built from an sdist or + when the wheel is downloaded using `download_only` feature to aid debugging. ### Fixed * (rules) Signals are properly received when using {obj}`--bootstrap_impl=script`