-
-
Notifications
You must be signed in to change notification settings - Fork 666
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
Bazel 0.26.0 fixed the autoconfigured C++ toolchains and now break cross-compilation of Go #2089
Comments
I didn't have time to think about it too much. I guess the simple workaround for the meantime is to revert dd527c7 (I didn't try it yet but it should help). The separate rule approach is worth thinking about. What will you select on? Ideally you want to take C++ toolchain if its present... @lberki had a nice evil idea - have an aspect traverse go dependencies, and if it finds C++ target, it steals its C++ toolchain from it. There's a lot of handwaving there :)) |
Another not great alternative is to register Null-object-like C++ toolchains that can only fail the build when actually used. |
Would reverting dd527c7 break cgo compilation in general? Is that required in Bazel 0.26.0, or still optional? About selecting on a separate rule, I think this will be easier once SBC is fully enabled. We'll probably have a control whether cgo is enabled, and we could select on that. Another idea: we could define a separate set of |
The "null toolchain" would only help if you could make it so that it's selected iff nothing else is. I thought that was not possible. |
Reverting dd527c7 will make go cross compilation work if there are no C++ dependencies. It will not make cgo with C++ dependencies work, cgo would only cross compile Go code, not the C++ code. Having a cgo option is a solution. This way users have to specify that they intend to use cgo and go rules will only then depend on C++ toolchain (and complain when cross compiling that the matching C++ toolchain is not present). I think SBC is non-experimental in 0.27, but we should double check. The last approach you propose is to have distinct sets of platforms for go and cgo, right? That will work as well, it's similar to the previous solution, except that we select on constraint_value, not on configuration option. Is that reasonable thing to ask from Go users to be explicit about cgo usage? Or they expect this to work automatically? |
I've kicked around the idea of having optional toolchains, in addition to
required toolchains. If we had that, then go could declare that the cpp
toolchain is optional, and if trying to build cgo code without a cpp
toolchain, produce its own error.
The code paths wouldn't be particularly tricky to add (but would require
starlark changes in the rule declaration API, and lots of tests). Does this
sound like a reasonable way forward?
…On Wed, Jun 5, 2019 at 4:58 AM Marcel Hlopko ***@***.***> wrote:
Reverting dd527c7
<dd527c7>
will make go cross compilation work if there are no C++ dependencies. It
will not make cgo with C++ dependencies work, cgo would only cross compile
Go code, not the C++ code.
Having a cgo option is a solution. This way users have to specify that
they intend to use cgo and go rules will only then depend on C++ toolchain
(and complain when cross compiling that the matching C++ toolchain is not
present). I think XBC is non-experimental in 0.27, but we should double
check.
The last approach you propose is to have distinct sets of platforms for go
and cgo, right? That will work as well, it's similar to the previous
solution, except that we select on constraint_value, not on configuration
option.
Is that reasonable thing to ask from Go users to be explicit about cgo
usage? Or they expect this to work automatically?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2089>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACPW75OSDHWRPPF3C4PELTPY553VANCNFSM4HS5I7PQ>
.
|
Shouting from sidelines: I could imagine conditional toolchains (i.e. "I need this toolchain in some cases, but not in others") but optional (as in "it's useful if I have it, but I can deal with it it's not available") is weird. Let's try to keep the user-facing API simple because the API is something everyone who uses toolchains needs to understand and they don't benefit from something added only for Go. If Go is the only place where a new API feature would be useful, I'd prefer the hackish "fish the C++ toolchain out from transitive dependencies" approach than extending the API. That way, it's only Go who pays the price. |
I am trying to remember who, besides Go, asked for this feature. The actual
feature was requested by Ian for rules_go (
bazelbuild/bazel#3601) about two years ago.
It hasn't come up much since, but other toolchains have asked about this
before.
…On Wed, Jun 5, 2019 at 10:24 AM lberki ***@***.***> wrote:
Shouting from sidelines: I could imagine conditional toolchains (i.e. "I
need this toolchain in some cases, but not in others") but optional (as in
"it's useful if I have it, but I can deal with it it's not available") is
weird.
Let's try to keep the user-facing API simple because the API is something
everyone who uses toolchains needs to understand and they don't necessarily
benefit from something added only for Go. If Go is the only place where a
new API feature would be useful, I'd prefer the hackish "fish the C++
toolchain out from transitive dependencies" approach than extending the
API. That way, it's only Go who pays the price.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2089>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACPW72BFZWIBPLR6A6CZQTPY7ECDANCNFSM4HS5I7PQ>
.
|
I think it's reasonable. With the Go command, cgo is enabled by default if the target platform is the same as the host platform and disabled otherwise. Users can explicitly turn it on by setting So here's what I'm thinking now:
|
If |
This should be fixed in #2090.
cc @steeve @hlopko Any update on the Unfortunately, I can't think of a way to get cgo to work on Windows by default without requiring users to configure their own toolchains or |
I have problems with cross-compiling ( bazel: 0.26.1
Is there a working workaround, if I do not need support cgo? |
@asv We shouldn't need the C compiler to run protoc. I'll look into that. We're using its path to set a value for
|
The C compiler was used to set PATH. No idea why that was useful though. Fixes bazel-contrib#2089
I have the same problem as #2089 (comment) when I'm using a starlark transition to set the correct platform based on the current cpu (where go is a dependency of a ios_framework or android_binary that's multiarch). My transition: _ANDROID_CPUS_TO_PLATFORMS = {
"x86": "@io_bazel_rules_go//go/toolchain:android_386_cgo",
"x86_64": "@io_bazel_rules_go//go/toolchain:android_amd64_cgo",
"armeabi-v7a": "@io_bazel_rules_go//go/toolchain:android_arm_cgo",
"arm64-v8a": "@io_bazel_rules_go//go/toolchain:android_arm64_cgo",
}
_IOS_CPUS_TO_PLATFORMS = {
"ios_armv7": "@io_bazel_rules_go//go/toolchain:ios_arm_cgo",
"ios_arm64": "@io_bazel_rules_go//go/toolchain:ios_arm64_cgo",
"ios_i386": "@io_bazel_rules_go//go/toolchain:ios_386_cgo",
"ios_x86_64": "@io_bazel_rules_go//go/toolchain:ios_amd64_cgo",
}
def _go_platform_transition_impl(settings, attr):
cpu = settings["//command_line_option:cpu"]
platform = ""
if settings["//command_line_option:crosstool_top"].workspace_name == "androidndk":
platform = _ANDROID_CPUS_TO_PLATFORMS[cpu]
elif cpu in _IOS_CPUS_TO_PLATFORMS:
platform = _IOS_CPUS_TO_PLATFORMS[cpu]
return {
"//command_line_option:platforms": platform,
}
go_platform_transition = transition(
implementation = _go_platform_transition_impl,
inputs = [
"//command_line_option:cpu",
"//command_line_option:crosstool_top",
],
outputs = [
"//command_line_option:platforms",
],
) On android the error is somewhat different:
|
If I enable legacy objc/cgo mode:
|
#2101 will remove the However, I'm not able to build anything proto-related on Windows without MSVC, including |
@hlopko I could get away with manually registering them, though: [
toolchain(
name = "android_%s" % cpu,
target_compatible_with = [
"@bazel_tools//platforms:android",
"@bazel_tools//platforms:%s" % cpu,
],
toolchain = "@androidndk//:%s-linux-android-clang7.0.2-libcpp" % cpu,
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
)
for cpu in ["x86", "x86_64", "arm", "aarch64"]
] [
register_toolchains("//:android_%s" % cpu)
for cpu in ["x86_64", "arm", "aarch64"]
] Is there anything more needed? I can compile |
Registering ndk toolchains is one of the problems. Other is having the fat apk produced correctly. |
@hlopko the problem though is that it seems if we put a |
I believe this causes and issue for people using We're upgrading rules_docker to 0.12.0 which now depends on
We don't need |
Just to recap where we are with this issue:
I've spent several weeks analyzing this issue, and I've basically timeboxed it at this point. It should be fixed eventually but probably won't be any time soon. I'd recommend folks work around this by installing a C/C++ toolchain or configuring a dummy |
@jayconrod We have bazel installation doc: https://docs.bazel.build/versions/master/install-ubuntu.html#step-1-install-required-packages but it fails with the same error.
Something else that might be helpful is with:
I can post the full log output, it's just that it's a couple of a hundred lines. |
@Toxicable Do you have When I built a container earlier today following those instructions, something pulled in |
I removed In CI
But still failing on this error:
Note this part here:
Whys it looking for arm toolchain configs? |
@Toxicable rules_go doesn't load that file directly. Something in the Bazel toolchain auto-configuration is probably loading it. Is this in a freshly started container, or was I don't know enough about your environment to help more than that. Feel free to open a new issue with enough information to reproduce the problem, and I can take a look. |
thanks for the additional info @Toxicable , I think the error might be related to some recent toolchain changes we made in rules_docker. |
It's alright, thanks for taking the time to debug this. Both locally and in CI I get:
when trying to use your PR I get this error locally:
Note that nodejs_base is not the default one from ruels_docker, it's one we defiend at |
thanks @Toxicable , it looks like that PR solves the toolchain issue, let's open up a separate issue in rules_docker to talk about the issue you are having pulling that image. |
Hi, |
Hi, @jayconrod @hlopko The platform functions of
|
We also face a similar problem (like @yicm) in a mixed Java project:
I can create a separate issue along with the repro repository if needed. Edit: problem occurs only under Mac OS. On linux we have no problem with cross compilation. |
Hi, @asv My error message is the same as yours. I think there is a need to create a new issue. I have not succeeded under the Linux platform. Maybe we can show this problem better through a bazel demo project. |
Closing this issue because it's been fixed for a long time. At the time, rules_go had an unconditional dependency on the C/C++ toolchain in configurations where it wasn't needed and wasn't available. @asv @yicm Could you open new issues and fill out the issue template? I don't think the issues you're commenting on are related to the problems you're seeing, but I don't have enough information to help more than that. |
It was fixed over a year ago according to bazel-contrib/rules_go#2089 I've had some users confused by this message since we tell them to use this rule on Mac
It was fixed over a year ago according to bazel-contrib/rules_go#2089 I've had some users confused by this message since we tell them to use this rule on Mac
Hello :)
With the fix for bazelbuild/bazel#8330 we discovered one flaw in our design of Go cross compilation. Currently rules_go depend on C++ toolchain in //go/private:context.bzl (
toolchains = ["@bazel_tools//tools/cpp:toolchain_type"]
). This dependency exists no matter if we actually have C++ dependencies or not.The problem is that with bazelbuild/bazel#8330 C++ autoconfigured toolchains properly report their constraints, and now Bazel detects that we don't actually have a C++ cross-compilation toolchain available. As a result the cross-compilation build of go fails:
To reproduce, create a hello world go_binary, and build it with
bazel build //:hello_world --platforms=@io_bazel_rules_go//go/toolchain:darwin_amd64
on linux, (or if you're on mac, use a different platform).The text was updated successfully, but these errors were encountered: