Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bzlmod): allow modules to register the same toolchain as rules_python's default #1642

Merged

Conversation

rickeylev
Copy link
Contributor

This fixes a bug where, if a module tries to register a non-default toolchain with the
same version as rules_python's default toolchain, an error would occur. This happened
because the earlier (non-default) toolchain caused the later (default) toolchain to
be entirely skipped, and then no default toolchain would be seen. This most affects
intermediary modules that need to register a toolchain, but can't specify a default one.

To fix, just skip creating and registering the duplicate toolchain, but still check
its default-ness to determine if it's the default toolchain.

Fixes #1638

…thon's default

This fixes a bug where, if a module tries to register a non-default
toolchain with the same version as rules_python's default toolchain, an
error would occur. This happened because the earlier (non-default)
toolchain caused the later (default) toolchain to be entirely skipped,
and then no default toolchain would be seen. This most affects
intermediary modules that need to register a toolchain, but can't
specify a default one.

To fix, just skip creating and registering the duplicate toolchain, but
still check its default-ness to determine if it's the default toolchain.

Fixes bazelbuild#1638
@rickeylev rickeylev force-pushed the submodule.reregisters.default.toolchain branch from 5fbfeef to 5654c62 Compare January 3, 2024 21:39
@rickeylev rickeylev added this pull request to the merge queue Jan 3, 2024
Merged via the queue into bazelbuild:main with commit 619fa0c Jan 3, 2024
4 checks passed
@rickeylev rickeylev deleted the submodule.reregisters.default.toolchain branch January 4, 2024 17:57
@cpsauer
Copy link

cpsauer commented Jan 8, 2024

Hit this bug. Thank you so much for fixing. Is there a release that would include it on the horizon?

@rickeylev
Copy link
Contributor Author

Yes. The PR updating the changlog for 0.28.0 is in the merge queue now, a new release will be made once that's merged

@cpsauer
Copy link

cpsauer commented Jan 9, 2024

Amazing. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants