rustPlatform.buildRustPackage: support direct use of Cargo.lock#122158
rustPlatform.buildRustPackage: support direct use of Cargo.lock#122158danieldk merged 3 commits intoNixOS:masterfrom
Conversation
andir
left a comment
There was a problem hiding this comment.
This is the right direction. Less abuse of FODs for fetching a random set of dependencies.
If we merge this in we need at least one test that checks that the feature isn't broken. Any small packaging test might do as long as it makes use of outputHash and whatever else we might end up with.
|
I also agree with this approach but would let the review up to @andir as he has way more experience of subtile implementation details of Rust's dependency resolution. |
8e09468 to
f027400
Compare
Added three tests:
I used To avoid that a user has to specify the hashes for all the dependencies from a workspace crate, I changed outputHash = {
"rand-0.8.3" = "sha256-CnMJWpSnU6slmCJhbxaXq0ikxVZDk4SQwmFYNlSEQnk=";
};
outputHash = {
"f0e01ee0a7257753cc51b291f62666f4765923ef" = "sha256-CnMJWpSnU6slmCJhbxaXq0ikxVZDk4SQwmFYNlSEQnk=";
}; |
andir
left a comment
There was a problem hiding this comment.
In general it looks nice. I haven't tested it yet and thus there is no +1. I left a few minor remarks.
f3540bb to
d6e6ccb
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/why-dont-nix-hashes-use-base-16/11325/27 |
Did you find the time to test this? |
I just tried to convert a random package (in a hacky read from derivation way) from nixpkgs to this. I picked but it fails like this: Should it be this way? |
Looks good. The issue is in workspaces where crates are not top-level directories. I have it building now, but I need to clean up the changes. |
d6e6ccb to
0ecb452
Compare
@andir this should work now. For crates from workspaces |
0ecb452 to
8f94729
Compare
|
@alyssais any objections against merging this? |
|
This feature would be a significant step forward in packaging usability. |
|
I would hate for this to get stuck, so unless there are objections, I'll merge this at Friday 28, 12:00 CET. |
asymmetric
left a comment
There was a problem hiding this comment.
This would be great to have. Left a couple of (mostly stylistic) comments.
ebae8d0 to
308a869
Compare
This function can be used to create an output path that is a cargo
vendor directory. In contrast to e.g. fetchCargoTarball all the
dependent crates are fetched using fixed-output derivations. The
hashes for the fixed-output derivations are gathered from the
Cargo.lock file.
Usage is very simple, e.g.:
importCargoLock {
lockFile = ./Cargo.lock;
}
would use the lockfile from the current directory.
The implementation of this function is based on Eelco Dolstra's
import-cargo:
https://github.com/edolstra/import-cargo/blob/master/flake.nix
Compared to upstream:
- We use fetchgit in place of builtins.fetchGit.
- Sync to current cargo vendoring.
This change introduces the cargoLock argument to buildRustPackage,
which can be used in place of cargo{Sha256,Hash} or cargoVendorDir. It
uses the importCargoLock function to build the vendor
directory. Differences compared to cargo{Sha256,Hash}:
- Requires a Cargo.lock file.
- Does not require a Cargo hash.
- Retrieves all dependencies as fixed-output derivations.
This makes buildRustPackage much easier to use as part of a Rust
project, since it does not require updating cargo{Sha256,Hash} for
every change to the lock file.
308a869 to
d3769e4
Compare
|
@ofborg build tests.importCargoLock |
|
Thanks for all the reviews! |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Support for directly passing the Cargo.lock was added in <NixOS/nixpkgs#122158>.
|
A pattern I find myself using when some upstream project doesn't have a # ...
pkgs.rustPlatform.buildRustPackage {
# ...
cargoLock.lockFile = ./Cargo.lock;
cargoPatches = [
(pkgs.runCommand "Cargo.lock.patch" { buildInputs = [ pkgs.git ]; } ''
cp ${./Cargo.lock} Cargo.lock
git init && git add Cargo.lock
git diff --cached > $out
'')
];
}That way I don't need to manually provide the Cargo.lock patch. Maybe this patch generation could be automated in rustPlatform.buildRustPackage when cargoLock.lockFile is specified? If I omit the cargoPatches entry, I get an error message during the build: |
|
Probably it shouldn't be even apply a patch but just copy the whole thing over? |
Motivation for this change
This change introduces the
cargoLockargument tobuildRustPackage,which can be used in place of
cargo{Sha256,Hash}orcargoVendorDir. Ituses the
importCargoLockfunction to build the vendordirectory. Differences compared to
cargo{Sha256,Hash}:Cargo.lockfile.This makes
buildRustPackagemuch easier to use as part of a Rustproject, since it does not require updating
cargo{Sha256,Hash}forevery change to the lock file.
To introduce this functionality, the
importCargoLockfunction wasalso added. This function can be used to create an output path that is
a cargo vendor directory. In contrast to e.g.
fetchCargoTarballallthe dependent crates are fetched using fixed-output derivations. The
hashes for the fixed-output derivations are gathered from the
Cargo.lockfile.Usage is very simple, e.g.:
would use the lockfile from the current directory.
The implementation of this function is based on Eelco Dolstra's
import-cargo:
https://github.com/edolstra/import-cargo/blob/master/flake.nix
Compared to
cargo-import:fetchgitin place ofbuiltins.fetchGit. This requires specifying output hashes, which can be done through theoutputHashattribute (see the documentation). Since crates.io does not allow uploads with non-crates.io dependencies, git dependencies do not occur much in practive.cc @Ma27, @edolstra
Related issue: #89563
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)