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

[WIP] Change --extra_toolchains precedence to last-wins, instead of first… #20031

Closed
wants to merge 2 commits into from

Conversation

katre
Copy link
Member

@katre katre commented Nov 2, 2023

…-wins.

This only changes the precedence within all uses of --extra_toolchains, it does not change the precedence for register_toolchains calls.

After this, the priority order for toolchains is:

  1. Consider toolchains registered via --extra_toolchains
  2. Within this set, the last mentioned toolchain has highest priority
  3. Consider toolchains registered via register_toolchains
  4. Within this set, the first mentioned toolchain has highest priority

In all cases, pseudo-targets like :all and /... are ordered by Bazel's package loading mechanism, which is currently using a lexicographic ordering.

Fixes #19945.

Closes #19942.

PiperOrigin-RevId: 578850060
Change-Id: Ibd9c32e5479a6a27717734852b875f9b3f31b510

…-wins.

This only changes the precedence within all uses of `--extra_toolchains`, it does not change the precedence for `register_toolchains` calls.

After this, the priority order for toolchains is:
1. Consider toolchains registered via `--extra_toolchains`
  1. Within this set, the last mentioned toolchain has highest priority
2. Consider toolchains registered via `register_toolchains`
  1. Within this set, the first mentioned toolchain has highest priority

In all cases, pseudo-targets like `:all` and `/...` are ordered by Bazel's package loading mechanism, which is currently using a lexicographic ordering.

Fixes bazelbuild#19945.

Closes bazelbuild#19942.

PiperOrigin-RevId: 578850060
Change-Id: Ibd9c32e5479a6a27717734852b875f9b3f31b510
@katre
Copy link
Member Author

katre commented Nov 2, 2023

This is a slight change of #19942 by @layus.

Copy link
Contributor

@layus layus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation is always nice ! Thanks for pushing this forward

bazel \
build \
"//${pkg}/demo:use" &> $TEST_log || fail "Build failed"
expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from toolchain_2"'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two first tests here are not absolutely needed.

@copybara-service copybara-service bot closed this in 75f8017 Nov 6, 2023
@katre katre deleted the i19945-et-reverse branch November 6, 2023 16:07
keertk pushed a commit that referenced this pull request Nov 17, 2023
…-wins.

This only changes the precedence within all uses of `--extra_toolchains`, it does not change the precedence for `register_toolchains` calls.

After this, the priority order for toolchains is:
1. Consider toolchains registered via `--extra_toolchains`
  1. Within this set, the last mentioned toolchain has highest priority
2. Consider toolchains registered via `register_toolchains`
  1. Within this set, the first mentioned toolchain has highest priority

In all cases, pseudo-targets like `:all` and `/...` are ordered by Bazel's package loading mechanism, which is currently using a lexicographic ordering.

Fixes #19945.

Closes #19942. Closes #20031.

PiperOrigin-RevId: 579845262
Change-Id: Ibd9c32e5479a6a27717734852b875f9b3f31b510
keertk added a commit that referenced this pull request Nov 17, 2023
…of first-wins. (#20239)

This only changes the precedence within all uses of
`--extra_toolchains`, it does not change the precedence for
`register_toolchains` calls.

After this, the priority order for toolchains is:
1. Consider toolchains registered via `--extra_toolchains`
  1. Within this set, the last mentioned toolchain has highest priority
2. Consider toolchains registered via `register_toolchains`
  1. Within this set, the first mentioned toolchain has highest priority

In all cases, pseudo-targets like `:all` and `/...` are ordered by
Bazel's package loading mechanism, which is currently using a
lexicographic ordering.

Fixes #19945.

Closes #19942. Closes #20031.

PiperOrigin-RevId: 579845262
Change-Id: Ibd9c32e5479a6a27717734852b875f9b3f31b510

Co-authored-by: Guillaume Maudoux <[email protected]>
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.

Precedence of --extra_toolchains toolchains is wrong
2 participants