fetchCargoTarball: Normalize the fetched files to make hashes more stable?

[primeos]

See #119640 (comment). This seems to have been caused by a Cargo change (#119640 (comment)) that caused some file permissions to change and as a result the hash of the tarball differs. One option to avoid such issues in the future would be to normalize the tarball (e.g. like fetchzip which extracts a tarball and produces therefore more stable hashes than fetchurl).

[symphorien]

There is already some kind of normalisation https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/cargo-vendor-normalise.py

[danieldk]

There is already some kind of normalisation https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/cargo-vendor-normalise.py

Indeed. I think the primary question is how we want to normalize permissions. Upstream cargo vendoring has the correct behavior again (retain the permissions set by upstream). But things could break again if another permission-modifying bug is introduced. So, that speaks in favor of normalizing permissions.

We cannot set all files to 0644 and directories to 0755, since some crates contain other material (e.g. scripts that are run by build.rs).

An idea: set all directory permissions to 0755 and *.{rs,toml} to 0644. For all other files, set permissions to 0755 if the file has the executable bit set for users or 0644 otherwise. Let's hope that Rust crates do not have files with the SUID bit set 😨 .

[symphorien]

I failed to find the exact cargo behavior change, but your idea seems sensible. Or even: 0644 if not executable and 0755 otherwise for everyone (more or less find . -execdir chmod a+rw '{}' \;)

[DavHau]

If the cargo-vendor package affects the FOD's hashes in buildRustPackage, then why doesn't buildRustPackage just use it's own static cargo-vendor package which is never to be updated.

If it really needs to be updated one day, than this update should be done carefully, including rechecking all outputs and possibly patching to ensure the outputs stay the same.
In case after an updated cargo-vendor, reproducibility cannot be ensured, then a new function buildRustPackage_2 could be introduced containing the new cargo-vendor version, so people's hashes don't get changed accidentally and updating to the new logic becomes an intentional process.

Despite I'm not really affected by it, I think this topic should be taken care of by all means. It seem to be a unpleasant experience for maintainers and I feel the whole rust hash mismatch topic has dragged FODs in general into a bad light community wide.

[danieldk]

If the cargo-vendor package affects the FOD's hashes in buildRustPackage, then why doesn't buildRustPackage just use it's own static cargo-vendor package which is never to be updated.

I agree that that would be a nice solution, but it may need to be updated some times. E.g. the lock file schema was updated relatively recently. Perhaps there are also additions over time that require newer cargo versions.

However, IMO the correct solution is to use proper fixed-output derivations, either through buildRustCrate + crate2nix or through the new importCargoLock function. Then we fully control vendoring.

[DavHau]

I agree that that would be a nice solution, but it may need to be updated some times. E.g. the lock file schema was updated relatively recently. Perhaps there are also additions over time that require newer cargo versions.

I think this would still be fine. If it has to be updated, it has to be update. We would end up having a bunch of different buildRustPackage_XX packages. Still I don't see a big issue with that.

However, IMO the correct solution is to use proper fixed-output derivations, either through buildRustCrate + crate2nix or through the new importCargoLock function. Then we fully control vendoring.

If it really is the case that importCargoLock / crate2nix is generally a superior approach, then I think buildRustPackage should be deprecated and removed. But as long as it exists, it should be taken care of, because many people depend on it.

[danieldk]

If it really is the case that importCargoLock / crate2nix is generally a superior approach, then I think buildRustPackage should be deprecated and removed. But as long as it exists, it should be taken care of, because many people depend on it.

buildRustPackage supports importCargoLock:

https://nixos.org/manual/nixpkgs/unstable/#importing-a-cargo.lock-file

However, since we can't use IFD in nixpkgs, we would have to add Cargo.lock files to nixpkgs. As far as I know there is no consensus on adding lock files.

[DavHau]

I don't see why adding lock files should be a problem. Other language frameworks dump massive package indices into nixpkgs which seems to be accepted as well. If there is a discussion going on about this, could you please share the URL? Thanks.

[danieldk]

I don't see why adding lock files should be a problem. Other language frameworks dump massive package indexes into nixpkgs which seems to be accepted as well. If there is there a discussion going on about this, could you please share the URL? Thanks.

I am not aware of a concrete issue or PR, I have just heard this on various occasions from project contributors who have been around longer than I have.

[symphorien]

If the cargo-vendor package affects the FOD's hashes in buildRustPackage, then why doesn't buildRustPackage just use it's own static cargo-vendor package which is never to be updated.

format changes do happen rust-lang/cargo#7579

Other language frameworks dump massive package indices into nixpkgs which seems to be accepted as well.

they are a massive pain as well: constant merge conflicts, and no one dares reviewing/accepting PRs regenerating nodpackages for example.
So it's a tradeoff, with no perfect solution.

[danieldk]

they are a massive pain as well: constant merge conflicts, and no one dares reviewing/accepting PRs regenerating nodpackages for example.
So it's a tradeoff, with no perfect solution.

But these wouldn’t be issues with per-derivation Cargo lock files.