feat(config_settings): add extra settings for toolchain platforms#1743
Closed
feat(config_settings): add extra settings for toolchain platforms#1743
Conversation
aignas
commented
Feb 8, 2024
python/pip_install/private/generate_whl_library_build_bazel.bzl
Outdated
Show resolved
Hide resolved
aignas
commented
Feb 8, 2024
aignas
commented
Feb 8, 2024
python/pip_install/private/generate_whl_library_build_bazel.bzl
Outdated
Show resolved
Hide resolved
Collaborator
Author
|
Thinking about it a little more, I am wondering if it is OK to have this code in here. It may be better to not expose extra That said, having the mapping between a python version string and the expected flag values that are passed to Feel free to comment if you disagree with my statement above. |
aignas
added a commit
that referenced
this pull request
Feb 12, 2024
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.
aignas
added a commit
that referenced
this pull request
Feb 12, 2024
aignas
added a commit
that referenced
this pull request
Feb 12, 2024
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.
aignas
added a commit
to aignas/rules_python
that referenced
this pull request
Mar 16, 2024
The PR bazel-contrib#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 bazel-contrib#1744 to support whl-only hub repos more self-contained. Supersedes bazel-contrib#1743.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 18, 2024
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. --------- Co-authored-by: Richard Levasseur <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prior to this PR
//python/config_settingswould contain only theconfig_settingtargets for the Python versions from//python:versions.bzl.We would only match the
python_versionconfiguration and if someone neededmatching against the
constraint_valuesthey would have to redefine theconfig_settingvalues themselves asmatch_allhelper from bazel skylibcannot be used in this case do to how specialization priority of
config_settingin aselectworks -match_allwould createselectstatement entries that are not specialized enough, i.e. they would not have the
python version config setting and the target platform constraint values in a single
config_setting.This PR creates additional
config_settingvalues that are useful for rulesetsimplementing handling of platform-specific wheels in a multi-python version scenario
and it lays a good foundation for supporting
download_only = Truein a better waywithin the
pip.parseextension itself.NOTE:
config_settingvalues are only created for the Python toolchain targetplatforms.