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
buildRustPackage now installs binaries to bin and libraries to lib #47556
Conversation
pkgs/build-support/rust/default.nix
Outdated
find target/release -maxdepth 1 -executable -type f -exec cp "{}" $out/bin \; | ||
mkdir -p $out/bin $out/lib | ||
find target/release -maxdepth 1 -type f -executable ! \( -regex ".*.\(so.[0-9]\|so\|a\|dylib\)" \) -print0 | xargs -r -0 cp -t $out/bin | ||
find target/release -maxdepth 1 -regex ".*.\(so.[0-9]\|so\|a\|dylib\)" -print0 | xargs -r -0 cp -t $out/lib |
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.
Not sure what the rust does but openssl has more complex sonames: libcrypto.so.1.0.0
Should be covered by
so.[0-9.]\+
I hope.
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.
Does find match the whole string with the regex or does it need a $
at the end?
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 changed it to [0-9.]+
since the escape char breaks it. According to man
find
matches on the whole path.
…integers at the end.
@GrahamcOfBorg build fd |
Please change this pull request to the staging branch (since it rebuilds quite a lot). |
Success on aarch64-linux (full log) Attempted: fd Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: fd Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: fd Partial log (click to expand)
|
Generally I agree that patches should go in staging. This change however is required for any Rust project which builds static libraries and changes a very small amount of code - it is a required bugfix for anyone writing Rust FFI code which depends on static libraries. IMO this qualifies for a change that should be expeditiously merged into master. If the final commit is deemed to not be appropriate (final) and requires extra future rebuilds then rebasing onto staging seems like a better idea but I don't think this is the case here. |
Well also it is a small change it still will rebuild many packages. We only try to merge code we are confident about to staging. It has nothing to do with the complexity of the change but the amount of rebuilds. |
This should also go to staging-18.09 |
Made a pull request on staging-18.09 here #47709 |
Your change was merged into staging |
Motivation for this change
buildRustPackage was not installing static libs and was installing .so to bin.
Fixes this issue: #46985
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)