Conversation
alyssais
requested review from
Ericson2314,
Mic92,
figsoda,
winterqt and
zowoq
as code owners
github-actions
bot
added
8.has: documentation
This comment was marked as duplicate.
This comment was marked as duplicate.
There was a problem hiding this comment.
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.
ofborg
bot
added
10.rebuild-linux-stdenv
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Built (8257 derivations):
-
x86_64-linuxworkstation+server package sets -
powerpc64le-linuxworkstation+server package sets -
aarch64-linux(cross fromx86_64-linux) workstation package set -
mips64el-linux32(cross fromx86_64-linux) router package set -
tests.cross.sanity
ghost
mentioned this pull request
delroth
added
the
12.approvals: 1
|
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
alyssais
force-pushed
the
rustc-wrapper
branch
from
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 |
yu-re-ka
left a comment
yu-re-ka
left a comment
There was a problem hiding this comment.
I have some more ideas, but for now this improves the situation. Thanks!
ghost
merged commit 4 participants
Description of changes
We keep running into situations where we can't get the right combination of rustc flags through build systems into rustc.
RUSTFLAGSis the only variable supported across build systems, but ifRUSTFLAGSis 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.crosvmis currently broken — it works if you unsetseparateDebugInfo, which causesRUSTFLAGSnot 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_RUSTFLAGSenvironment 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 = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)