From 1a1b1914f6220cb28db973c7c14b103216889891 Mon Sep 17 00:00:00 2001 From: aignas <240938+aignas@users.noreply.github.com> Date: Mon, 12 Feb 2024 09:50:32 +0900 Subject: [PATCH] internal(config_settings): make config_setting creation reusable The PR #1743 explored the idea of creating extra config settings for each target platform that our toolchain is targetting, however that has a drawback of not being usable in `bzlmod` if someone built Python for a platform that we don't provide a toolchain for and tried to use the `pip.parse` machinery with that by providing the `python_interpreter_target`. That is a niche usecase, but `rules_python` is a core ruleset that should only provide abstractions/helpers that work in all cases or make it possible to extend things. This explores a way to decouple the definition of the available `config_settings` values and how they are constructed by adding an extra `is_python_config_setting` macro, that could be used to declare the config settings from within the `pip.parse` hub repo. This makes the work in #1744 to support whl-only hub repos more self-contained. Supersedes #1743. --- python/config_settings/BUILD.bazel | 32 +++- python/config_settings/config_settings.bzl | 173 +++++++++++---------- 2 files changed, 116 insertions(+), 89 deletions(-) diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 4f12ef4791..d08640a114 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -1,5 +1,5 @@ -load("//python:versions.bzl", "TOOL_VERSIONS") -load(":config_settings.bzl", "construct_config_settings") +load("@bazel_skylib//rules:common_settings.bzl", "string_flag") +load(":config_settings.bzl", "VERSION_FLAG_VALUES", "is_python_config_setting") filegroup( name = "distribution", @@ -10,7 +10,29 @@ filegroup( visibility = ["//python:__pkg__"], ) -construct_config_settings( - name = "construct_config_settings", - python_versions = TOOL_VERSIONS.keys(), +string_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 + # also prevents version-unaware targets from inadvertently matching + # a select condition when they shouldn't. + build_setting_default = "", + values = [""] + VERSION_FLAG_VALUES.keys(), + visibility = ["//visibility:public"], ) + +[ + is_python_config_setting( + name = "is_python_{}".format(version), + flag_values = {":python_version": version}, + match_extra = [ + # Use the internal labels created by this macro in order to handle matching + # 3.8 config value if using the 3.8 version from MINOR_MAPPING. If we used the + # public labels we would have a circular dependency loop. + ("_is_python_{}" if VERSION_FLAG_VALUES[x] else "is_python_{}").format(x) + for x in extras + ], + visibility = ["//visibility:public"], + ) + for version, extras in VERSION_FLAG_VALUES.items() +] diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 9e6bbd6595..0baec49bfe 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -16,104 +16,109 @@ """ load("@bazel_skylib//lib:selects.bzl", "selects") -load("@bazel_skylib//rules:common_settings.bzl", "string_flag") -load("//python:versions.bzl", "MINOR_MAPPING") +load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") -def construct_config_settings(name, python_versions): - """Constructs a set of configs for all Python versions. +def _ver_key(s): + _, _, s = s.partition(".") # All are 3 + minor, _, s = s.partition(".") + micro, _, s = s.partition(".") + return 100 * int(minor) + int(micro) + +def _flag_values(python_versions): + """Construct a map of python_version to a list of toolchain values. + + This mapping maps the concept of a config setting to a list of compatible toolchain versions. + For using this in the code, the VERSION_FLAG_VALUES should be used instead. Args: - name: str, unused; only specified to satisfy buildifier lint checks - and allow programatic modification of the target. - python_versions: list of all (x.y.z) Python versions supported by rules_python. + python_versions: A list of all versions. + + Returns: + A map with config settings as keys and values as extra flag values to be included in + the config_setting_group if they should be also matched, which is used for generating + correct entries for matching the latest 3.8 version, etc. """ # Maps e.g. "3.8" -> ["3.8.1", "3.8.2", etc] - minor_to_micro_versions = {} - - allowed_flag_values = [] - for micro_version in python_versions: - minor, _, _ = micro_version.rpartition(".") - minor_to_micro_versions.setdefault(minor, []).append(micro_version) - allowed_flag_values.append(micro_version) - - allowed_flag_values.extend(list(minor_to_micro_versions)) - - string_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 - # also prevents version-unaware targets from inadvertently matching - # a select condition when they shouldn't. - build_setting_default = "", - values = [""] + sorted(allowed_flag_values), - visibility = ["//visibility:public"], - ) + ret = {} + + for micro_version in sorted(python_versions, key = _ver_key): + minor_version, _, _ = micro_version.rpartition(".") - for minor_version, micro_versions in minor_to_micro_versions.items(): # This matches the raw flag value, e.g. --//python/config_settings:python_version=3.8 # It's private because matching the concept of e.g. "3.8" value is done # using the `is_python_X.Y` config setting group, which is aware of the # minor versions that could match instead. - equals_minor_version_name = "_python_version_flag_equals_" + minor_version - native.config_setting( - name = equals_minor_version_name, - flag_values = {":python_version": minor_version}, - ) - matches_minor_version_names = [equals_minor_version_name] + ret.setdefault(minor_version, []).append(micro_version) + # Ensure that is_python_3.9.8 is matched if python_version is set + # to 3.9 if MINOR_MAPPING points to 3.9.8 default_micro_version = MINOR_MAPPING[minor_version] + ret[micro_version] = [minor_version] if default_micro_version == micro_version else [] + + return ret + +VERSION_FLAG_VALUES = _flag_values(TOOL_VERSIONS.keys()) + +def is_python_config_setting(name, flag_values, match_extra, **kwargs): + """Create a config setting for matching 'python_version' configuration flag. - for micro_version in micro_versions: - is_micro_version_name = "is_python_" + micro_version - if default_micro_version != micro_version: - native.config_setting( - name = is_micro_version_name, - flag_values = {":python_version": micro_version}, - visibility = ["//visibility:public"], - ) - matches_minor_version_names.append(is_micro_version_name) - continue - - # Ensure that is_python_3.9.8 is matched if python_version is set - # to 3.9 if MINOR_MAPPING points to 3.9.8 - equals_micro_name = "_python_version_flag_equals_" + micro_version - native.config_setting( - name = equals_micro_name, - flag_values = {":python_version": micro_version}, - ) - - # An alias pointing to an underscore-prefixed config_setting_group - # is used because config_setting_group creates - # `is_{minor}_N` targets, which are easily confused with the - # `is_{minor}.{micro}` (dot) targets. - selects.config_setting_group( - name = "_" + is_micro_version_name, - match_any = [ - equals_micro_name, - equals_minor_version_name, - ], - ) - native.alias( - name = is_micro_version_name, - actual = "_" + is_micro_version_name, - visibility = ["//visibility:public"], - ) - matches_minor_version_names.append(equals_micro_name) - - # This is prefixed with an underscore to prevent confusion due to how - # config_setting_group is implemented and how our micro-version targets - # are named. config_setting_group will generate targets like - # "is_python_3.10_1" (where the `_N` suffix is len(match_any). - # Meanwhile, the micro-version tarets are named "is_python_3.10.1" -- - # just a single dot vs underscore character difference. - selects.config_setting_group( - name = "_is_python_" + minor_version, - match_any = matches_minor_version_names, + This function is mainly intended for internal use within the `whl_library` and `pip_parse` + machinery. + + Args: + name: name for the target that will be created to be used in select statements. + flag_values: The flag_values in the `config_setting`. + match_extra: The extra flag values that we should matched when the `name` is used + in the config setting. You can either pass a list of labels that will be included + in the bazel-skylib selects.config_setting_group match_any clause or it can be a + dict[str, dic], where the keys are the names of the extra config_setting targets + to be created and the value is the `flag_values` attribute. + **kwargs: extra kwargs passed to the `config_setting` + """ + visibility = kwargs.pop("visibility", []) + if not match_extra: + native.config_setting( + name = name, + flag_values = flag_values, + visibility = visibility, + **kwargs ) + return + + create_config_settings = {"_" + name: flag_values} + match_any = ["_" + name] + if type(match_extra) == type([]): + match_any.extend(match_extra) + elif type(match_extra) == type({}): + match_any.extend(match_extra.keys()) + create_config_settings.update(match_extra) + else: + fail("unsupported match_extra type, can be either a list or a dict of dicts") - native.alias( - name = "is_python_" + minor_version, - actual = "_is_python_" + minor_version, - visibility = ["//visibility:public"], + # Create all of the necessary config setting values for the config_setting_group + for name_, flag_values_ in create_config_settings.items(): + native.config_setting( + name = name_, + flag_values = flag_values_, + # We need to pass the visibility here because of how `config_setting_group` is + # implemented, it is using the internal aliases here, hence the need for making + # them with the same visibility as the `alias` itself. + visibility = visibility, + **kwargs ) + + # An alias pointing to an underscore-prefixed config_setting_group + # is used because config_setting_group creates + # `is_{version}_N` targets, which are easily confused with the + # `is_{minor}.{micro}` (dot) targets. + selects.config_setting_group( + name = "_{}_group".format(name), + match_any = match_any, + visibility = ["//visibility:private"], + ) + native.alias( + name = name, + actual = "_{}_group".format(name), + visibility = visibility, + )