Skip to content

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Oct 26, 2024

#2253 broke how the config settings are generated and only generated the config
setting values for the python version values that we would have the registered
toolchains for. This PR restores the previous behaviour. However, if the root
module uses python.override to remove the allowed toolchains, then
config_settings will be also affected.

Ensure that default toolchain versions are the same as in the
TOOL_VERSIONS array.
…ble versions

I think that bazel-contrib#2253 has introduced an issue that went unnoticed because
we did not have an integration test checking this. The main issue is
that config settings of `is_python_3.x.y` were being generated only for
registered toolchains and that is not necessarily the desired behaviour
-- we may want to use the config settings for more values, so coupling
that was a mistake.
@rickeylev rickeylev added this pull request to the merge queue Oct 26, 2024
Merged via the queue into bazel-contrib:main with commit 9340a81 Oct 26, 2024
4 checks passed
@aignas aignas deleted the fix/config-settings branch October 27, 2024 02:25
aignas added a commit that referenced this pull request Oct 27, 2024
…#2350)

PR #2253 broke how the config settings are generated and only generated the
config
setting values for the python version values that we would have the
registered
toolchains for. This PR restores the previous behaviour. However, if the
root
module uses `python.override` to remove the allowed toolchains, then
`config_settings` will be also affected.

(cherry picked from commit 9340a81)

Conflicts:
- CHANGELOG.md
aignas added a commit that referenced this pull request Oct 27, 2024
…#2350)

PR #2253 broke how the config settings are generated and only generated the
config
setting values for the python version values that we would have the
registered
toolchains for. This PR restores the previous behaviour. However, if the
root
module uses `python.override` to remove the allowed toolchains, then
`config_settings` will be also affected.

(cherry picked from commit 9340a81)

Conflicts:
- CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants