-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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
rustc: add a compiler wrapper #267876
rustc: add a compiler wrapper #267876
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
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.
Hooray! It's about time we did this.
I pushed a patch (hope that's okay) to this PR to fix CI. Running builds now; I'll approve this PR as soon as they finish. Two stylistic suggestions included below; neither one is critical but I think the second one will save us headaches later on.
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.
Built (8257 derivations):
-
x86_64-linux
workstation+server package sets -
powerpc64le-linux
workstation+server package sets -
aarch64-linux
(cross fromx86_64-linux
) workstation package set -
mips64el-linux32
(cross fromx86_64-linux
) router package set -
tests.cross.sanity
Thank you for doing this! |
Should be unchanged — unless separateDebugInfo is enabled, this should result in rustc being run with exactly the same flags as before, fastCross or not. |
This comment was marked as off-topic.
This comment was marked as off-topic.
We keep running into situations where we can't get the right combination of rustc flags through build systems into rustc. RUSTFLAGS is the only variable supported across build systems, but if RUSTFLAGS is set, Cargo will ignore all other ways of specifying rustc flags, including the target-specific ones, which we need to make dynamic musl builds work. (This is why pkgsCross.musl64.crosvm is currently broken — it works if you unset separateDebugInfo, which causes RUSTFLAGS not to be set.) So, we need to do the same thing we do for C and C++ compilers, and add a compiler wrapper so we can inject the flags we need, regardless of the build system. Currently the wrapper only supports a single mechanism for injecting flags — the NIX_RUSTFLAGS environment variable. As time goes on, we'll probably want to add additional features, like target-specific environment variables.
This avoids having two layers of wrapper for cross rustc.
Setting RUSTFLAGS causes Cargo to ignore other ways of configuring flags, including the target-specific RUSTFLAGS options. This broke pkgsCross.musl64.crosvm, and was surprising to users. Fixes: NixOS#261727
b51d1c9
to
47c1fa2
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
pkgsCross.musl64.rustfmt is similarly broken on current master for me — how's it relevant here? |
Sorry, I did not realize that. I built |
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.
I have some more ideas, but for now this improves the situation. Thanks!
Description of changes
We keep running into situations where we can't get the right combination of rustc flags through build systems into rustc.
RUSTFLAGS
is the only variable supported across build systems, but ifRUSTFLAGS
is set, Cargo will ignore all other ways of specifying rustc flags, including the target-specific ones, which we need to make dynamic musl builds work. (This is whypkgsCross.musl64.crosvm
is currently broken — it works if you unsetseparateDebugInfo
, which causesRUSTFLAGS
not to be set.)So, we need to do the same thing we do for C and C++ compilers, and add a compiler wrapper so we can inject the flags we need, regardless of the build system.
Currently the wrapper only supports a single mechanism for injecting flags — the
NIX_RUSTFLAGS
environment variable. As time goes on, we'll probably want to add additional features, like target-specific environment variables.This fixes #261727, and
pkgsCross.musl64.crosvm
.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/
)