From 866ab7d087847745c4736ec5802cf0148a8153c0 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 22 Dec 2024 13:53:04 +0900 Subject: [PATCH 1/5] doc(pypi): A better error message when the wheel select hits no_match With this change we get the current values of the python configuration values printed in addition to the message printed previously. This should help us advise users who don't have their builds configured correctly. We are adding an extra `build_setting` which we can set in order to get an error message instead of a `DEBUG` warning. This has been documented as part of our config settings and in the `no_match_error` in the `select` statement. Example output now ```console $ bazel cquery --@rules_python//python/config_settings:python_version=3.12 @dev_pip//sphinx DEBUG: /home/aignas/src/github/aignas/rules_python/python/private/config_settings.bzl:193:14: The current configuration rules_python config flags is: @@//python/config_settings:pip_whl: "auto" @@//python/config_settings:pip_whl_glibc_version: "" @@//python/config_settings:pip_whl_muslc_version: "" @@//python/config_settings:pip_whl_osx_arch: "arch" @@//python/config_settings:pip_whl_osx_version: "" @@//python/config_settings:py_freethreaded: "no" @@//python/config_settings:py_linux_libc: "glibc" @@//python/config_settings:python_version: "3.12" If the value is missing, then the default value is being used, see documentation: https://rules-python.readthedocs.io/en/latest/api/rules_python/python/config_settings ERROR: /home/aignas/.cache/bazel/_bazel_aignas/6f0de8c9128ee8d5dbf27ba6dcc48bdd/external/+pip+dev_pip/sphinx/BUILD.bazel:6:12: configurable attribute "actual" in @@+pip+dev_pip//sphinx:_no_matching_repository doesn't match this configuration: No matching wheel for current configuration's Python version. The current build configuration's Python version doesn't match any of the Python wheels available for this distribution. This distribution supports the following Python configuration settings: //_config:is_cp3.11_py3_none_any //_config:is_cp3.13_py3_none_any To determine the current configuration's Python version, run: `bazel config ` (shown further below) For the current configuration value see the debug message above that is printing the current flag values. If you can't see the message, then re-run the build to make it a failure instead by running the build with: --@@//python/config_settings:current_config=fail However, the command above will hide the `bazel config ` message. This instance of @@+pip+dev_pip//sphinx:_no_matching_repository has configuration identifier 29ffcf8. To inspect its configuration, run: bazel config 29ffcf8. For more help, see https://bazel.build/docs/configurable-attributes#faq-select-choose-condition. ERROR: Analysis of target '@@+pip+dev_pip//sphinx:sphinx' failed; build aborted: Analysis failed INFO: Elapsed time: 0.112s INFO: 0 processes. ERROR: Build did NOT complete successfully ``` Fixes #2466 --- CHANGELOG.md | 3 + .../python/config_settings/index.md | 18 +++++ python/config_settings/BUILD.bazel | 9 +++ python/private/config_settings.bzl | 78 +++++++++++++++++-- python/private/pypi/pkg_aliases.bzl | 68 ++++++++-------- tests/pypi/pkg_aliases/pkg_aliases_test.bzl | 66 ++++++++++------ 6 files changed, 176 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5583399e96..4871531ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,9 @@ Unreleased changes template. * (pypi) Using {bzl:obj}`pip_parse.experimental_requirement_cycles` and {bzl:obj}`pip_parse.use_hub_alias_dependencies` together now works when using WORKSPACE files. +* The error messages when the wheel distributions do not match anything + are now printing more details and include the currently active flag + values. Fixes [#2466](https://github.com/bazelbuild/rules_python/issues/2466). [pep-695]: https://peps.python.org/pep-0695/ diff --git a/docs/api/rules_python/python/config_settings/index.md b/docs/api/rules_python/python/config_settings/index.md index ef829bab76..dcecd8e8fc 100644 --- a/docs/api/rules_python/python/config_settings/index.md +++ b/docs/api/rules_python/python/config_settings/index.md @@ -240,3 +240,21 @@ instead. ::: :::: + +::::{bzl:flag} current_config +Fail the build if the current build configuration does not match the +{obj}`pip.parse` defined wheels. + +Values: +* `fail`: Will fail in the build action ensuring that we get the error + message no matter the action cache. +* ``: The default value, that will just print a warning. + +:::{seealso} +{obj}`pip.parse` +::: + +:::{versionadded} 1.1.0 +::: + +:::: diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 5455f5aef7..fcebcd76dc 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -29,6 +29,15 @@ filegroup( construct_config_settings( name = "construct_config_settings", default_version = DEFAULT_PYTHON_VERSION, + documented_flags = [ + ":pip_whl", + ":pip_whl_glibc_version", + ":pip_whl_muslc_version", + ":pip_whl_osx_arch", + ":pip_whl_osx_version", + ":py_freethreaded", + ":py_linux_libc", + ], minor_mapping = MINOR_MAPPING, versions = PYTHON_VERSIONS, ) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 10b4d686a7..d26e32a1ce 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -17,12 +17,21 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") +load("//python/private:text_util.bzl", "render") load(":semver.bzl", "semver") _PYTHON_VERSION_FLAG = Label("//python/config_settings:python_version") _PYTHON_VERSION_MAJOR_MINOR_FLAG = Label("//python/config_settings:python_version_major_minor") -def construct_config_settings(*, name, default_version, versions, minor_mapping): # buildifier: disable=function-docstring +_DEBUG_ENV_MESSAGE_TEMPLATE = """\ +The current configuration rules_python config flags is: + {flags} + +If the value is missing, then the default value is being used, see documentation: +{docs_url}/python/config_settings +""" + +def construct_config_settings(*, name, default_version, versions, minor_mapping, documented_flags): # buildifier: disable=function-docstring """Create a 'python_version' config flag and construct all config settings used in rules_python. This mainly includes the targets that are used in the toolchain and pip hub @@ -33,6 +42,8 @@ def construct_config_settings(*, name, default_version, versions, minor_mapping) default_version: {type}`str` the default value for the `python_version` flag. versions: {type}`list[str]` A list of versions to build constraint settings for. minor_mapping: {type}`dict[str, str]` A mapping from `X.Y` to `X.Y.Z` python versions. + documented_flags: {type}`list[str]` The labels of the documented settings + that affect build configuration. """ _ = name # @unused _python_version_flag( @@ -44,6 +55,7 @@ def construct_config_settings(*, name, default_version, versions, minor_mapping) _python_version_major_minor_flag( name = _PYTHON_VERSION_MAJOR_MINOR_FLAG.name, build_setting_default = "", + python_version_flag = _PYTHON_VERSION_FLAG, visibility = ["//visibility:public"], ) @@ -101,6 +113,25 @@ def construct_config_settings(*, name, default_version, versions, minor_mapping) visibility = ["//visibility:public"], ) + _current_config( + name = "current_config", + build_setting_default = "", + settings = documented_flags + [_PYTHON_VERSION_FLAG.name], + visibility = ["//visibility:private"], + ) + native.config_setting( + name = "is_not_matching_current_config", + # We use the rule above instead of @platforms//:incompatible so that the + # printing of the current env always happens when the _current_config rule + # is executed. + # + # NOTE: This should in practise only happen if there is a missing compatible + # `whl_library` in the hub repo created by `pip.parse`. + flag_values = {"current_config": "will-never-match"}, + # Only public so that PyPI hub repo can access it + visibility = ["//visibility:public"], + ) + def _python_version_flag_impl(ctx): value = ctx.build_setting_value return [ @@ -122,7 +153,7 @@ _python_version_flag = rule( ) def _python_version_major_minor_flag_impl(ctx): - input = ctx.attr._python_version_flag[config_common.FeatureFlagInfo].value + input = _flag_value(ctx.attr.python_version_flag) if input: version = semver(input) value = "{}.{}".format(version.major, version.minor) @@ -135,8 +166,45 @@ _python_version_major_minor_flag = rule( implementation = _python_version_major_minor_flag_impl, build_setting = config.string(flag = False), attrs = { - "_python_version_flag": attr.label( - default = _PYTHON_VERSION_FLAG, - ), + "python_version_flag": attr.label(mandatory = True), + }, +) + +def _flag_value(s): + if config_common.FeatureFlagInfo in s: + return s[config_common.FeatureFlagInfo].value + else: + return s[BuildSettingInfo].value + +def _print_current_config_impl(ctx): + flags = "\n".join([ + "{}: \"{}\"".format(k, v) + for k, v in sorted({ + str(setting.label): _flag_value(setting) + for setting in ctx.attr.settings + }.items()) + ]) + + msg = ctx.attr._template.format( + docs_url = "https://rules-python.readthedocs.io/en/latest/api/rules_python", + flags = render.indent(flags).lstrip(), + ) + if ctx.build_setting_value and ctx.build_setting_value != "fail": + fail("Only 'fail' and empty build setting values are allowed for {}".format( + str(ctx.label), + )) + elif ctx.build_setting_value: + fail(msg) + else: + print(msg) # buildifier: disable=print + + return [config_common.FeatureFlagInfo(value = "")] + +_current_config = rule( + implementation = _print_current_config_impl, + build_setting = config.string(flag = True), + attrs = { + "settings": attr.label_list(mandatory = True), + "_template": attr.string(default = _DEBUG_ENV_MESSAGE_TEMPLATE), }, ) diff --git a/python/private/pypi/pkg_aliases.bzl b/python/private/pypi/pkg_aliases.bzl index a6872fdce9..8229b5d56c 100644 --- a/python/private/pypi/pkg_aliases.bzl +++ b/python/private/pypi/pkg_aliases.bzl @@ -36,8 +36,6 @@ load(":whl_target_platforms.bzl", "whl_target_platforms") # it. It is more of an internal consistency check. _VERSION_NONE = (0, 0) -_CONFIG_SETTINGS_PKG = str(Label("//python/config_settings:BUILD.bazel")).partition(":")[0] - _NO_MATCH_ERROR_TEMPLATE = """\ No matching wheel for current configuration's Python version. @@ -49,37 +47,18 @@ configuration settings: To determine the current configuration's Python version, run: `bazel config ` (shown further below) -and look for one of: - {settings_pkg}:python_version - {settings_pkg}:pip_whl - {settings_pkg}:pip_whl_glibc_version - {settings_pkg}:pip_whl_muslc_version - {settings_pkg}:pip_whl_osx_arch - {settings_pkg}:pip_whl_osx_version - {settings_pkg}:py_freethreaded - {settings_pkg}:py_linux_libc - -If the value is missing, then the default value is being used, see documentation: -{docs_url}/python/config_settings""" - -def _no_match_error(actual): - if type(actual) != type({}): - return None - - if "//conditions:default" in actual: - return None - - return _NO_MATCH_ERROR_TEMPLATE.format( - config_settings = render.indent( - "\n".join(sorted([ - value - for key in actual - for value in (key if type(key) == "tuple" else [key]) - ])), - ).lstrip(), - settings_pkg = _CONFIG_SETTINGS_PKG, - docs_url = "https://rules-python.readthedocs.io/en/latest/api/rules_python", - ) +For the current configuration value see the debug message above that is +printing the current flag values. If you can't see the message, then re-run the +build to make it a failure instead by running the build with: + --{current_flags}=fail + +However, the command above will hide the `bazel config ` message. +""" + +_LABEL_NONE = Label("//python:none") +_LABEL_CURRENT_CONFIG = Label("//python/config_settings:current_config") +_LABEL_CURRENT_CONFIG_NO_MATCH = Label("//python/config_settings:is_not_matching_current_config") +_INCOMPATIBLE = "_no_matching_repository" def pkg_aliases( *, @@ -120,7 +99,25 @@ def pkg_aliases( } actual = multiplatform_whl_aliases(aliases = actual, **kwargs) - no_match_error = _no_match_error(actual) + if type(actual) == type({}) and "//conditions:default" not in actual: + native.alias( + name = _INCOMPATIBLE, + actual = select( + {_LABEL_CURRENT_CONFIG_NO_MATCH: _LABEL_NONE}, + no_match_error = _NO_MATCH_ERROR_TEMPLATE.format( + config_settings = render.indent( + "\n".join(sorted([ + value + for key in actual + for value in (key if type(key) == "tuple" else [key]) + ])), + ).lstrip(), + current_flags = str(_LABEL_CURRENT_CONFIG), + ), + ), + visibility = ["//visibility:private"], + ) + actual["//conditions:default"] = _INCOMPATIBLE for name, target_name in target_names.items(): if type(actual) == type(""): @@ -134,10 +131,9 @@ def pkg_aliases( v: "@{repo}//:{target_name}".format( repo = repo, target_name = name, - ) + ) if repo != _INCOMPATIBLE else repo for v, repo in actual.items() }, - no_match_error = no_match_error, ) else: fail("The `actual` arg must be a dictionary or a string") diff --git a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl index 23a0f01db9..f8b0604c99 100644 --- a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl +++ b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl @@ -56,12 +56,8 @@ def _test_config_setting_aliases(env): actual_no_match_error = [] def mock_select(value, no_match_error = None): - actual_no_match_error.append(no_match_error) - env.expect.that_str(no_match_error).contains("""\ -configuration settings: - //:my_config_setting - -""") + if no_match_error and no_match_error not in actual_no_match_error: + actual_no_match_error.append(no_match_error) return value pkg_aliases( @@ -71,7 +67,7 @@ configuration settings: }, extra_aliases = ["my_special"], native = struct( - alias = lambda name, actual: got.update({name: actual}), + alias = lambda name, actual, visibility = None: got.update({name: actual}), ), select = mock_select, ) @@ -80,9 +76,22 @@ configuration settings: want = { "pkg": { "//:my_config_setting": "@bar_baz_repo//:pkg", + "//conditions:default": "_no_matching_repository", }, + # This will be printing the current config values and will make sure we + # have an error. + "_no_matching_repository": {Label("//python/config_settings:is_not_matching_current_config"): Label("//python:none")}, } env.expect.that_dict(got).contains_at_least(want) + env.expect.that_collection(actual_no_match_error).has_size(1) + env.expect.that_str(actual_no_match_error[0]).contains("""\ +configuration settings: + //:my_config_setting + +""") + env.expect.that_str(actual_no_match_error[0]).contains( + "//python/config_settings:current_config=fail", + ) _tests.append(_test_config_setting_aliases) @@ -92,13 +101,8 @@ def _test_config_setting_aliases_many(env): actual_no_match_error = [] def mock_select(value, no_match_error = None): - actual_no_match_error.append(no_match_error) - env.expect.that_str(no_match_error).contains("""\ -configuration settings: - //:another_config_setting - //:my_config_setting - //:third_config_setting -""") + if no_match_error and no_match_error not in actual_no_match_error: + actual_no_match_error.append(no_match_error) return value pkg_aliases( @@ -112,7 +116,8 @@ configuration settings: }, extra_aliases = ["my_special"], native = struct( - alias = lambda name, actual: got.update({name: actual}), + alias = lambda name, actual, visibility = None: got.update({name: actual}), + config_setting = lambda **_: None, ), select = mock_select, ) @@ -125,9 +130,17 @@ configuration settings: "//:another_config_setting", ): "@bar_baz_repo//:my_special", "//:third_config_setting": "@foo_repo//:my_special", + "//conditions:default": "_no_matching_repository", }, } env.expect.that_dict(got).contains_at_least(want) + env.expect.that_collection(actual_no_match_error).has_size(1) + env.expect.that_str(actual_no_match_error[0]).contains("""\ +configuration settings: + //:another_config_setting + //:my_config_setting + //:third_config_setting +""") _tests.append(_test_config_setting_aliases_many) @@ -137,15 +150,8 @@ def _test_multiplatform_whl_aliases(env): actual_no_match_error = [] def mock_select(value, no_match_error = None): - actual_no_match_error.append(no_match_error) - env.expect.that_str(no_match_error).contains("""\ -configuration settings: - //:my_config_setting - //_config:is_cp3.9_linux_x86_64 - //_config:is_cp3.9_py3_none_any - //_config:is_cp3.9_py3_none_any_linux_x86_64 - -""") + if no_match_error and no_match_error not in actual_no_match_error: + actual_no_match_error.append(no_match_error) return value pkg_aliases( @@ -168,7 +174,7 @@ configuration settings: }, extra_aliases = [], native = struct( - alias = lambda name, actual: got.update({name: actual}), + alias = lambda name, actual, visibility = None: got.update({name: actual}), ), select = mock_select, glibc_versions = [], @@ -183,9 +189,19 @@ configuration settings: "//_config:is_cp3.9_linux_x86_64": "@bzlmod_repo_for_a_particular_platform//:pkg", "//_config:is_cp3.9_py3_none_any": "@filename_repo//:pkg", "//_config:is_cp3.9_py3_none_any_linux_x86_64": "@filename_repo_for_platform//:pkg", + "//conditions:default": "_no_matching_repository", }, } env.expect.that_dict(got).contains_at_least(want) + env.expect.that_collection(actual_no_match_error).has_size(1) + env.expect.that_str(actual_no_match_error[0]).contains("""\ +configuration settings: + //:my_config_setting + //_config:is_cp3.9_linux_x86_64 + //_config:is_cp3.9_py3_none_any + //_config:is_cp3.9_py3_none_any_linux_x86_64 + +""") _tests.append(_test_multiplatform_whl_aliases) From 1e3fe43a46aa655b926720c8efdea4bba076325c Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 23 Dec 2024 00:43:18 -0800 Subject: [PATCH 2/5] Update docs/api/rules_python/python/config_settings/index.md --- docs/api/rules_python/python/config_settings/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/rules_python/python/config_settings/index.md b/docs/api/rules_python/python/config_settings/index.md index dcecd8e8fc..793f6e08fd 100644 --- a/docs/api/rules_python/python/config_settings/index.md +++ b/docs/api/rules_python/python/config_settings/index.md @@ -248,7 +248,7 @@ Fail the build if the current build configuration does not match the Values: * `fail`: Will fail in the build action ensuring that we get the error message no matter the action cache. -* ``: The default value, that will just print a warning. +* ``: (empty string) The default value, that will just print a warning. :::{seealso} {obj}`pip.parse` From 617e20a0d0e7953b651a2faac0a1c93920c0f222 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 23 Dec 2024 21:16:49 +0900 Subject: [PATCH 3/5] comment: revert the making attr public --- python/private/config_settings.bzl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index d26e32a1ce..9192ee0d32 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -55,7 +55,6 @@ def construct_config_settings(*, name, default_version, versions, minor_mapping, _python_version_major_minor_flag( name = _PYTHON_VERSION_MAJOR_MINOR_FLAG.name, build_setting_default = "", - python_version_flag = _PYTHON_VERSION_FLAG, visibility = ["//visibility:public"], ) @@ -153,7 +152,7 @@ _python_version_flag = rule( ) def _python_version_major_minor_flag_impl(ctx): - input = _flag_value(ctx.attr.python_version_flag) + input = _flag_value(ctx.attr._python_version_flag) if input: version = semver(input) value = "{}.{}".format(version.major, version.minor) @@ -166,7 +165,7 @@ _python_version_major_minor_flag = rule( implementation = _python_version_major_minor_flag_impl, build_setting = config.string(flag = False), attrs = { - "python_version_flag": attr.label(mandatory = True), + "_python_version_flag": attr.label(default = _PYTHON_VERSION_FLAG), }, ) From 22333e1ab5105bde4524e8d5a8b686c395de6071 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 23 Dec 2024 21:19:20 +0900 Subject: [PATCH 4/5] comment: add a manual tag --- python/private/pypi/pkg_aliases.bzl | 1 + tests/pypi/pkg_aliases/pkg_aliases_test.bzl | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/python/private/pypi/pkg_aliases.bzl b/python/private/pypi/pkg_aliases.bzl index 8229b5d56c..980921b474 100644 --- a/python/private/pypi/pkg_aliases.bzl +++ b/python/private/pypi/pkg_aliases.bzl @@ -116,6 +116,7 @@ def pkg_aliases( ), ), visibility = ["//visibility:private"], + tags = ["manual"], ) actual["//conditions:default"] = _INCOMPATIBLE diff --git a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl index f8b0604c99..f13b62f13d 100644 --- a/tests/pypi/pkg_aliases/pkg_aliases_test.bzl +++ b/tests/pypi/pkg_aliases/pkg_aliases_test.bzl @@ -67,7 +67,7 @@ def _test_config_setting_aliases(env): }, extra_aliases = ["my_special"], native = struct( - alias = lambda name, actual, visibility = None: got.update({name: actual}), + alias = lambda *, name, actual, visibility = None, tags = None: got.update({name: actual}), ), select = mock_select, ) @@ -116,7 +116,7 @@ def _test_config_setting_aliases_many(env): }, extra_aliases = ["my_special"], native = struct( - alias = lambda name, actual, visibility = None: got.update({name: actual}), + alias = lambda *, name, actual, visibility = None, tags = None: got.update({name: actual}), config_setting = lambda **_: None, ), select = mock_select, @@ -174,7 +174,7 @@ def _test_multiplatform_whl_aliases(env): }, extra_aliases = [], native = struct( - alias = lambda name, actual, visibility = None: got.update({name: actual}), + alias = lambda *, name, actual, visibility = None, tags = None: got.update({name: actual}), ), select = mock_select, glibc_versions = [], From 3f5fee60c678ffefe9a51c6ee6f996fe415de8b8 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 23 Dec 2024 21:21:09 +0900 Subject: [PATCH 5/5] fixup! comment: revert the making attr public --- python/private/config_settings.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 9192ee0d32..e5f9d865d1 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -165,7 +165,9 @@ _python_version_major_minor_flag = rule( implementation = _python_version_major_minor_flag_impl, build_setting = config.string(flag = False), attrs = { - "_python_version_flag": attr.label(default = _PYTHON_VERSION_FLAG), + "_python_version_flag": attr.label( + default = _PYTHON_VERSION_FLAG, + ), }, )