Skip to content

Commit

Permalink
refactor(pypi): A better error message when the wheel select hits no_…
Browse files Browse the repository at this point in the history
…match (#2519)

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 <config id>` (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 <config id>` 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

---------

Co-authored-by: Richard Levasseur <[email protected]>
  • Loading branch information
aignas and rickeylev authored Dec 23, 2024
1 parent e3c9406 commit be950f9
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 63 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* (pypi) 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).
* (py_proto_library) Fix import paths in Bazel 8.

[pep-695]: https://peps.python.org/pep-0695/
Expand Down
18 changes: 18 additions & 0 deletions docs/api/rules_python/python/config_settings/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* ``: (empty string) The default value, that will just print a warning.

:::{seealso}
{obj}`pip.parse`
:::

:::{versionadded} 1.1.0
:::

::::
9 changes: 9 additions & 0 deletions python/config_settings/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
73 changes: 71 additions & 2 deletions python/private/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -101,6 +112,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 [
Expand All @@ -122,7 +152,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)
Expand All @@ -140,3 +170,42 @@ _python_version_major_minor_flag = rule(
),
},
)

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),
},
)
69 changes: 33 additions & 36 deletions python/private/pypi/pkg_aliases.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -49,37 +47,18 @@ configuration settings:
To determine the current configuration's Python version, run:
`bazel config <config id>` (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 <config id>` 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(
*,
Expand Down Expand Up @@ -120,7 +99,26 @@ 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"],
tags = ["manual"],
)
actual["//conditions:default"] = _INCOMPATIBLE

for name, target_name in target_names.items():
if type(actual) == type(""):
Expand All @@ -134,10 +132,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")
Expand Down
66 changes: 41 additions & 25 deletions tests/pypi/pkg_aliases/pkg_aliases_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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, tags = None: got.update({name: actual}),
),
select = mock_select,
)
Expand All @@ -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)

Expand All @@ -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(
Expand All @@ -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, tags = None: got.update({name: actual}),
config_setting = lambda **_: None,
),
select = mock_select,
)
Expand All @@ -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)

Expand All @@ -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(
Expand All @@ -168,7 +174,7 @@ configuration settings:
},
extra_aliases = [],
native = struct(
alias = lambda name, actual: got.update({name: actual}),
alias = lambda *, name, actual, visibility = None, tags = None: got.update({name: actual}),
),
select = mock_select,
glibc_versions = [],
Expand All @@ -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)

Expand Down

0 comments on commit be950f9

Please sign in to comment.