-
-
Notifications
You must be signed in to change notification settings - Fork 668
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
Cgo and --compiler=gcc #1357
Comments
Cgo determines the types of symbols by attempting to compile a specially crafted C file, then checking whether errors occur in specific places. It's possible it's expecting the compiler to behave a certain way, but a different compiler is actually being used? Could you tell me a bit more about your environment? OS, compiler version, Bazel version? What is the command that produces the top error message? Does anything different happen if the environment variable |
OS: debian jessie Here's the exact subcommand being run: The environment variable BAZEL_OUTPUT_ROOT is set to the execution root of the sandbox. CC is currently pointing to a shell script that converts the full paths to relative paths. Within that shell script if I change the compiler to point to /usr/bin/gcc instead, I get a similar set of errors. If I don't try to set a full path for all of the -isystem options, then the install stops at an earlier stage where it cannot find the basic header files like stddef.h |
Actually if I remove the environment variables COMPILER_PATH and CGO_CPP_FLAGS, everything now succeeds. This is true is I set CC to either /usr/bin/gcc, or the sandboxed compiler we have in external/amd64_compilers. Maybe it's one of the flags that is throwing things off? |
Thanks for the info. I'll try and reproduce this, but I probably won't be able to get to it until tomorrow or Friday. Something to try before then: what happens if you remove the Do you see the same error using a newer version of rules_go? We rewrote the stdlib builder where the error is happening, I think after 0.9.0. Might fix this. Also, just to confirm: are you using the Bazel auto-configuration (detects compiler in PATH or CC) or do you have a custom CROSSTOOL file? |
Removing the -fdiagnostics-color flag does not solve the problem. I just tried upgrading to a 0.10.1. It's now failing out at an earlier stage because it cannot find the compiler we placed in external/amd64_compilers_repo/usr/bin/x86_64-linux-gnu-gcc. We are using a custom CROSSTOOL file. |
Ok, let's proceed with 0.10.1 if possible. I'm only vaguely familiar with custom CROSSTOOL files. I'll help as much as I can, but I'm coming up to speed on this. :) Could you paste the error you're getting about the compiler being missing? I'm wondering if this is a Bazel error (some dependency missing) or an execution error (bad path on the command line or something). If you're able to get to the part where it builds the stdlib, could you paste the command line? I'm interested in both the command line for stdlib builder (Bazel will print this with the
|
I suspect it's a bad path. Here's the error I'm currently blocked on:
Do you know where the commands are being executed? |
Forgot to paste in the subcommand: SUBCOMMAND: # @io_bazel_rules_go//:stdlib [action 'GoStdlib external/io_bazel_rules_go/linux_amd64/stdlib~/pkg'] |
The commands will be executed in the execroot. Bazel prints it before each command. In this case,
I don't see anything odd, other than the relative paths for the includes. They might not be in the sandbox. Does this work if you build with The way I usually debug is to disable sandboxing, then execute commands manually in the execroot until I find something that works. Maybe try removing some of the flags and see what affects the build? |
I'm going to put together a reproduction repo to send you. I'm collecting up a fully hermetic compiler that I can share. I'm hoping doing that will make it easier to reproduce issues. I'm assuming if I were to get you a repo for Linux, that would be helpful. |
@jayconrod , https://github.com/AustinSchuh/golang_demo is a working demo. It's setup for Debian Jessie (Debian 8) from our robotics repo (4 target architectures). If you edit tools/bazel.rc and remove the crosstool_top flags, everything works. If you add them back, you get cgo errors. I included a trivial C++ application as an example to show that the CROSSTOOL file works. @nathantchan @bsilver8192 and I have some modifications we've had to make to get rules_go working with a similar crosstool setup. It would be nice to figure out how to get this setup to work out of the box to make future upgrades less painful. Once this works, we can figure out how to get a repo which has both GCC and Clang in it to show you that as well. As a side note, that should be a fully reproducible and hermetic compiler. I did some basic md5sum tests on that repo. |
@AustinSchuh Thanks for posting that! I was able to reproduce the failure.
The script The next error I ran into was a missing |
@jayconrod Wohoo! Progress. The challenge here is that if we convert anything to absolute paths in the clang/gcc invocation, they get baked into all our binaries. If we have a generated shell script with an absolute path to the compiler, that won't work with remote-ex. Bazel propper works really hard to not expose absolute paths to the action. The only way to be reproducible is to do relative paths. The toolchain lives at a relative path (or a path findable with |
@mhlopko Any tips for writing a CROSSTOOL for a compiler in the workspace that avoids relative paths while remaining reproducible? We're looking at this CROSSTOOL, specifically. One idea (maybe not the best idea): if you have a wrapper script around the compiler, you could find the location of the wrapper script itself and then use that to transform paths relative to the workspace root. |
I agree with Austin, we try hard not to produce absolute paths. Compilers are funny and when they see absolute paths they use them everywhere in output artifacts (be it binaries, libraries, or small things like .d files) and spoil the remote caches with host specific entries. There are things that some compilers stubbornly use absolute paths for (e.g. debug info), but we try. Without knowing much about go use case, I'd expect that you will also want to get rid of all absolute paths. Is it an option? |
@mhlopko The Go compiler is a little more well-behaved w.r.t. absolute paths. It accepts some options like In this case though, Does it make sense to wrap the C compiler with a script that transforms relative paths? Is there a better solution that has worked elsewhere? |
Hmm tough. Then yes, you could use wrapper scripts, just note that they are quite costly on windows (TF build faster by ~30% when not using them). But ideally you would use the same crosstool in go that the rest of bazel world uses, and that can mean relative paths... Any cool idea how to fix the go install side? |
We have a wrapper Go program that we invoke My concern is that we will either miss some relative paths that we should fix (different compilers have different ways of saying What causes wrapper scripts to be slow on Windows? Are regular executables much faster, or is it the extra exec that's slow? |
@meteorcloudy knows the details of why wrapper scripts were slow on Windows, I always assumed it's the exec that's costly. How do you get C compiler and flags currently? From CppConfiguration? |
IIRC, wrapper scripts are slow because creating new processes are expensive on Windows. @laszlocsomor can confirm that or correct me. :) |
Yes, but not as slow as I used to tell everyone. Mea culpa. I measured very slow process creation times (hundreds of milliseconds to several seconds for a single CreateProcess call) because -- I believe -- Bit9/Carbon Black was scanning, with each CreateProcess, the binaries I tried to run. I measured process creation speed on our Windows VMs that we use in the Bazel CI, and process creation there is a lot faster, something like 5 to 20ms. |
...which is still orders of magnitudes slower than it is on Linux, but it is what it is. |
@mhlopko We currently get the C compiler and flags from the @laszlocsomor 5-20ms sounds tolerable, though we should still seek to minimize process creation. I think anyone running Bazel (or any build tool for that matter) with a heavy antivirus in the background is going to have a long coffee break though. |
@jayconrod A wrapper script is going to be required. It's a matter of what it will look at this point from my experience. Also, you will very likely find with this crosstool that there are flags hidden today that you can't get access to through the My rather ugly wrapper script which made this all work in an older version of rules_go (it's broken today...)
ie, pass in an environment variable signaling where the root of the repo is, and do all the calculation to convert the flags to ones relative to the current directory from the repo root. It does a lot of path mangling to things like Another thought would be to generate the wrapper script (instead of passing the args in that way) so the wrapper script can have all the flags in it. I'm pondering if the wrapper script could start with a (The patch above is then paired with something like the following to mangle the input.)
|
Hopefully that will be sorted out with the ongoing work to expose more C++ stuff in the Skylark API.
I'd be happy to have this be an integration test. It will be a little tricky to set up, but I think it would fit into our integration test framework. It would also be useful to point to this as an example. There are people who want custom CROSSTOOLs for cross-compilation, especially. |
@jayconrod : I ran the test again to get up-to-date numbers. See commit message of laszlocsomor/projects@bb6a2d9 . |
Re hidden flags: Yes, that should be fixed by bazelbuild/bazel#4571. |
@laszlocsomor Wow. I know antivirus really slows things down, but I'm always surprised by how much. @ianthehat mentioned a while back that it took him ~5 minutes to build a "hello world" Go program on Windows (nearly all of that time is spent building the standard library). I guess that's why. |
I've been reviewing #1365. As part of that PR, @steeve added some code to all the builders that absolutized include paths. He was able to build with a custom CROSSTOOL that used relative paths after that without needed wrapper scripts. That solved the problem you are seeing. See env.go:83. However, this caused reproducibility problems, and it caused tests to fail on Linux when sandboxing was enabled. cgo embeds the contents of This makes it more difficult to build stdlib with a custom CROSSTOOL, but maybe not impossible. I think absolute paths in |
The main issue is that |
If Ideally, we could build the standard library ourselves with normal compile / assemble / link actions. Converting that is a lot of work though, and I don't think it's feasible right now. |
@jayconrod @steeve , I think reproducibility is actually the easier part of the problem to solve. The trick is to use a compiler wrapper shell script to convert the paths. Pass in a "root of repo" magic string, and convert that to a relative path in the wrapper script. And/or convert any absolute paths to relative paths in that script. We did that in our main compiler wrapper script previously, but that could be a secondary wrapper script for just golang (which would be cleaner).
That lets you compute a relative path from the current directory to the root of the repo. Then, you can use bash to replace anything absolute (or your magic token) with that relative path. The following bash can do that. You'll need a couple of similar bash lines to convert the args as the following
|
I did some investigation real quick. Doing two consecutive runs in a Docker container and comparing the hashes gave me a list of affected packages, I think those are CGo packages.
|
Looking at 8,9c8,9
< 00000070 62 75 69 6c 64 20 69 64 20 22 49 69 78 62 6e 54 |build id "IixbnT|
< 00000080 46 35 61 53 30 4e 61 59 50 45 49 53 78 73 2f 73 |F5aS0NaYPEISxs/s|
---
> 00000070 62 75 69 6c 64 20 69 64 20 22 58 59 7a 63 77 6e |build id "XYzcwn|
> 00000080 63 4e 4f 4b 4e 59 7a 35 57 53 44 70 43 44 2f 73 |cNOKNYz5WSDpCD/s|
12,13c12,13
< 000000b0 20 69 64 20 22 49 69 78 62 6e 54 46 35 61 53 30 | id "IixbnTF5aS0|
< 000000c0 4e 61 59 50 45 49 53 78 73 2f 73 71 38 38 4e 51 |NaYPEISxs/sq88NQ|
---
> 000000b0 20 69 64 20 22 58 59 7a 63 77 6e 63 4e 4f 4b 4e | id "XYzcwncNOKN|
> 000000c0 59 7a 35 57 53 44 70 43 44 2f 73 71 38 38 4e 51 |Yz5WSDpCD/sq88NQ|
37,38c37,38
< 00000240 49 69 78 62 6e 54 46 35 61 53 30 4e 61 59 50 45 |IixbnTF5aS0NaYPE|
< 00000250 49 53 78 73 2f 73 71 38 38 4e 51 53 47 58 6a 6b |ISxs/sq88NQSGXjk|
---
> 00000240 58 59 7a 63 77 6e 63 4e 4f 4b 4e 59 7a 35 57 53 |XYzcwncNOKNYz5WS|
> 00000250 44 70 43 44 2f 73 71 38 38 4e 51 53 47 58 6a 6b |DpCD/sq88NQSGXjk| It seems this is because of build id. I think the key is in go tool compile's -buildid flag maybe ? |
I've filed #1391 to track -buildid. I don't know how it's determined yet; needs investigation. Are you seeing any differences due to absolute paths? Any difference near a "sandbox" string would be interesting. |
@AustinSchuh Do you need any changes in the stdlib builder to make that work? Ideally, rules_go would provide these wrappers, since they would be useful to anyone with a custom CROSSTOOL that contains relative paths. I don't think I'll have bandwidth to implement that soon though. |
@jayconrod , Here are the changes we've made to support that.
You need a way to find the root of the repo, and need to convert relative paths in flags to absolute paths on the way into the rules. This is a big chunk of our solution, but it feels brittle. (I think I've already gotten some of the GOROOT changes already upstreamed. We'll do another pass at that once the dust settles.) |
@jayconrod I have not seen absolute paths in the generated objects, only the build id changes, and I think this is a bug we have already on regardless of this ticket |
@jayconrod , did you guys get anywhere? |
@AustinSchuh, We got the buildid out of the standard library (thanks @steeve !), but we still have the general problem with relative paths. I think the proper way to solve this is a proper builder for the standard library instead of copying files and using Bazel folks are making progress on a |
In the mean time, perhaps @AustinSchuh can use my absolute cgo commit ? |
Just so you know @jayconrod, we're using that commit for the Android support, it's working well for us, save for unforeseen side effects of course 😃 |
I'll open a PR for it so that we have a place to discuss it maybe? |
@jayconrod, this is the ticket I was talking about today at the Bazel Conference. #1357 (comment) in particular has the hermetic CROSSTOOLs. If you run into any issues with the CROSSTOOLs, I'm happy to update them to enable testing. @mhlopko , FYI. |
(Re-triaging old issues) Closing as obsolete. I think all of the stdlib buildid and relative path issues have been resolved. Bazel's C/C++ toolchain support and rules_go's integration with it have been rewritten since this was filed, so hopefully there's nothing more to do here. |
I can confirm this is fixed. I'm able to use custom CROSSTOOL files with golang now. |
When using a custom cpp toolchain that adds libc++.a and libc++abi.a in link_libs, linking the stdlib fails because it can't find the archives by their relative paths. Same as issue bazel-contrib#1357
Make the gopackagesdriver work when using a C/C++ toolchain with sysroots. We need to turn the relative in flag like --sysroot into an absolute. Address the todo by using the code already available in the stdlib.go as part of bazel-contrib#1536.
Make the gopackagesdriver work when using a C/C++ toolchain with sysroots. We need to turn the relative in flag like --sysroot into an absolute. Address the todo by using the code already available in the stdlib.go as part of #1536.
Copied from bazel-go-discuss. cc @nathantchan
One of our verification builds uses gcc, and I currently cannot get past the installation portion of the stdlib rule. It's throwing errors like the following:
It doesn't seem like setting --compiler=gcc works right now in 0.9.0 of the go rules. Any ideas?
The text was updated successfully, but these errors were encountered: