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.

Sign up for GitHub

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

Jump to bottom

Intermediate modules that register the same python version as rules_python's default prevent identifying default toolchain #1638

Closed
rickeylev opened this issue Dec 20, 2023 · 1 comment · Fixed by #1642
Closed

Intermediate modules that register the same python version as rules_python's default prevent identifying default toolchain #1638

rickeylev opened this issue Dec 20, 2023 · 1 comment · Fixed by #1642
Labels
type: bug type: bzlmod bzlmod work

Comments

@rickeylev
Copy link
Collaborator

rickeylev commented Dec 20, 2023 •
edited
Loading

🐞 bug report

Affected Rule

python.toolchain

Is this a regression?

Exists as far back as 0.25.0, not sure before that.

Description

If an intermediate module registers the same python version as rules_python's default, then it prevents a default toolchain from being identified.

🔬 Minimal Reproduction

The basic repro is to have an earlier module register the same version as what rules_python sets as the default.

# root MODULE
bazel_dep(name="rules_python")

python = use_extension("@rules_python//python/extensions:python.bzl", "python")
# Two must be registered; one is always treated as is_default=True
python.toolchain(python_version="3.11", is_default=False)
python.toolchain(python_version="3.10", is_default=False)

# rules_python MODULE
python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(python_version="3.11", is_default=True)

The error/output is:


... python/extensions/python.bzl:44:10: WARNING: Ignoring toolchain 'python_3_11' from module 'rules_python': Toolchain 'python_3_11' from module 'middle' already registered Python version 3.11 and has precedence
ERROR: Traceback (most recent call last):
	File ".../python/extensions/python.bzl", line 140, column 13, in _python_impl
		fail("No default Python toolchain configured. Is rules_python missing `is_default=True`?")

Anything else relevant?

Blocking rules_proto_grpc getting onto bcr, but also would affect other intermediate modules
@rickeylev rickeylev mentioned this issue Dec 20, 2023
Closed
@rickeylev
Copy link
Collaborator Author

It's late so I gotta pack it in. I think to fix this, the logic needs to do two things:

  1. Track what version (instead of toolchain struct) was marked as default. Then later look up the toolchain struct for that python version
  2. Before skipping a lower-precedence toolchain entirely, check its default-ness so (1) can work.

rickeylev added a commit to rickeylev/rules_python that referenced this issue Dec 20, 2023
@rickeylev
fix(bzlmod): allow modules to register the same toolchain as rules_py…
0ea9c1b 
…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 added a commit to rickeylev/rules_python that referenced this issue Dec 20, 2023
@rickeylev
fix(bzlmod): allow modules to register the same toolchain as rules_py…
0cd2227 
…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 added a commit to rickeylev/rules_python that referenced this issue Dec 20, 2023
@rickeylev
fix(bzlmod): allow modules to register the same toolchain as rules_py…
5fbfeef 
…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 mentioned this issue Dec 20, 2023
Merged
rickeylev added a commit to rickeylev/rules_python that referenced this issue Jan 3, 2024
@rickeylev
fix(bzlmod): allow modules to register the same toolchain as rules_py…
5654c62 
…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
github-merge-queue bot pushed a commit that referenced this issue Jan 3, 2024
@rickeylev
fix(bzlmod): allow modules to register the same toolchain as rules_py…
619fa0c 
…thon's default (#1642)

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
cpsauer added a commit to hedronvision/bazel-compile-commands-extractor that referenced this issue Jan 8, 2024
@cpsauer
Internal: Downgrade to python 3.10
9335ff4 
Avoids rules_python internal clash
Works around bazelbuild/rules_python#1638 until the next rules_python is released
Should fix #157
ngiloq6 added a commit to ngiloq6/bazel-compile-commands-extractor that referenced this issue Aug 17, 2024
@ngiloq6 @cpsauer
Internal: Downgrade to python 3.10
ff2e881 
Avoids rules_python internal clash
Works around bazelbuild/rules_python#1638 until the next rules_python is released
Should fix hedronvision/bazel-compile-commands-extractor#157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: bug type: bzlmod bzlmod work
Projects
None yet
Milestone
No milestone
1 participant
@rickeylev