-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
fetchCargoTarball: Normalize the fetched files to make hashes more stable? #121259
Comments
There is already some kind of normalisation https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/cargo-vendor-normalise.py |
Seems like this was affected by vendoring permission normalization, see NixOS#121259.
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 An idea: set all directory permissions to |
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 |
If the cargo-vendor package affects the FOD's hashes in 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. 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. |
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 However, IMO the correct solution is to use proper fixed-output derivations, either through |
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
If it really is the case that |
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 |
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. |
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. |
format changes do happen rust-lang/cargo#7579
they are a massive pain as well: constant merge conflicts, and no one dares reviewing/accepting PRs regenerating nodpackages for example. |
But these wouldn’t be issues with per-derivation Cargo lock files. |
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 thanfetchurl
).The text was updated successfully, but these errors were encountered: