-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Prevent using the default cc
when that'd result in a broken build
#111351
Conversation
This comment has been minimized.
This comment has been minimized.
FYI I am not planning to review this currently since the CI failure seems to show it's not really workable. Unfortunately I don't have suggestions for improving it ... maybe thomcc has suggestions since he maintains cc, but tbh I think the real problem here is that cc generates broken code instead of giving an error. |
@jyn514 I think the CI problem above will need to be fixed regardless of whether we implement the error in bootstrap (like this PR does) or in The error is that we're trying to compile 32bit mingw on that builder without the appropriate compiler. I'll fix it in the coming days, but I still think the PR can be reviewed (apart from the CI changes needed to inject the correct compilers). |
I think CI is still failing (not sure if that was intended to be fixed?). My sense is that this will make it harder to do "trivial" cross-testing for cases where you don't actually need a C compiler, because you're not planning on using it. We have an escape hatch (or did?) in the form of an env-variable override ("BOOTSTRAP_SKIP_TARGET_SANITY"), added in #103569... but forcing people to use something like that doesn't feel great. I think at minimum we should move this new check into the same code that's trying to confirm that we have C/C++ compilers in sanity.rs. That would probably fix CI without any other changes needed, and be a good enough starting point that I feel OK merging this. But if we see issues I'd be inclined to back this out or move it to a warning rather than hard error. |
☔ The latest upstream changes (presumably #112418) made this pull request unmergeable. Please resolve the merge conflicts. |
c9864d0
to
efbbd0c
Compare
Rewrote the patch to take in account Mark's feedback and the discussion on Zulip. To only show the error message when a C compiler is actually used, there is now a Before merging: I tested this on Linux, and it should also work for macOS. We need to test whether this behaves correctly on Windows though, and I don't have a Windows machine at hand. Right now the condition has |
r? @onur-ozkan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging: I tested this on Linux, and it should also work for macOS. We need to test whether this behaves correctly on Windows though, and I don't have a Windows machine at hand. Right now the condition has !compiler.is_like_clang(), as clang has a single binary for all targets, but I don't know how the linker behaves on Windows and whether we should change that condition to compiler.is_like_gnu().
I don't have Windows environment to check this.
iirc you do have Windows environment, if so, can you check this ? @albertlarsan68
r=me with the Windows compatibility check
Checked out this branch locally and can confirm that running
works on recent Windows on a 64bit 12 x AMD Ryzen 5 PRO 4650 and with the appropriate Visual Studio Developer tools installed. |
@bors r+ rollup=iffy |
…r-ozkan Prevent using the default `cc` when that'd result in a broken build This PR adds a check in bootstrap to prevent issues like rust-lang#111142 to happen. What happened there is, no compiler for `aarch64-unknown-none` was detected by bootstrap, and so the `cc` crate defaulted to the system host C compiler. That resulted in a broken target, as `libcompiler_builtins.rlib` (or well anything compiling C code in its build script) contained half the object files compiled for aarch64 and half of them compiled for x86_64. The check added in the PR ensures that, for cross-compilation, the detected C compiler is not the same one as the host compiler, unless the detected compiler is clang (as it supports all targets from a single binary). This should not cause too many false positives, as for example a `./x build library --target i686-unknown-linux-gnu` still works from a x86_64 host. If a false positive occurs, the error message still explains the workaround. Example error message: ``` running: "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" cargo:warning= cargo:warning= Error: the automatic detection of the C compiler for cross-compiled cargo:warning= target aarch64-unknown-none returned the compiler also used for the cargo:warning= current host platform. cargo:warning= cargo:warning= This is likely wrong, and will likely result in a broken compilation cargo:warning= artifact. Please specify the correct compiler for that target, either cargo:warning= with environment variables: cargo:warning= cargo:warning= CC_aarch64_unknown_none=path/to/cc cargo:warning= CXX_aarch64_unknown_none=path/to/cxx cargo:warning= cargo:warning= ...or in config.toml: cargo:warning= cargo:warning= [target."aarch64-unknown-none"] cargo:warning= cc = "path/to/cc" cargo:warning= cxx = "path/to/cxx" cargo:warning= cargo:warning= The detected C compiler was: cargo:warning= cargo:warning= cc cargo:warning= exit status: 1 --- stderr error occurred: Command "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" with args "broken-cc" did not execute successfully (status code exit status: 1). Build completed unsuccessfully in 0:00:02 ``` Fixes rust-lang#111142 r? `@jyn514`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Seems like I gated things to run only on nvptx, rather than everywhere but nvptx 😅 @bors r=onur-ozkan rollup=never |
…r-ozkan Prevent using the default `cc` when that'd result in a broken build This PR adds a check in bootstrap to prevent issues like rust-lang#111142 to happen. What happened there is, no compiler for `aarch64-unknown-none` was detected by bootstrap, and so the `cc` crate defaulted to the system host C compiler. That resulted in a broken target, as `libcompiler_builtins.rlib` (or well anything compiling C code in its build script) contained half the object files compiled for aarch64 and half of them compiled for x86_64. The check added in the PR ensures that, for cross-compilation, the detected C compiler is not the same one as the host compiler, unless the detected compiler is clang (as it supports all targets from a single binary). This should not cause too many false positives, as for example a `./x build library --target i686-unknown-linux-gnu` still works from a x86_64 host. If a false positive occurs, the error message still explains the workaround. Example error message: ``` running: "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" cargo:warning= cargo:warning= Error: the automatic detection of the C compiler for cross-compiled cargo:warning= target aarch64-unknown-none returned the compiler also used for the cargo:warning= current host platform. cargo:warning= cargo:warning= This is likely wrong, and will likely result in a broken compilation cargo:warning= artifact. Please specify the correct compiler for that target, either cargo:warning= with environment variables: cargo:warning= cargo:warning= CC_aarch64_unknown_none=path/to/cc cargo:warning= CXX_aarch64_unknown_none=path/to/cxx cargo:warning= cargo:warning= ...or in config.toml: cargo:warning= cargo:warning= [target."aarch64-unknown-none"] cargo:warning= cc = "path/to/cc" cargo:warning= cxx = "path/to/cxx" cargo:warning= cargo:warning= The detected C compiler was: cargo:warning= cargo:warning= cc cargo:warning= exit status: 1 --- stderr error occurred: Command "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" with args "broken-cc" did not execute successfully (status code exit status: 1). Build completed unsuccessfully in 0:00:02 ``` Fixes rust-lang#111142 r? `@jyn514`
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
…r-ozkan Prevent using the default `cc` when that'd result in a broken build This PR adds a check in bootstrap to prevent issues like rust-lang#111142 to happen. What happened there is, no compiler for `aarch64-unknown-none` was detected by bootstrap, and so the `cc` crate defaulted to the system host C compiler. That resulted in a broken target, as `libcompiler_builtins.rlib` (or well anything compiling C code in its build script) contained half the object files compiled for aarch64 and half of them compiled for x86_64. The check added in the PR ensures that, for cross-compilation, the detected C compiler is not the same one as the host compiler, unless the detected compiler is clang (as it supports all targets from a single binary). This should not cause too many false positives, as for example a `./x build library --target i686-unknown-linux-gnu` still works from a x86_64 host. If a false positive occurs, the error message still explains the workaround. Example error message: ``` running: "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" cargo:warning= cargo:warning= Error: the automatic detection of the C compiler for cross-compiled cargo:warning= target aarch64-unknown-none returned the compiler also used for the cargo:warning= current host platform. cargo:warning= cargo:warning= This is likely wrong, and will likely result in a broken compilation cargo:warning= artifact. Please specify the correct compiler for that target, either cargo:warning= with environment variables: cargo:warning= cargo:warning= CC_aarch64_unknown_none=path/to/cc cargo:warning= CXX_aarch64_unknown_none=path/to/cxx cargo:warning= cargo:warning= ...or in config.toml: cargo:warning= cargo:warning= [target."aarch64-unknown-none"] cargo:warning= cc = "path/to/cc" cargo:warning= cxx = "path/to/cxx" cargo:warning= cargo:warning= The detected C compiler was: cargo:warning= cargo:warning= cc cargo:warning= exit status: 1 --- stderr error occurred: Command "/home/pietro/r/github/rust-lang/rust/2/build/bootstrap/debug/broken-cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-ffunction-sections" "-fdata-sections" "-fPIC" "--broken-cc-target=aarch64-unknown-none" "--broken-cc-detected=cc" "-I" "/home/pietro/r/github/rust-lang/rust/2/src/llvm-project/compiler-rt/lib/builtins" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/ea072bc2688e9ac2-lse_cas1_relax.o" "-c" "/home/pietro/r/github/rust-lang/rust/2/build/x86_64-unknown-linux-gnu/stage0-std/aarch64-unknown-none/release/build/compiler_builtins-13da5a491a5cb882/out/lse_cas1_relax.S" with args "broken-cc" did not execute successfully (status code exit status: 1). Build completed unsuccessfully in 0:00:02 ``` Fixes rust-lang#111142 r? `@jyn514`
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
(I had to synchronize the queue) |
@pietroalbini any updates on this? thanks |
@pietroalbini Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github. @rustbot label: +S-inactive |
This PR adds a check in bootstrap to prevent issues like #111142 to happen. What happened there is, no compiler for
aarch64-unknown-none
was detected by bootstrap, and so thecc
crate defaulted to the system host C compiler. That resulted in a broken target, aslibcompiler_builtins.rlib
(or well anything compiling C code in its build script) contained half the object files compiled for aarch64 and half of them compiled for x86_64.The check added in the PR ensures that, for cross-compilation, the detected C compiler is not the same one as the host compiler, unless the detected compiler is clang (as it supports all targets from a single binary). This should not cause too many false positives, as for example a
./x build library --target i686-unknown-linux-gnu
still works from a x86_64 host. If a false positive occurs, the error message still explains the workaround.Example error message:
Fixes #111142
r? @jyn514