From 7de71890624a9c52116d4112b7a728a525c53f60 Mon Sep 17 00:00:00 2001 From: Alexey Preobrazhenskiy Date: Wed, 25 Oct 2023 13:37:25 +0200 Subject: [PATCH 1/8] feat(config_settings): accept minor versions of Python in `python_version` flag Currently user has to specify a full (x.y.z) version of Python when setting the `python_version` flag. When they upgrade `rules_python` or change `MINOR_MAPPING` in some other way, user has to update the flag's value to keep it in sync with `MINOR_MAPPING`. This patch allows `python_version` flag to accept minor (x.y) versions and resolves them to the corresponding full versions using `MINOR_MAPPING`. --- CHANGELOG.md | 2 ++ python/config_settings/BUILD.bazel | 7 ++-- python/config_settings/config_settings.bzl | 38 ++++++++++++++++++---- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc3b079d9d..1a4f3e7324 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,8 @@ Breaking changes: * (utils) Added a `pip_utils` struct with a `normalize_name` function to allow users to find out how `rules_python` would normalize a PyPI distribution name. +* (config_settings) The `python_version` flag now accepts minor Python versions. + ## [0.26.0] - 2023-10-06 ### Changed diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index ab4ee8d880..04e820bb3f 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -1,4 +1,4 @@ -load("//python:versions.bzl", "TOOL_VERSIONS") +load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") load(":config_settings.bzl", "construct_config_settings") filegroup( @@ -10,4 +10,7 @@ filegroup( visibility = ["//python:__pkg__"], ) -construct_config_settings(python_versions = TOOL_VERSIONS.keys()) +construct_config_settings( + minor_mapping = MINOR_MAPPING, + python_versions = TOOL_VERSIONS.keys(), +) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 21e477e644..363205f048 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -15,26 +15,52 @@ """This module is used to construct the config settings in the BUILD file in this same package. """ +load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "string_flag") # buildifier: disable=unnamed-macro -def construct_config_settings(python_versions): +def construct_config_settings(minor_mapping, python_versions): """Constructs a set of configs for all Python versions. Args: - python_versions: The Python versions supported by rules_python. + minor_mapping: mapping from minor (x.y) versions to the corresponding full (x.y.z) versions. + python_versions: list of full (x.y.z) Python versions supported by rules_python. """ + + minor_versions = list(minor_mapping.keys()) + allowed_flag_values = python_versions + minor_versions + string_flag( name = "python_version", build_setting_default = python_versions[0], - values = python_versions, + values = allowed_flag_values, visibility = ["//visibility:public"], ) - for python_version in python_versions: - python_version_constraint_setting = "is_python_" + python_version + for flag_value in allowed_flag_values: + flag_value_constraint_setting = "python_version_flag_equals_" + flag_value native.config_setting( + name = flag_value_constraint_setting, + flag_values = {":python_version": flag_value}, + visibility = ["//visibility:public"], + ) + + flag_values_that_enable_version = { + full_version: [full_version] + for full_version in python_versions + } + + for minor_version, full_version in minor_mapping.items(): + flag_values_that_enable_version[full_version].append(minor_version) + + for full_version, flag_values in flag_values_that_enable_version.items(): + python_version_constraint_setting = "is_python_" + full_version + flag_value_constraints = [ + ":python_version_flag_equals_" + flag_value + for flag_value in flag_values + ] + selects.config_setting_group( name = python_version_constraint_setting, - flag_values = {":python_version": python_version}, + match_any = flag_value_constraints, visibility = ["//visibility:public"], ) From 768417c5fd7407ab7e600f015e8c9392c455884d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:21:27 -0800 Subject: [PATCH 2/8] generate fewer settings to match across versions --- python/config_settings/BUILD.bazel | 2 +- python/config_settings/config_settings.bzl | 84 +++++++++++++++------- tests/config_settings/BUILD.bazel | 19 +++++ 3 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 tests/config_settings/BUILD.bazel diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 04e820bb3f..44cdd3ea2b 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -11,6 +11,6 @@ filegroup( ) construct_config_settings( - minor_mapping = MINOR_MAPPING, + name = "construct_config_settings", python_versions = TOOL_VERSIONS.keys(), ) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 363205f048..339e7c69b1 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -18,49 +18,81 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@bazel_skylib//rules:common_settings.bzl", "string_flag") -# buildifier: disable=unnamed-macro -def construct_config_settings(minor_mapping, python_versions): +def construct_config_settings(name, python_versions): """Constructs a set of configs for all Python versions. Args: - minor_mapping: mapping from minor (x.y) versions to the corresponding full (x.y.z) versions. - python_versions: list of full (x.y.z) Python versions supported by rules_python. + python_versions: list of all (x.y.z) Python versions supported by rules_python. """ - minor_versions = list(minor_mapping.keys()) - allowed_flag_values = python_versions + minor_versions + # 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(minor) + allowed_flag_values.append(micro_version) string_flag( name = "python_version", build_setting_default = python_versions[0], - values = allowed_flag_values, + values = sorted(allowed_flag_values), visibility = ["//visibility:public"], ) - for flag_value in allowed_flag_values: - flag_value_constraint_setting = "python_version_flag_equals_" + flag_value + 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 = flag_value_constraint_setting, - flag_values = {":python_version": flag_value}, - visibility = ["//visibility:public"], + name = equals_minor_version_name, + flag_values = {":python_version": minor_version}, ) - flag_values_that_enable_version = { - full_version: [full_version] - for full_version in python_versions - } + matches_minor_version_names = [equals_minor_version_name] - for minor_version, full_version in minor_mapping.items(): - flag_values_that_enable_version[full_version].append(minor_version) + for micro_version in micro_versions: + is_micro_version_name = "is_python_" + 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) - for full_version, flag_values in flag_values_that_enable_version.items(): - python_version_constraint_setting = "is_python_" + full_version - flag_value_constraints = [ - ":python_version_flag_equals_" + flag_value - for flag_value in flag_values - ] selects.config_setting_group( - name = python_version_constraint_setting, - match_any = flag_value_constraints, + name = "is_python_" + minor_version, + match_any = matches_minor_version_names, visibility = ["//visibility:public"], ) + + ##for flag_value in allowed_flag_values: + ## flag_value_constraint_setting = "_python_version_flag_equals_" + flag_value + ## native.config_setting( + ## name = flag_value_constraint_setting, + ## flag_values = {":python_version": flag_value}, + ## visibility = ["//visibility:public"], + ## ) + + ##flag_values_that_enable_version = { + ## full_version: [full_version] + ## for full_version in python_versions + ##} + + ##for minor_version, full_version in minor_mapping.items(): + ## flag_values_that_enable_version[full_version].append(minor_version) + + ##for full_version, flag_values in flag_values_that_enable_version.items(): + ## python_version_constraint_setting = "is_python_" + full_version + ## flag_value_constraints = [ + ## ":python_version_flag_equals_" + flag_value + ## for flag_value in flag_values + ## ] + ## selects.config_setting_group( + ## name = python_version_constraint_setting, + ## match_any = flag_value_constraints, + ## visibility = ["//visibility:public"], + ## ) diff --git a/tests/config_settings/BUILD.bazel b/tests/config_settings/BUILD.bazel new file mode 100644 index 0000000000..519408d175 --- /dev/null +++ b/tests/config_settings/BUILD.bazel @@ -0,0 +1,19 @@ +# Copyright 2022 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load(":construct_config_settings_test.bzl", "construct_config_settings_test_suite") + +construct_config_settings_test_suite( + name = "construct_config_settings_tests", +) From 28d950f85b62b5922975b51b309b20e22b2306d1 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:29:13 -0800 Subject: [PATCH 3/8] rename test file to be consistent, expand test cases --- python/config_settings/config_settings.bzl | 28 ------- tests/config_settings/BUILD.bazel | 2 +- .../construct_config_settings_tests.bzl | 81 +++++++++++++++++++ 3 files changed, 82 insertions(+), 29 deletions(-) create mode 100644 tests/config_settings/construct_config_settings_tests.bzl diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 339e7c69b1..c424d316b3 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -68,31 +68,3 @@ def construct_config_settings(name, python_versions): match_any = matches_minor_version_names, visibility = ["//visibility:public"], ) - - ##for flag_value in allowed_flag_values: - ## flag_value_constraint_setting = "_python_version_flag_equals_" + flag_value - ## native.config_setting( - ## name = flag_value_constraint_setting, - ## flag_values = {":python_version": flag_value}, - ## visibility = ["//visibility:public"], - ## ) - - ##flag_values_that_enable_version = { - ## full_version: [full_version] - ## for full_version in python_versions - ##} - - ##for minor_version, full_version in minor_mapping.items(): - ## flag_values_that_enable_version[full_version].append(minor_version) - - ##for full_version, flag_values in flag_values_that_enable_version.items(): - ## python_version_constraint_setting = "is_python_" + full_version - ## flag_value_constraints = [ - ## ":python_version_flag_equals_" + flag_value - ## for flag_value in flag_values - ## ] - ## selects.config_setting_group( - ## name = python_version_constraint_setting, - ## match_any = flag_value_constraints, - ## visibility = ["//visibility:public"], - ## ) diff --git a/tests/config_settings/BUILD.bazel b/tests/config_settings/BUILD.bazel index 519408d175..212e3f7b02 100644 --- a/tests/config_settings/BUILD.bazel +++ b/tests/config_settings/BUILD.bazel @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load(":construct_config_settings_test.bzl", "construct_config_settings_test_suite") +load(":construct_config_settings_tests.bzl", "construct_config_settings_test_suite") construct_config_settings_test_suite( name = "construct_config_settings_tests", diff --git a/tests/config_settings/construct_config_settings_tests.bzl b/tests/config_settings/construct_config_settings_tests.bzl new file mode 100644 index 0000000000..2a264f9376 --- /dev/null +++ b/tests/config_settings/construct_config_settings_tests.bzl @@ -0,0 +1,81 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test") +load("@rules_testing//lib:truth.bzl", "subjects") +load("@rules_testing//lib:test_suite.bzl", "test_suite") +load("@rules_testing//lib:util.bzl", rt_util = "util") +load("//python:versions.bzl", "TOOL_VERSIONS") +load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") + +_tests = [] + +def _subject_impl(ctx): + return [DefaultInfo()] + +_subject = rule( + implementation = _subject_impl, + attrs = { + "match_minor": attr.string(), + "match_micro": attr.string(), + "no_match": attr.string(), + }, +) + +def _test_minor_version_matching(name): + rt_util.helper_target( + _subject, + name = name + "_subject", + match_minor = select({ + "//python/config_settings:is_python_3.11": "matched-3.11", + "//conditions:default": "matched-default", + }), + match_micro = select({ + "//python/config_settings:is_python_3.11": "matched-3.11", + "//conditions:default": "matched-default", + }), + no_match = select({ + "//python/config_settings:is_python_3.12": "matched-3.12", + "//conditions:default": "matched-default", + }), + ) + + analysis_test( + name = name, + target = name + "_subject", + impl = _test_minor_version_matching_impl, + config_settings = { + str(Label("//python/config_settings:python_version")): "3.11.1", + }, + ) + +def _test_minor_version_matching_impl(env, target): + target = env.expect.that_target(target) + target.attr("match_minor", factory = subjects.str).equals( + "matched-3.11", + ) + target.attr("match_micro", factory = subjects.str).equals( + "matched-3.11", + ) + target.attr("no_match", factory = subjects.str).equals( + "matched-default", + ) + +_tests.append(_test_minor_version_matching) + +def construct_config_settings_test_suite(name): + test_suite( + name = name, + tests = _tests, + ) From f76a5d5e5ddecffb35a70d5c4e875915f7df40eb Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:34:20 -0800 Subject: [PATCH 4/8] fix bug with repeated allowed values, add note about default value --- python/config_settings/config_settings.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index c424d316b3..4e3a189fbb 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -31,11 +31,13 @@ def construct_config_settings(name, python_versions): for micro_version in python_versions: minor, _, _ = micro_version.rpartition(".") minor_to_micro_versions.setdefault(minor, []).append(micro_version) - allowed_flag_values.append(minor) allowed_flag_values.append(micro_version) + allowed_flag_values.extend(minor_to_micro_versions.keys()) + string_flag( name = "python_version", + # TODO: The default here should somehow match the MODULE config build_setting_default = python_versions[0], values = sorted(allowed_flag_values), visibility = ["//visibility:public"], From 497123e63aaa177118cff5608e0b828c7f1f36ff Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:42:24 -0800 Subject: [PATCH 5/8] update changelog to better reflect feature added --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e65a509941..c689e9d6b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,9 @@ A brief description of the categories of changes: `python_register_toolchains`. Note that this only available on the Starlark implementation of the provider. -* (config_settings) The `python_version` flag now accepts minor Python versions. +* (config_settings) Added `//python/config_settings:is_python_X.Y` config + settings to match on minor Python version. These settings match any `X.Y` + version instead of just an exact `X.Y.Z` version. ## [0.28.0] - 2024-01-07 From b943bc8f026d4d7ca64cfad1e38c0f035a4518f1 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:46:06 -0800 Subject: [PATCH 6/8] fix lint errors --- python/config_settings/BUILD.bazel | 2 +- python/config_settings/config_settings.bzl | 2 ++ .../config_settings/construct_config_settings_tests.bzl | 9 ++++----- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 44cdd3ea2b..4f12ef4791 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -1,4 +1,4 @@ -load("//python:versions.bzl", "MINOR_MAPPING", "TOOL_VERSIONS") +load("//python:versions.bzl", "TOOL_VERSIONS") load(":config_settings.bzl", "construct_config_settings") filegroup( diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 4e3a189fbb..91e7c31928 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -22,6 +22,8 @@ def construct_config_settings(name, python_versions): """Constructs a set of configs for all Python versions. 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. """ diff --git a/tests/config_settings/construct_config_settings_tests.bzl b/tests/config_settings/construct_config_settings_tests.bzl index 2a264f9376..1ed3f6c255 100644 --- a/tests/config_settings/construct_config_settings_tests.bzl +++ b/tests/config_settings/construct_config_settings_tests.bzl @@ -11,24 +11,23 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +"""Tests for construction of Python version matching config settings.""" load("@rules_testing//lib:analysis_test.bzl", "analysis_test") -load("@rules_testing//lib:truth.bzl", "subjects") load("@rules_testing//lib:test_suite.bzl", "test_suite") -load("@rules_testing//lib:util.bzl", rt_util = "util") -load("//python:versions.bzl", "TOOL_VERSIONS") -load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") +load("@rules_testing//lib:truth.bzl", "subjects") _tests = [] def _subject_impl(ctx): + _ = ctx # @unused return [DefaultInfo()] _subject = rule( implementation = _subject_impl, attrs = { - "match_minor": attr.string(), "match_micro": attr.string(), + "match_minor": attr.string(), "no_match": attr.string(), }, ) From 8bcdd00ece7fcbcaddc1f00d194a0d5a1a770dd9 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 18:57:37 -0800 Subject: [PATCH 7/8] dont accept minor version as flag value, add missing import in tests --- python/config_settings/config_settings.bzl | 2 -- tests/config_settings/construct_config_settings_tests.bzl | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 91e7c31928..15cf4a45f5 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -35,8 +35,6 @@ def construct_config_settings(name, python_versions): minor_to_micro_versions.setdefault(minor, []).append(micro_version) allowed_flag_values.append(micro_version) - allowed_flag_values.extend(minor_to_micro_versions.keys()) - string_flag( name = "python_version", # TODO: The default here should somehow match the MODULE config diff --git a/tests/config_settings/construct_config_settings_tests.bzl b/tests/config_settings/construct_config_settings_tests.bzl index 1ed3f6c255..8e4217139f 100644 --- a/tests/config_settings/construct_config_settings_tests.bzl +++ b/tests/config_settings/construct_config_settings_tests.bzl @@ -16,6 +16,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:truth.bzl", "subjects") +load("@rules_testing//lib:util.bzl", rt_util = "util") _tests = [] From 997d37c926064d5f8fb4a76930ccefaaa8f7bfb0 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Jan 2024 22:18:27 -0800 Subject: [PATCH 8/8] alias config group for better ux; fix typo --- python/config_settings/config_settings.bzl | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/python/config_settings/config_settings.bzl b/python/config_settings/config_settings.bzl index 15cf4a45f5..bd4a1b2166 100644 --- a/python/config_settings/config_settings.bzl +++ b/python/config_settings/config_settings.bzl @@ -44,7 +44,7 @@ def construct_config_settings(name, python_versions): ) 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 + # 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. @@ -65,8 +65,19 @@ def construct_config_settings(name, python_versions): ) matches_minor_version_names.append(is_micro_version_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, + name = "_is_python_" + minor_version, match_any = matches_minor_version_names, + ) + + native.alias( + name = "is_python_" + minor_version, + actual = "_is_python_" + minor_version, visibility = ["//visibility:public"], )