-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
gcc: fix c++ headers when same triplet cross compiling #240435
Conversation
37452bb
to
51ea2f3
Compare
51ea2f3
to
029949b
Compare
Updated the commit message to match the PR title. |
What build does this fix? |
Unfortunately it doesn't fix the entire build, it only fixes c++ compilation. There's more issues in the linking after the compilation completes successfully (still looking into those, they look like a repeat of #40797). |
I've been testing with |
Found the issue: #213453 |
Could you provide a test case? I do tons of builds where "build platform and host platform differ, but have the same triple" because I customize my compiler flags by modifying the |
029949b
to
cb98e8c
Compare
@amjoseph-nixpkgs
Both
in
diff --git a/lib/systems/examples.nix b/lib/systems/examples.nix
index 45b95c150fc..e8ed0d2d502 100644
--- a/lib/systems/examples.nix
+++ b/lib/systems/examples.nix
@@ -113,6 +113,10 @@ rec {
gnu64 = { config = "x86_64-unknown-linux-gnu"; };
gnu32 = { config = "i686-unknown-linux-gnu"; };
+ gnu64-with-xcode = gnu64 // {
+ xcodePlatform = "foo";
+ };
+
musl64 = { config = "x86_64-unknown-linux-musl"; };
musl32 = { config = "i686-unknown-linux-musl"; }; That's why I think it's the same root cause as #213453. |
Updated the PR description with a repro of a failure that this fixes. |
@amjoseph-nixpkgs I tested this PR with #238154. It fails later when trying to fix up libgcc:
|
ping |
@ofborg eval |
I believe it's related, but that doesn't fix it entirely, see #240435 (comment). Note that all these things are unrelated to this PR, this PR only addresses c++ compilation issues such as the ones hit when building |
Filed a specific issue for this in the meantime, since this PR's discussion is now a bit hard to follow: #243166. |
I'll rebase on current staging since I'm here and restart the eval. |
cb98e8c
to
05f767c
Compare
Tested on current master with #238154, but now it doesn't even build the stdenv so I can't verify if it fixes the c++ compilation issue, so I'll reopen in the meantime. |
I apologize for taking so long to review this. I'm rebuilding my environment with this PR; if all goes well I will merge it right after 23.11 branches off. |
Awesome! I saw your recent change to compare platforms instead of triples (#264976), that's related to the same "platforms being different but triples being the same issue" right? Maybe there's a better fix similar to that than what I've done here. |
Yes; that, plus this, mostly solves the problem. There are a bunch of small package-specific fixes too, like these: |
Also there is a test case for this now in It's commented out now, but once it passes I will uncomment it. So when this gets fixed it will stay fixed. Thank you again for your patience. |
So, I have good news and bad news. The good news is that everything but one package builds properly with this PR. The bad news is that package is Any ideas on what's going on here? The failure is posted below (for There's a four-year-old issue with the same symptoms, but the way it got resolved doesn't seem immediately applicable: Firefox does a bunch of wacky stuff with
|
This comment was marked as resolved.
This comment was marked as resolved.
Ooh, I bet the clang-specific junk in |
05f767c
to
f128c6a
Compare
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.
Good to go!
@ofborg eval
|
When build platform and host platform differ, but have the same triple, the code in nixpkgs will consider it a cross compilation, but gcc won't. This will lead some derivations to look for c++ headers in the wrong place. To solve this always output the headers in the non-cross location, like we do for the other gcc headers already.
The commit prior to this one, "gcc: fix c++ headers when same triplet cross compiling" causes gcc's c++ headers to be in the same outpath subdirectory regardless of whether the gcc build is a host==target or host!=target compiler. As a result of that change, the hack in cc-wrapper which adapted to the different paths is no longer needed. And, in fact, it must be removed, since if it is left in place builds such as pkgsCross.aarch64-multiplatform.firefox will fail as shown below. ``` firefox-unwrapped> 0:02.01(B checking the host C compiler works... yes(B(B firefox-unwrapped> 0:02.01(B checking for the host C++ compiler... /nix/store/1asqji9djmdlapzs70q7jw2j308ry7cn-clang-wrapper-16.0.6/bin/c++(B(B firefox-unwrapped> 0:02.14(B checking whether the host C++ compiler can be used... yes(B(B firefox-unwrapped> 0:02.14(B checking the host C++ compiler version... 16.0.6(B(B firefox-unwrapped> 0:02.21(B checking the host C++ compiler works... yes(B(B firefox-unwrapped> 0:02.40(B checking for target linker... lld(B(B firefox-unwrapped> 0:02.51(B checking for host linker... lld(B(B firefox-unwrapped> 0:02.60(B checking for 64-bit OS... yes(B(B firefox-unwrapped> 0:02.67(B checking for new enough STL headers from libstdc++...(B(B firefox-unwrapped> 0:02.67(B DEBUG: <truncated - see config.log for full output>(B(B firefox-unwrapped> 0:02.67(B DEBUG: | #if defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE)(B(B firefox-unwrapped> 0:02.67(B DEBUG: | # error libstdc++ not new enough(B(B firefox-unwrapped> 0:02.67(B DEBUG: | #endif(B(B firefox-unwrapped> 0:02.67(B DEBUG: | #if defined(_GLIBCXX_RELEASE)(B(B firefox-unwrapped> 0:02.67(B DEBUG: | # if _GLIBCXX_RELEASE < 8(B(B firefox-unwrapped> 0:02.67(B DEBUG: | # error libstdc++ not new enough(B(B firefox-unwrapped> 0:02.67(B DEBUG: | # else(B(B firefox-unwrapped> 0:02.67(B DEBUG: | (void) 0(B(B firefox-unwrapped> 0:02.67(B DEBUG: | # endif(B(B firefox-unwrapped> 0:02.67(B DEBUG: | #endif(B(B firefox-unwrapped> 0:02.67(B DEBUG: | ;(B(B firefox-unwrapped> 0:02.67(B DEBUG: | return 0;(B(B firefox-unwrapped> 0:02.67(B DEBUG: | }(B(B firefox-unwrapped> 0:02.67(B DEBUG: Executing: `/nix/store/7v4bi4q334yircaznwm353h1l5i7k98f-aarch64-unknown-linux-gnu-clang-wrapper-16.0.6/bin/aarch64-unknown-linux-gnu-clang++ /build/conftest.qvbo58dv.cpp -c`(B(B firefox-unwrapped> 0:02.67(B DEBUG: The command returned non-zero exit status 1.(B(B firefox-unwrapped> 0:02.67(B DEBUG: Its error output was:(B(B firefox-unwrapped> 0:02.67(B DEBUG: | /build/conftest.qvbo58dv.cpp:1:10: fatal error: 'cstddef' file not found(B(B firefox-unwrapped> 0:02.67(B DEBUG: | #include <cstddef>(B(B firefox-unwrapped> 0:02.67(B DEBUG: | ^~~~~~~~~(B(B firefox-unwrapped> 0:02.67(B DEBUG: | 1 error generated.(B(B firefox-unwrapped> 0:02.67(B ERROR: The libstdc++ in use is not new enough. Please run ./mach bootstrap to update your compiler, or update your system libstdc++ installation.(B(B firefox-unwrapped> *** Fix above errors and then restart with "./mach build" error: build of '/nix/store/5in7xkji5hzqkl14ygwq3vxnni54lykk-firefox-unwrapped-aarch64-unknown-linux-gnu-119.0.1.drv' on 'ssh://[email protected]' failed: builder for '/nix/store/5in7xkji5hzqkl14ygwq3vxnni54lykk-firefox-unwrapped-aarch64-unknown-linux-gnu-119.0.1.drv' failed with exit code 1 error: builder for '/nix/store/5in7xkji5hzqkl14ygwq3vxnni54lykk-firefox-unwrapped-aarch64-unknown-linux-gnu-119.0.1.drv' failed with exit code 1; ```
Rebased to force ofborg to re-eval. |
f128c6a
to
981b640
Compare
Alright, I will merge this as soon as |
We're in december, can this be merged yet? |
Sorry about the delay! |
Description of changes
When build platform and host platform differ, but have the same triple, the code in nixpkgs will consider it a cross compilation, but gcc won't. This will lead some derivations to look for c++ headers in the wrong place. To solve this always output the headers in the non-cross location, like we do for the other gcc headers already.
Before:
After:
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Closes #243166