From 3b62fd50d075cafebc2c1f42b9c3fb622deb12c4 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 17 Jan 2024 09:21:33 +0900 Subject: [PATCH 1/5] fix(bzlmod): use X.Y select for the hub repo This allows better interoperability between toolchains and the wheels built by the `whl_library`, as wheels are usually compatible across all of the python patch versions. --- CHANGELOG.md | 4 ++ python/private/bzlmod/pip.bzl | 9 ++- python/private/bzlmod/pip_repository.bzl | 2 +- .../render_pkg_aliases_test.bzl | 67 ++++++++++--------- 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c689e9d6b5..66f0fc9ea4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ A brief description of the categories of changes: * **BREAKING** (wheel) The `incompatible_normalize_name` and `incompatible_normalize_version` flags have been removed. They had been flipped to `True` in 0.27.0 release. +* (bzlmod) The pip hub repository now uses the newly introduced config settings + using the `X.Y` python version notation. This improves cross module + interoperability and allows to share wheels built by interpreters using + different patch versions. ### Fixed diff --git a/python/private/bzlmod/pip.bzl b/python/private/bzlmod/pip.bzl index ae19dad610..caa9da490a 100644 --- a/python/private/bzlmod/pip.bzl +++ b/python/private/bzlmod/pip.bzl @@ -31,6 +31,11 @@ load("//python/private:parse_whl_name.bzl", "parse_whl_name") load("//python/private:version_label.bzl", "version_label") load(":pip_repository.bzl", "pip_repository") +def _major_minor_version(version): + version = full_version(version) + major_minor, _, _ = version.rpartition(".") + return major_minor + def _whl_mods_impl(mctx): """Implementation of the pip.whl_mods tag class. @@ -190,7 +195,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides): if whl_name not in whl_map[hub_name]: whl_map[hub_name][whl_name] = {} - whl_map[hub_name][whl_name][full_version(pip_attr.python_version)] = pip_name + "_" + whl_map[hub_name][whl_name][_major_minor_version(pip_attr.python_version)] = pip_name + "_" def _pip_impl(module_ctx): """Implementation of a class tag that creates the pip hub and corresponding pip spoke whl repositories. @@ -341,7 +346,7 @@ def _pip_impl(module_ctx): name = hub_name, repo_name = hub_name, whl_map = whl_map, - default_version = full_version(DEFAULT_PYTHON_VERSION), + default_version = _major_minor_version(DEFAULT_PYTHON_VERSION), ) def _pip_parse_ext_attrs(): diff --git a/python/private/bzlmod/pip_repository.bzl b/python/private/bzlmod/pip_repository.bzl index 9e6b0f4669..8ea5ee7526 100644 --- a/python/private/bzlmod/pip_repository.bzl +++ b/python/private/bzlmod/pip_repository.bzl @@ -63,7 +63,7 @@ pip_repository_attrs = { "default_version": attr.string( mandatory = True, doc = """\ -This is the default python version in the format of X.Y.Z. This should match +This is the default python version in the format of X.Y. This should match what is setup by the 'python' extension using the 'is_default = True' setting.""", ), diff --git a/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl b/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl index dff7cd0fbc..3f99ef371f 100644 --- a/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl +++ b/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl @@ -73,11 +73,11 @@ _tests.append(_test_all_legacy_aliases_are_created) def _test_bzlmod_aliases(env): actual = render_pkg_aliases( - default_version = "3.2.3", + default_version = "3.2", repo_name = "pypi", rules_python = "rules_python", whl_map = { - "bar-baz": ["3.2.3"], + "bar-baz": ["3.2"], }, ) @@ -94,7 +94,7 @@ alias( name = "pkg", actual = select( { - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:pkg", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:pkg", "//conditions:default": "@pypi_32_bar_baz//:pkg", }, ), @@ -104,7 +104,7 @@ alias( name = "whl", actual = select( { - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:whl", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:whl", "//conditions:default": "@pypi_32_bar_baz//:whl", }, ), @@ -114,7 +114,7 @@ alias( name = "data", actual = select( { - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:data", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:data", "//conditions:default": "@pypi_32_bar_baz//:data", }, ), @@ -124,7 +124,7 @@ alias( name = "dist_info", actual = select( { - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:dist_info", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:dist_info", "//conditions:default": "@pypi_32_bar_baz//:dist_info", }, ), @@ -141,7 +141,7 @@ def _test_bzlmod_aliases_with_no_default_version(env): repo_name = "pypi", rules_python = "rules_python", whl_map = { - "bar-baz": ["3.2.3", "3.1.3"], + "bar-baz": ["3.2", "3.1"], }, ) @@ -154,7 +154,7 @@ No matching wheel for current configuration's Python version. The current build configuration's Python version doesn't match any of the Python versions available for this wheel. This wheel supports the following Python versions: - 3.1.3, 3.2.3 + 3.1, 3.2 As matched by the `@rules_python//python/config_settings:is_python_` configuration settings. @@ -177,8 +177,8 @@ alias( name = "pkg", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:pkg", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:pkg", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:pkg", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:pkg", }, no_match_error = _NO_MATCH_ERROR, ), @@ -188,8 +188,8 @@ alias( name = "whl", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:whl", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:whl", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:whl", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:whl", }, no_match_error = _NO_MATCH_ERROR, ), @@ -199,8 +199,8 @@ alias( name = "data", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:data", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:data", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:data", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:data", }, no_match_error = _NO_MATCH_ERROR, ), @@ -210,8 +210,8 @@ alias( name = "dist_info", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:dist_info", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:dist_info", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:dist_info", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:dist_info", }, no_match_error = _NO_MATCH_ERROR, ), @@ -224,11 +224,18 @@ _tests.append(_test_bzlmod_aliases_with_no_default_version) def _test_bzlmod_aliases_for_non_root_modules(env): actual = render_pkg_aliases( - default_version = "3.2.4", + # NOTE @aignas 2024-01-17: if the default X.Y version coincides with the + # versions that are used in the root module, then this would be the same as + # as _test_bzlmod_aliases. + # + # However, if the root module uses a different default version than the + # non-root module, then we will have a no-match-error because the default_version + # is not in the list of the versions in the whl_map. + default_version = "3.3", repo_name = "pypi", rules_python = "rules_python", whl_map = { - "bar-baz": ["3.2.3", "3.1.3"], + "bar-baz": ["3.2", "3.1"], }, ) @@ -241,7 +248,7 @@ No matching wheel for current configuration's Python version. The current build configuration's Python version doesn't match any of the Python versions available for this wheel. This wheel supports the following Python versions: - 3.1.3, 3.2.3 + 3.1, 3.2 As matched by the `@rules_python//python/config_settings:is_python_` configuration settings. @@ -264,8 +271,8 @@ alias( name = "pkg", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:pkg", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:pkg", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:pkg", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:pkg", }, no_match_error = _NO_MATCH_ERROR, ), @@ -275,8 +282,8 @@ alias( name = "whl", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:whl", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:whl", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:whl", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:whl", }, no_match_error = _NO_MATCH_ERROR, ), @@ -286,8 +293,8 @@ alias( name = "data", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:data", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:data", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:data", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:data", }, no_match_error = _NO_MATCH_ERROR, ), @@ -297,8 +304,8 @@ alias( name = "dist_info", actual = select( { - "@@rules_python//python/config_settings:is_python_3.1.3": "@pypi_31_bar_baz//:dist_info", - "@@rules_python//python/config_settings:is_python_3.2.3": "@pypi_32_bar_baz//:dist_info", + "@@rules_python//python/config_settings:is_python_3.1": "@pypi_31_bar_baz//:dist_info", + "@@rules_python//python/config_settings:is_python_3.2": "@pypi_32_bar_baz//:dist_info", }, no_match_error = _NO_MATCH_ERROR, ), @@ -311,12 +318,12 @@ _tests.append(_test_bzlmod_aliases_for_non_root_modules) def _test_bzlmod_aliases_are_created_for_all_wheels(env): actual = render_pkg_aliases( - default_version = "3.2.3", + default_version = "3.2", repo_name = "pypi", rules_python = "rules_python", whl_map = { - "bar": ["3.1.2", "3.2.3"], - "foo": ["3.1.2", "3.2.3"], + "bar": ["3.1", "3.2"], + "foo": ["3.1", "3.2"], }, ) From 8f86a22e6e22c964b2e8e02091c2fda54bada6d1 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 17 Jan 2024 10:04:16 +0900 Subject: [PATCH 2/5] fix: set a different default for the python_version setting --- python/config_settings/config_settings.bzl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index bd4a1b2166..6874a1888f 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -38,8 +38,10 @@ def construct_config_settings(name, python_versions): string_flag( name = "python_version", # TODO: The default here should somehow match the MODULE config - build_setting_default = python_versions[0], - values = sorted(allowed_flag_values), + # NOTE @aignas 2024-01-17: python_versions[0] as default makes the + # default wheel selection on bzlmod not work. + build_setting_default = "", + values = [""] + sorted(allowed_flag_values), visibility = ["//visibility:public"], ) @@ -53,7 +55,6 @@ def construct_config_settings(name, python_versions): name = equals_minor_version_name, flag_values = {":python_version": minor_version}, ) - matches_minor_version_names = [equals_minor_version_name] for micro_version in micro_versions: From ef5c7e81ef0a884afcef42b1720fb648f2def95a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Thu, 18 Jan 2024 09:52:14 +0900 Subject: [PATCH 3/5] comment: improve the version parsing, which can later become a utility function --- python/config_settings/config_settings.bzl | 13 +++++++------ python/private/bzlmod/pip.bzl | 20 ++++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 6874a1888f..fde16fb592 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -29,7 +29,9 @@ def construct_config_settings(name, python_versions): # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] minor_to_micro_versions = {} - allowed_flag_values = [] + + # Add the legacy value that is used in non-version aware toolchains + allowed_flag_values = ["PY3"] for micro_version in python_versions: minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version) @@ -37,11 +39,10 @@ def construct_config_settings(name, python_versions): string_flag( name = "python_version", - # TODO: The default here should somehow match the MODULE config - # NOTE @aignas 2024-01-17: python_versions[0] as default makes the - # default wheel selection on bzlmod not work. - build_setting_default = "", - values = [""] + sorted(allowed_flag_values), + # Default to the legacy value that non-version aware toolchains use + # when setting the `python_version` attribute on `py_*` rules. + build_setting_default = allowed_flag_values[0], + values = sorted(allowed_flag_values), visibility = ["//visibility:public"], ) diff --git a/python/private/bzlmod/pip.bzl b/python/private/bzlmod/pip.bzl index caa9da490a..ce3ddde66c 100644 --- a/python/private/bzlmod/pip.bzl +++ b/python/private/bzlmod/pip.bzl @@ -25,16 +25,28 @@ load( "whl_library", ) load("//python/pip_install:requirements_parser.bzl", parse_requirements = "parse") -load("//python/private:full_version.bzl", "full_version") load("//python/private:normalize_name.bzl", "normalize_name") load("//python/private:parse_whl_name.bzl", "parse_whl_name") load("//python/private:version_label.bzl", "version_label") load(":pip_repository.bzl", "pip_repository") +def _parse_version(version): + major, _, version = version.partition(".") + minor, _, version = version.partition(".") + patch, _, version = version.partition(".") + build, _, version = version.partition(".") + + return struct( + # use semver vocabulary here + major = major, + minor = minor, + patch = patch, # this is called `micro` in the Python interpreter versioning scheme + build = build, + ) + def _major_minor_version(version): - version = full_version(version) - major_minor, _, _ = version.rpartition(".") - return major_minor + version = _parse_version(version) + return "{}.{}".format(version.major, version.minor) def _whl_mods_impl(mctx): """Implementation of the pip.whl_mods tag class. From 7c7ba687e1f61ae8a962be2570c233988d8fef7c Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 17 Jan 2024 21:58:39 -0800 Subject: [PATCH 4/5] wip: reproing bug --- examples/multi_python_versions/tests/my_lib_test.py | 4 +++- python/config_settings/config_settings.bzl | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/examples/multi_python_versions/tests/my_lib_test.py b/examples/multi_python_versions/tests/my_lib_test.py index 1d4880ff8a..3f334f531f 100644 --- a/examples/multi_python_versions/tests/my_lib_test.py +++ b/examples/multi_python_versions/tests/my_lib_test.py @@ -23,5 +23,7 @@ if not my_lib.websockets_is_for_python_version( workspace_version ) and not my_lib.websockets_is_for_python_version(bzlmod_version): - print("expected package for Python version is different than returned") + print("expected package for Python version is different than returned\n" + f"expected either {workspace_version} or {bzlmod_version}\n" + f"but got {my_lib.websockets.__file__}") sys.exit(1) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index fde16fb592..0602db4029 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -39,10 +39,12 @@ def construct_config_settings(name, python_versions): string_flag( name = "python_version", - # Default to the legacy value that non-version aware toolchains use - # when setting the `python_version` attribute on `py_*` rules. - build_setting_default = allowed_flag_values[0], - values = sorted(allowed_flag_values), + # TODO: The default here should somehow match the MODULE config. Until + # then, use the empty string to indicate an unknown version. This + # also prevents version-unaware targets from inadvertently matching + # a select condition when they shouldn't. + build_setting_default = "", + values = [""] + sorted(allowed_flag_values), visibility = ["//visibility:public"], ) From af5a51e934030a906d59453e425fddb83a03f2f2 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 17 Jan 2024 23:20:01 -0800 Subject: [PATCH 5/5] remove PY3 as valid flag value --- python/config_settings/config_settings.bzl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 0602db4029..f1e142cc36 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -30,8 +30,7 @@ def construct_config_settings(name, python_versions): # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] minor_to_micro_versions = {} - # Add the legacy value that is used in non-version aware toolchains - allowed_flag_values = ["PY3"] + allowed_flag_values = [] for micro_version in python_versions: minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version)