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 non-strict toolchain's precedence and diagnostic message #8576

Closed
brandjon opened this issue Jun 6, 2019 · 0 comments
Closed

Fix non-strict toolchain's precedence and diagnostic message #8576

brandjon opened this issue Jun 6, 2019 · 0 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Python Native rules for Python type: bug

Comments

@brandjon
Copy link
Member

brandjon commented Jun 6, 2019

052167e (for #8547) added a non-strict Python toolchain to reduce pain in migrating for --incompatible_use_python_toolchains. However, there are two critical bugs in this change, which has already been cherrypicked into the 0.27 RC:

  1. The Python rules workspace suffix does not specify which order the toolchains should be registered in, so the non-strict one may take precedence over the strict one.

  2. The error message does not print the flag name --extra_toolchains=... correctly, defeating much of the benefit of the change.

@brandjon brandjon added type: bug P1 I'll work on this now. (Assignee required) release blocker team-Rules-Python Native rules for Python labels Jun 6, 2019
@brandjon brandjon self-assigned this Jun 6, 2019
laurentlb pushed a commit that referenced this issue Jun 7, 2019
1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name.

2. Fix bad quoting that caused the suggested command line flag to not be printed

3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages.

Fixes #8576, follow-up to #8547.

RELNOTES: None
PiperOrigin-RevId: 251937305
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name.

2. Fix bad quoting that caused the suggested command line flag to not be printed

3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages.

Fixes bazelbuild#8576, follow-up to bazelbuild#8547.

RELNOTES: None
PiperOrigin-RevId: 251937305
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name.

2. Fix bad quoting that caused the suggested command line flag to not be printed

3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages.

Fixes bazelbuild#8576, follow-up to bazelbuild#8547.

RELNOTES: None
PiperOrigin-RevId: 251937305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Python Native rules for Python type: bug
Projects
None yet
Development

No branches or pull requests

1 participant