-
-
Notifications
You must be signed in to change notification settings - Fork 17.5k
rustPlatform.cargoSetupHook: remove references to build directory in binaries #401281
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
Conversation
bd0632f to
6ed48c9
Compare
lilyball
left a 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.
I haven't looked at these hooks before but this certainly looks like it'll work. I suppose we can assume that $NIX_BUILD_TOP will never contain a path that has a " embedded in it?
6ed48c9 to
7d29171
Compare
7d29171 to
82a3626
Compare
|
I realized I also need to add this to |
|
Ideally, we'd wait for rust-lang/rust#140778 to be fixed before landing this, as without it, rustc source paths get embedded in the resulting binaries. |
|
Something interesting between the setup hook commit and the rustc commit: Where's that |
c26a286 to
075cb7a
Compare
We can reproduce the intended behavior with a useful use of `cat`. ;)
…binaries By default, Rust binaries include debug information (e.g. for the panic handler) that specifies source file paths. On sandboxed Linux, this isn't much of an issue since the build will always be in `/build`, but on any other platform (including sandboxed macOS), this will result in potential impurities (e.g. when using `--keep-failed` or building multiple derivations with the same name concurrently). Luckily, rustc provides an option to remap path prefixes when writing this debug info, in the form of `--remap-path-prefix`. This allows the replacement of any path prefix that would be written to the binary. This change sets this to `$NIX_BUILD_TOP`, falling back to a noop if there is no `$NIX_BUILD_TOP` during build (as cases where it's unset, like devshells, don't really benefit from purity).
See previous commit for more info -- we just need to set the same option here, as the rustc derivation doesn't use `cargoSetupHook`.
075cb7a to
9fb5358
Compare
| [ | ||
| ''"--remap-path-prefix"'' | ||
| ''"@BUILD_TOP@=/build"'' | ||
| ] |
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.
Could we put this in build.rustflags and avoid having to repeat it?
|
Seems a better solution will be in-flight eventually: https://matrix.to/#/!UNVBThoJtlIiVwiDjU:nixos.org/$pghG2JN87PUdBDTSG9Wda8Zs15w72PQZxuZk_0TxXXs?via=nixos.org&via=matrix.org&via=tchncs.de |
By default, Rust binaries include debug information (e.g. for the panic handler) that specifies source file paths. On sandboxed Linux, this isn't much of an issue since the build will always be in
/build, but on any other platform (including sandboxed macOS), this will result in potential impurities (e.g. when using--keep-failedor building multiple derivations with the same name concurrently).Luckily, rustc provides an option to remap path prefixes when writing this debug info, in the form of
--remap-path-prefix. This allows the replacement of any path prefix that would be written to the binary. This change sets this to$NIX_BUILD_TOP, falling back to a noop if there is no$NIX_BUILD_TOPduring build (as cases where it's unset, like devshells, don't really benefit from purity).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/)Add a 👍 reaction to pull requests you find important.