go_local_sdk: more strict SDK platform detection #2765
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We determine the "SDK platform" by looking at the goos_goarch subdirectory of pkg/tool. If the goroot has actually built the toolchain for multiple platforms, then we just pick the first one and potentially fail cryptically later. Ideally, we would simply register toolchains for all of the host platforms we find, but that is likely a much larger change. Until then, provide a much clearer error message that we don't like finding multiple platforms. Fixes bazelbuild#2764
jayconrod
approved these changes
Dec 21, 2020
jayconrod
pushed a commit
that referenced
this pull request
Dec 23, 2020
We determine the "SDK platform" by looking at the goos_goarch subdirectory of pkg/tool. If the goroot has actually built the toolchain for multiple platforms, then we just pick the first one and potentially fail cryptically later. Ideally, we would simply register toolchains for all of the host platforms we find, but that is likely a much larger change. Until then, provide a much clearer error message that we don't like finding multiple platforms. Fixes #2764
jayconrod
pushed a commit
that referenced
this pull request
Dec 23, 2020
We determine the "SDK platform" by looking at the goos_goarch subdirectory of pkg/tool. If the goroot has actually built the toolchain for multiple platforms, then we just pick the first one and potentially fail cryptically later. Ideally, we would simply register toolchains for all of the host platforms we find, but that is likely a much larger change. Until then, provide a much clearer error message that we don't like finding multiple platforms. Fixes #2764
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We determine the "SDK platform" by looking at the goos_goarch
subdirectory of pkg/tool. If the goroot has actually built the toolchain
for multiple platforms, then we just pick the first one and potentially
fail cryptically later.
Ideally, we would simply register toolchains for all of the host
platforms we find, but that is likely a much larger change. Until then,
provide a much clearer error message that we don't like finding multiple
platforms.
Fixes #2764
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
When pointing a
go_local_sdkto a toolchain with multiple platforms this improves the error from a cryptic "no matching toolchains found for types @io_bazel_rules_go//go:toolchain" to "Error in fail: Could not detect SDK platform: found multiple platforms ["darwin_amd64", "linux_amd64"] in /usr/local/google/home/mpratt/src/go/pkg/tool".This will make it more clear that the user should delete the extra platforms. Though ideally they could either specify the platform in
go_local_sdkor we could register all of the platforms.Which issues(s) does this PR fix?
Fixes #2764
Other notes for review