Skip to content
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

buildRustCrate: pass link flags when building libraries #177436

Merged
merged 2 commits into from
Jun 15, 2022

Conversation

lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Jun 12, 2022

Description of changes

With Rust 1.61, it is necessary to link to external static/dynamic libaries when building the rlib that uses them, rather than when linking the final binary. In fact, it is no longer necessary to specify the libraries to link (-l) when building the final binary, but the library search path flags (-L) must still be included.

I figured this out by comparing buildRustCrate's rustc arguments with those used by cargo, and haven't identified the upstream commit that changed this behavior.

I added a test that builds the rcgen package, which triggers this issue because it depends on ring. It has quite a few dependencies, so if someone knows of a simpler test case, I'd be happy to switch. This test fails to cross-compile due to an existing issue described here: #138321 (comment)

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

cc @Ericson2314 @happysalada

With Rust 1.61, it is necessary to link to external static/dynamic libaries
when building the rlib that uses them, rather than when linking the final
binary. In fact, it is no longer necessary to specify the libraries to link
when building the final binary, but the library search path flags must still
be included.
rcgen depends on ring, and therefore exercises support for static libraries
@happysalada
Copy link
Contributor

This looks good to me. The CI issue at the moment is likely unrelated.

@marsam
Copy link
Contributor

marsam commented Jun 12, 2022

@GrahamcOfBorg eval

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Jun 12, 2022
@happysalada
Copy link
Contributor

Thank you, I think we are good to go!

@happysalada happysalada merged commit 882741f into NixOS:master Jun 15, 2022
@lopsided98 lopsided98 deleted the build-rust-crate-static-lib branch June 15, 2022 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: rust 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants