-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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.libgcc: compare host and target platforms, rather than their triples #264976
Conversation
@ofborg build pkgsCross.gnu64_simplekernel.bash |
pkgs/shells/bash/5.nix
Outdated
@@ -91,7 +91,7 @@ stdenv.mkDerivation rec { | |||
|
|||
strictDeps = true; | |||
# Note: Bison is needed because the patches above modify parse.y. | |||
depsBuildBuild = [ buildPackages.stdenv.cc ]; | |||
depsBuildBuild = [ stdenv ]; |
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.
Why this change? I've never seen depending on entire other stdenvs like that.
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.
If we just skip this, everything else looks good to me.
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.
Why this change?
Because !(buildPackages.stdenv.cc?__spliced)
.
See #264992 for the test case.
The reproducer requires two platforms which have the same triple but aren't ==
-equal to each other. People are creating them on their own and getting burned by this. Latest example:
There aren't any in lib.systems.examples
, so #264992 adds one in order to be able to answer your question "why this change".
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.
Why this change?
Because: #264992 (comment)
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.
If we just skip this, everything else looks good to me.
I moved it into
It looks like this should fix #263040, except you missed a comparison, |
We have several cross-compilation bugs that show up if hostPlatform!=buildPlatform yet hostPlatform.config==buildPlatform.config. These bugs have appeared and disappeared as we've fiddled with the definition of equality for platform objects. This commit adds a clear-cut case where they are *not* equal and never will be, so we can test it.
…ples The rest of our gcc expression prepends "${targetPlatform.config}-" to paths and binaries if `hostPlatform!=targetPlatform`. The `libgcc.nix` expression was using 'hostPlatform.config!=targetPlatform.config`, which caused it to look in the wrong place when moving files. This commit corrects that.
I have verified that your test case (deflakified below) builds correctly at a37a7d2
|
I rebuilt the entire This won't break anything. If it does I take full responsibility. |
Description of changes
The rest of our gcc expression prepends
"${targetPlatform.config}-"
to paths and binaries ifhostPlatform!=targetPlatform
. Thelibgcc.nix
expression was usinghostPlatform.config!=targetPlatform.config
, which caused it to look in the wrong place when moving files. This commit corrects that.Closes #263040
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)