From 54c9fab03af56869ac5b36fb2ab614a63a40a7ab Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sat, 31 Aug 2024 07:19:47 -0700
Subject: [PATCH] refactor(flags): return FeatureFlagInfo in --python_version
 flag (#2167)

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.
---
 CHANGELOG.md                               |  2 ++
 python/private/config_settings.bzl         | 36 ++++++++++++++++++++--
 sphinxdocs/inventories/bazel_inventory.txt |  3 ++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 387b55b0c9..ee71951b44 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl
index 0537655a47..99b8b94adf 100644
--- a/python/private/config_settings.bzl
+++ b/python/private/config_settings.bzl
@@ -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"))
@@ -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
@@ -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.",
+        ),
+    },
+)
diff --git a/sphinxdocs/inventories/bazel_inventory.txt b/sphinxdocs/inventories/bazel_inventory.txt
index 445f0f71f4..caf5866d8a 100644
--- a/sphinxdocs/inventories/bazel_inventory.txt
+++ b/sphinxdocs/inventories/bazel_inventory.txt
@@ -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 -
@@ -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 -