Skip to content

Commit

Permalink
refactor(flags): return FeatureFlagInfo in --python_version flag (#2167)
Browse files Browse the repository at this point in the history
Make the `--python_version` flag also return the `FeatureFlagInfo`
provider.

There are two reasons to also return the FeatureFlagInfo provider:

First, it allows the flag implementation to change the value and have
that value respected
by config_setting() later. This allows, for example, the rule to use
custom logic (and
information from things it depends on) to determine the effective flag
value.

Secondly, it makes the flag compatible with the Google fork of this
rule, which is
implemented using FeatureFlagInfo, to help eventually converge them.

Along the way, add config_common to the Sphinx Bazel inventory.
  • Loading branch information
rickeylev committed Aug 31, 2024
1 parent 4f2dd2f commit 54c9fab
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ A brief description of the categories of changes:

### Changed
* (gazelle): Update error messages when unable to resolve a dependency to be more human-friendly.
* (flags) The {obj}`--python_version` flag now also returns
{obj}`config_common.FeatureFlagInfo`.

### Fixed
* (whl_library): Remove `--no-index` and add `--no-build-isolation` to the
Expand Down
36 changes: 34 additions & 2 deletions python/private/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"""

load("@bazel_skylib//lib:selects.bzl", "selects")
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS")

_PYTHON_VERSION_FLAG = str(Label("//python/config_settings:python_version"))
Expand Down Expand Up @@ -170,7 +170,7 @@ def construct_config_settings(name = None): # buildifier: disable=function-docs
Args:
name(str): A dummy name value that is no-op for now.
"""
string_flag(
_python_version_flag(
name = "python_version",
# TODO: The default here should somehow match the MODULE config. Until
# then, use the empty string to indicate an unknown version. This
Expand Down Expand Up @@ -200,3 +200,35 @@ def construct_config_settings(name = None): # buildifier: disable=function-docs
},
visibility = ["//visibility:public"],
)

def _python_version_flag_impl(ctx):
value = ctx.build_setting_value
if value not in ctx.attr.values:
fail((
"Invalid --python_version value: {actual}\nAllowed values {allowed}"
).format(
actual = value,
allowed = ", ".join(sorted(ctx.attr.values)),
))

return [
# BuildSettingInfo is the original provider returned, so continue to
# return it for compatibility
BuildSettingInfo(value = value),
# FeatureFlagInfo is returned so that config_setting respects the value
# as returned by this rule instead of as originally seen on the command
# line.
# It is also for Google compatibility, which expects the FeatureFlagInfo
# provider.
config_common.FeatureFlagInfo(value = value),
]

_python_version_flag = rule(
implementation = _python_version_flag_impl,
build_setting = config.string(flag = True),
attrs = {
"values": attr.string_list(
doc = "Allowed values.",
),
},
)
3 changes: 3 additions & 0 deletions sphinxdocs/inventories/bazel_inventory.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ File bzl:type 1 rules/lib/File -
Label bzl:type 1 rules/lib/Label -
Target bzl:type 1 rules/lib/builtins/Target -
bool bzl:type 1 rules/lib/bool -
config_common.FeatureFlagInfo bzl:type 1 rules/lib/toplevel/config_common#FeatureFlagInfo -
config_common.toolchain_type bzl:function 1 rules/lib/toplevel/config_common#toolchain_type -
ctx.actions bzl:obj 1 rules/lib/builtins/ctx#actions -
ctx.aspect_ids bzl:obj 1 rules/lib/builtins/ctx#aspect_ids -
ctx.attr bzl:obj 1 rules/lib/builtins/ctx#attr -
Expand Down Expand Up @@ -65,6 +67,7 @@ native.repo_name bzl:function 1 rules/lib/toplevel/native#repo_name -
native.repository_name bzl:function 1 rules/lib/toplevel/native#repository_name -
str bzl:type 1 rules/lib/string -
struct bzl:type 1 rules/lib/builtins/struct -
toolchain_type bzl:type 1 ules/lib/builtins/toolchain_type.html -
Name bzl:type 1 concepts/labels#target-names -
CcInfo bzl:provider 1 rules/lib/providers/CcInfo -
CcInfo.linking_context bzl:provider-field 1 rules/lib/providers/CcInfo#linking_context -
Expand Down

0 comments on commit 54c9fab

Please sign in to comment.