bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
394 stars 180 forks source link

Typo in selects.bzl #416

Open Artem-B opened 1 year ago

Artem-B commented 1 year ago

https://github.com/bazelbuild/bazel-skylib/blob/5bfcb1a684550626ce138fe0fe8f5f702b3764c3/lib/selects.bzl#L141

    for setting in settings:
        if settings != "//conditions:default":
            new_settings.append(setting)
    return new_settings

This should've been if setting != "//conditions:default":

Artem-B commented 1 year ago

I've attempted to fix this in the past, but ran into an issue that's currently happens to be hidden by the typo: https://github.com/bazelbuild/bazel-skylib/pull/417

Fixing the typo exposes a more serious issue -- there's a recursion across function calls in the selects.bzl:

ERROR: Traceback (most recent call last):
        File "bazel-skylib/tests/BUILD", line 35, column 19, in <toplevel>
                selects_test_suite()
        File "bazel-skylib/tests/selects_tests.bzl", line 649, column 34, in selects_test_suite
                _create_config_setting_groups()
        File "bazel-skylib/tests/selects_tests.bzl", line 106, column 33, in _create_config_setting_groups
                selects.config_setting_group(
        File "bazel-skylib/lib/selects.bzl", line 125, column 33, in _config_setting_group
                _config_setting_or_group(name, match_any, visibility)
        File "bazel-skylib/lib/selects.bzl", line 158, column 36, in _config_setting_or_group
                _config_setting_always_true(name, visibility)
        File "bazel-skylib/lib/selects.bzl", line 243, column 36, in _config_setting_always_true
                return _config_setting_or_group(name, [":" + name_on, ":" + name_off], visibility)
        File "bazel-skylib/lib/selects.bzl", line 145, column 5, in _config_setting_or_group
                def _config_setting_or_group(name, settings, visibility):
Error: function '_config_setting_or_group' called recursively
ERROR: error loading package 'tests': Package 'tests' contains errors

This needs to be sorted out.

@gregestren -- This is resurrected cl/276532271 that I've never got to fix.

gregestren commented 1 year ago

Thanks for filing.

Do you remember what the user-facing behavior was as a result of the original typo? There was an issue with booleans?

Artem-B commented 1 year ago

The tests in https://github.com/bazelbuild/bazel-skylib/pull/417 reproduce it, if you undo the typo fix. Or, rather, blaze does not even get to the tests failing with a Error: dictionary expression has duplicate key: "//conditions:default"

$ bazel test tests/...
ERROR: Traceback (most recent call last):
        File "/usr/local/google/home/tra/work/bazel/bazel-skylib/tests/BUILD", line 35, column 19, in <toplevel>
                selects_test_suite()
        File "/usr/local/google/home/tra/work/bazel/bazel-skylib/tests/selects_tests.bzl", line 649, column 34, in selects_test_suite
                _create_config_setting_groups()
        File "/usr/local/google/home/tra/work/bazel/bazel-skylib/tests/selects_tests.bzl", line 106, column 33, in _create_config_setting_groups
                selects.config_setting_group(
        File "/usr/local/google/home/tra/work/bazel/bazel-skylib/lib/selects.bzl", line 125, column 33, in _config_setting_group
                _config_setting_or_group(name, match_any, visibility)
        File "/usr/local/google/home/tra/work/bazel/bazel-skylib/lib/selects.bzl", line 182, column 39, in _config_setting_or_group
                "//conditions:default": actual[i - 1],
Error: dictionary expression has duplicate key: "//conditions:default"
ERROR: error loading package 'tests': Package 'tests' contains errors