rustPlatform.fetchCargoTarball: drop by emilazy · Pull Request #394012 · NixOS/nixpkgs

emilazy · 2025-03-28T13:58:01Z

As discussed in the Nix Rust room on Matrix.

All existing users of fetchCargoTarball will have invalid FOD hashes upon the release of 25.05 due to changes in Cargo 1.84. We switched to fetchCargoVendor across the tree to fix this in Nixpkgs, but downstream users could still get reuse of cached but invalid FOD hashes. Since this is a difficult‐to‐detect and painful failure mode, and we want to ensure users move to the more reproducible fetchCargoVendor, let’s remove fetchCargoTarball entirely and set useFetchCargoVendor by default.

This isn’t any more of a breaking change than updating Cargo locked us into already, but it will ensure that it breaks loudly (as fetchCargoVendor uses a different derivation name for the FOD, preventing incorrect reuse), and that users migrate to the stabler, future‐proofed option. importCargoLock with a vendored Cargo.lock file is still available as a fallback if people run into things that fetchCargoVendor can’t yet handle.

Things done

Add a 👍 reaction to pull requests you find important.

pkgs/build-support/rust/build-rust-package/default.nix

TomaSajt · 2025-03-29T17:14:19Z

Edit: this comment has a mistake where it assumes the hashes also get invalidated for fetchCargoTarball. In reality, they would not be invalidated, though an eval-warning would still be present.

In this message I will try to summarize the two possible ways to move forward.
If I got something wrong, please correct me.

Let's suppose we don't merge this PR:

Then we will have a cycle when not doing a useFetchCargoVendor = true will lead to an eval-time warning.
For nixpkgs this makes a PR forgetting to add it ineligible for a merge (at least as far as the CI is concerned). So essentially, people are forced to add it, so it's essentially just cruft.

For out of tree this means people will get an eval warning and a FOD hash mismatch error.
They either switch over to fetchCargoVendor and regen the hash, as instructed in the warning, OR they ignore the warning, (they need to regen the hash anyways), but will get the warning every time the package is evaled. The user will eventually set useFetchCargoVendor = true to get rid of the warning, unless they require a feature of fetchCargoTarball that neither importCargoLock or fetchCargoVendor implement. If they are missing a feature, they'll ideally open an issue.

This option is safer.

The deprecation process might look something like this:

25.05 - warn on useFetchCargoVendor = true not being added.
25.11 - make useFetchCargoVendor always warn that it is no longer necessary
26.05 - no more warnings

Let's suppose we merge this PR:

Assuming we merge before the 25.05 branchoff, we will have to keep useFetchCargoVendor = true in our packages until 24.11 goes out of support so that we can backport the changes.
After that we will have a nice nixpkgs cycle where no extra cruft is needed.

For out of tree this means that people will get a FOD hash mismatch error if they weren't using useFetchCargoVendor = true before. Cache invalidation is not a problem: the FODs associated with the old hash will not be reused if they don't manually invalidate the hash because the name of the FODs is different between fetchCargoTarball and fetchCargoVendor.
This means out of tree users will be forced over to fetchCargoVendor.
If we're worried about people not knowing the reason behind the hash mismatch, we may insert an output message at the end of all fetchCargoVendor FODs mentioning something about "if you get a hash mismatch here after moving to 25.05, it is because we migrated to a new fetcher".

This option is a more radical choice. After (hopefully) short term problems, the rest of the cycle will be able to move forward without cruft.

The deprecation process might look something like this:

25.05 - error on useFetchCargoVendor = false being explicitly set (it's true by default if unset)
25.11 - warn on useFetchCargoVendor still being set`
26.05 - no more warnings

TomaSajt · 2025-03-29T17:16:56Z

After writing this comment, I am leaning more towards the safer option: the only con is that it will have the required cruft.
Otherwise, the total deprecation period seems to be the same length (AFAICT)

I can be persuaded if someone has some more pros for the other option

emilazy · 2025-03-29T18:45:55Z

For out of tree this means people will get an eval warning and a FOD hash mismatch error.

Not unless I misunderstand the situation: if they have a hash from Rust 1.83 in their store, it will silently be used. That’s why the current state is dangerous and we need to do this: silently invalidating FOD hashes without changing names does not work. We will be giving a dangerous reproducibility problem to basically every out‐of‐tree Rust package on the 25.05 upgrade. That’s not a better compatibility story than dropping the footgun. I don’t think the warning is enough, since if they follow it then it’s the same as a hard error, and if they don’t then things are just as bad.

In general we should just really not ever let a FOD with the same name change hash. It’s the worst kind of error because detecting it depends on the state of your environment; that’s why we did the mass migration in the first place. A guaranteed error like this PR causes is much better.

TomaSajt · 2025-03-29T19:27:19Z

I see, true...
We might as well just change the name of fetchCargoTarball's output :) problem solved?
(Do note that the hash actually depends on name for fetchCargoTarball because $name is used to create the tarball. Unfortunate, because if we change the name the hash will change. But the hash is expected to change anyways. (Except if someone already generated a hash with a new version, in that case they'll get a new hash))

emilazy · 2025-03-29T19:49:41Z

Why keep it at all? If we’re going to deliberately break every user and make people update the hash, then we should just remove it and drop the complexity rather than do anything to encourage further use of it. importCargoLock is there as a fallback.

TomaSajt · 2025-03-29T19:57:10Z

Okay true, if we'd be breaking their hash anyways then we can break it hard

I'm a bit worried about people missing features from the current impl. Or people not expecting this double-layered derivation system we've set up.

But yeah, I see why the "safe" option is not safe.

emilazy · 2025-03-29T20:03:14Z

I agree it’s a breaking change, it’s just that anything we do other than shipping Rust 1.84 as the default is going to be a breaking change. Since we want people on fetchCargoVendor anyway, it makes sense to break it like this. Hopefully importCargoLock will be sufficient fallback for as‐yet‐unimplemented features?

TomaSajt · 2025-03-29T20:04:02Z

Could you edit together something to put as the PR description if you have the time?

Also, will the deprecation process be as I described?

emilazy · 2025-03-29T20:06:40Z

I think that deprecating useFetchCargoVendor = true; entirely is kinda separate, right? But yes, the timeline sounds about right to me, since by 25.11’s release, 24.11 will be unsupported, so just removing the setting will be a valid way to fix the warning on all supported branches.

Is there anything you’d like to see in the PR message that isn’t in the release notes, or can I just copy those in?

TomaSajt · 2025-03-29T20:15:49Z

I'd probably also put something about: why now? and what next? But whatever you think is fine for someone looks back at this PR.

emilazy · 2025-03-30T16:28:35Z

I’ve updated the PR message; let me know what you think.

TomaSajt · 2025-03-30T17:22:24Z

doc/release-notes/rl-2505.section.md

-  It should generally be replaced with `rustPlatform.fetchCargoVendor`, but `rustPlatform.importCargoLock` may also be appropriate in some circumstances.
-  `rustPlatform.buildRustPackage` users must set `useFetchCargoVendor` to `true` and regenerate the `cargoHash`.
+- Rust packages will need to regenerate their `cargoHash`.
+  Cargo 1.84.0 changed the format of `cargo vendor` output, which invalidated all existing `rustPlatform.fetchCargoTarball` hashes.


Let's clear up the terminology here:

To me, when something "invalidates" a hash it doesn't mean that it causes a hash mismatch, it just means that it will try to re-check the hash, thus re-fetching the files. Unrelated whether or not there's a hash mismatch or not.

Here, I assume you meant something like "broke all existing hashes" (not sure what else to call it other than "break"), meaning those FODs will cause a hash mismatch if they are re-fetched. However they are very much not invalidated, in the sense that they will still be fetched from the cache (because the name didn't change)

In a literal sense, the hashes were previously valid and are no longer. I agree that there could potentially be confusion with the specific sense of cache invalidation (which is an operation you perform when the underlying result changes, making the cache entry no longer valid), but I’m hoping the context here makes it clear?

I don’t object to “broke” here if you have a strong opinion, though.

There is a helper called invalidateFetcherByDrvHash that makes the name value of the FOD always dependent on the derivation's hash, thus always "invalidating" it (making it always get re-fetched).
However I couldn't find much in favor of my definition being the correct usage.

I see that in #376038 "invalidate" also meant "break".

So... I don't know. Let's keep it as "invalidate".

Though, if the "invalidate" naming is already taken what would you call the "needs-to-be-refetched-to-make-sure-its-actually-correct" state of a FOD?

I guess it’s refreshing a cache entry, because you think it might be in need of cache invalidation, because perhaps the recorded hash it’s keyed on was invalidated :)

TomaSajt · 2025-03-30T17:25:06Z

I’ve updated the PR message; let me know what you think.

I think it's fine now, thanks :)
I also added some Related: links to some relevant earlier PRs.

With the recent deprecation of `rustPlatform.fetchCargoTarball` + migration to using `fetchCargoVendor` by default in `buildRustPackage` (NixOS/nixpkgs#394012), the `cargo-bundle` override strategy used here, as prescribed by the [nixos asia wiki](https://nixos.asia/en/buildRustPackage) no longer works: https://github.com/zed-industries/zed/blob/c6e2d20a02b523172aea8a22fa2ec6c8975b52e4/nix/build.nix#L100-L116 [`fetchCargoTarball` produced a single derivation][tarball-drv] but `fetchCargoVendor` [produces two][vendor-drvs]: - `${name}-vendor-staging` (inner; FoD) - `${name}-vendor` (outer) [tarball-drv]: https://github.com/NixOS/nixpkgs/blob/36fd87baa9083f34f7f5027900b62ee6d09b1f2f/pkgs/build-support/rust/fetch-cargo-tarball/default.nix#L79 [vendor-drvs]: https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/fetch-cargo-vendor.nix#L52-L103 `overrideAttrs` here is setting `outputHash` on the latter (which isn't a fixed-output-derivation and does not have `outputHashMode` set which implies `outputHashMode = "flat"`) instead of the inner; this results in errors like this: ```console ❯ nix develop error: output path '/nix/store/cb57w05zvsqxshqjl789kmsy9pbqjn06-cargo-bundle-0.6.1-zed-vendor.tar.gz' should be a non-executable regular file since recursive hashing is not enabled (outputHashMode=flat) error: 1 dependencies of derivation '/nix/store/k3azmxljgjn26hqyhg9m1y3lhx32y939-cargo-bundle-0.6.1-zed.drv' failed to build error: 1 dependencies of derivation '/nix/store/8ag4v0m90m4kcaq1ypp7f85pp8s6fxgc-nix-shell-env.drv' failed to build ``` > [!NOTE] > you will need to remove > `/nix/store/cb57w05zvsqxshqjl789kmsy9pbqjn06-cargo-bundle-0.6.1-zed-vendor.tar.gz` > from your nix store in order to be able to reproduce this We want to be setting `outputHash` on the [first derivation][first-drv] instead. This change has us just do the call to `fetchCargoTarball` manually instead of using overrides. [first-drv]: https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/fetch-cargo-vendor.nix#L85 --- I suspect CI/other machines didn't catch this due to a store path matching the name + `outputHash` already being present but I'm not entirely sure how this happened... `sha256-Q49FnXNHWhvbH1LtMUpXFcvGKu9VHwqOXXd+MjswO64=` is actually a `fetchCargoTarball` hash, not a `fetchCargoVendor` hash (and upstream `cargo-about`'s `cargoDeps` [has been using `cargoVendor`][ups] since before the nixpkgs bump in 50ad71a) [ups]: https://github.com/NixOS/nixpkgs/blob/1d09c579c12869453d98dd35f6ff9d28dc32e869/pkgs/by-name/ca/cargo-about/package.nix#L22 Note: this commit leaves the hash as is + uses `fetchCargoTarball` to demonstrate that we're not making any modifications to the cargo-about dependencies. The next commit will switch to using `fetchCargoVendor`. --- > [!NOTE] > eventually we'll be able to just have > `.overrideAttrs (_: { cargoHash = "..."; })` > work as expected [^2] [^2]: [now that `buildRustPackage`](NixOS/nixpkgs#382550) uses [`lib.extendMkDerivation`](https://github.com/nixos/nixpkgs/blob/bbdf8601bcf2a7e733d5ef2552109a5d8d5a44ce/doc/build-helpers/fixed-point-arguments.chapter.md) (NixOS/nixpkgs/#234651) the groundwork is in place; a follow PR [needs to use `cargoHash` and friends from `finalAttrs`](https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/build-rust-package/default.nix#L104)

see the previous commit; `fetchCargoTarball` is deprecated in nixpkgs 25.05: NixOS/nixpkgs#394012

With the recent deprecation of `rustPlatform.fetchCargoTarball` + migration to using `fetchCargoVendor` by default in `buildRustPackage` (NixOS/nixpkgs#394012), the `cargo-bundle` override strategy used here, as prescribed by the [nixos asia wiki](https://nixos.asia/en/buildRustPackage) no longer works: https://github.com/zed-industries/zed/blob/c6e2d20a02b523172aea8a22fa2ec6c8975b52e4/nix/build.nix#L100-L116 [`fetchCargoTarball` produced a single derivation][tarball-drv] but `fetchCargoVendor` [produces two][vendor-drvs]: - `${name}-vendor-staging` (inner; FoD) - `${name}-vendor` (outer) [tarball-drv]: https://github.com/NixOS/nixpkgs/blob/36fd87baa9083f34f7f5027900b62ee6d09b1f2f/pkgs/build-support/rust/fetch-cargo-tarball/default.nix#L79 [vendor-drvs]: https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/fetch-cargo-vendor.nix#L52-L103 `overrideAttrs` here is setting `outputHash` on the latter (which isn't a fixed-output-derivation and does not have `outputHashMode` set which implies `outputHashMode = "flat"`) instead of the inner; this results in errors like this: ```console ❯ nix develop error: output path '/nix/store/cb57w05zvsqxshqjl789kmsy9pbqjn06-cargo-bundle-0.6.1-zed-vendor.tar.gz' should be a non-executable regular file since recursive hashing is not enabled (outputHashMode=flat) error: 1 dependencies of derivation '/nix/store/k3azmxljgjn26hqyhg9m1y3lhx32y939-cargo-bundle-0.6.1-zed.drv' failed to build error: 1 dependencies of derivation '/nix/store/8ag4v0m90m4kcaq1ypp7f85pp8s6fxgc-nix-shell-env.drv' failed to build ``` > [!NOTE] > you will need to remove `/nix/store/cb57w05zvsqxshqjl789kmsy9pbqjn06-cargo-bundle-0.6.1-zed-vendor.tar.gz` > from your nix store in order to be able to reproduce this We want to be setting `outputHash` on the [first derivation][first-drv] instead. This change has us just do the call to `fetchCargoTarball` manually instead of using overrides. [first-drv]: https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/fetch-cargo-vendor.nix#L85 --- I suspect CI/other machines didn't catch this due to a store path matching the name + `outputHash` already being present but I'm not entirely sure how this happened... `sha256-Q49FnXNHWhvbH1LtMUpXFcvGKu9VHwqOXXd+MjswO64=` is actually a `fetchCargoTarball` hash, not a `fetchCargoVendor` hash (and upstream `cargo-about`'s `cargoDeps` [has been using `cargoVendor`][ups] since before the nixpkgs bump in 50ad71a) [ups]: https://github.com/NixOS/nixpkgs/blob/1d09c579c12869453d98dd35f6ff9d28dc32e869/pkgs/by-name/ca/cargo-about/package.nix#L22 --- > [!NOTE] > eventually we'll be able to just have `.overrideAttrs (_: { cargoHash = "..."; })` work as expected [^2] --- Release Notes: - N/A [^2]: [now that `buildRustPackage`](NixOS/nixpkgs#382550) uses [`lib.extendMkDerivation`](https://github.com/nixos/nixpkgs/blob/bbdf8601bcf2a7e733d5ef2552109a5d8d5a44ce/doc/build-helpers/fixed-point-arguments.chapter.md) (NixOS/nixpkgs/#234651) the groundwork is in place; a follow PR [needs to use `cargoHash` and friends from `finalAttrs`](https://github.com/NixOS/nixpkgs/blob/10214747f5e6e7cb5b9bdf9e018a3c7b3032f5af/pkgs/build-support/rust/build-rust-package/default.nix#L104)

- //3p/overlays/tvl: - Drop upstreamed thttpd override. NixOS/nixpkgs#370071 - Update telega.el and pin tdlib to compatible version. - Some packages were affected by the change from fetchCargoTarball to fetchCargoVendor which invalidates all cargoSha256 hashes without explaining this to the user in any way. See <NixOS/nixpkgs#394012>. - //users/emery/pkgs/syndicate-server: can be removed anyways - //third_party/nixpkgs:grpc-health-check: override is unnecessary now - //third_party/nixpkgs:harmonia: updated cargoSha256 - //tvix: address some new clippy lints - //tvix/cli: Reflect combined graphical iso change. http://github.com/nixos/nixpkgs/commit/a2636dae467e16b48 - //tvix/*-go: rerun protoc on protobuf definitions - //users/aspen/system/yeren: Use default kernel version from nixpkgs as 6.11 is EOL. This means DIGImend is no longer supported. Hopefully whatever tablet aspen is using is supported by upstream now. See also - https://spbnick.github.io/2024/11/03/Letting-go-of-DIGImend.html - NixOS/nixpkgs#363873 - NixOS/nixpkgs#396730 - NixOS/nixpkgs#378830 Change-Id: If8be8b336fc6d8652ecab087decc6fbbc0509647 Reviewed-on: https://cl.tvl.fyi/c/depot/+/13276 Autosubmit: sterni <[email protected]> Reviewed-by: emery <[email protected]> Reviewed-by: tazjin <[email protected]> Reviewed-by: aspen <[email protected]> Tested-by: BuildkiteCI

emilazy requested review from TomaSajt and alyssais March 28, 2025 13:58

github-actions bot added 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 8.has: documentation This PR adds or changes documentation labels Mar 28, 2025

nix-owners bot requested review from Mic92, figsoda, winterqt and zowoq March 28, 2025 13:59

alyssais reviewed Mar 28, 2025

View reviewed changes

pkgs/build-support/rust/build-rust-package/default.nix Outdated Show resolved Hide resolved

github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 28, 2025

rustPlatform.fetchCargoTarball: drop

7cf0473

emilazy force-pushed the push-rwyzuvqrsomr branch from 1451112 to 7cf0473 Compare March 28, 2025 15:10

leona-ya approved these changes Mar 28, 2025

View reviewed changes

wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 28, 2025

TomaSajt reviewed Mar 30, 2025

View reviewed changes

emilazy merged commit 445f90e into NixOS:master Apr 2, 2025
44 checks passed

emilazy deleted the push-rwyzuvqrsomr branch April 2, 2025 20:17

rrbutani added a commit to rrbutani/zed that referenced this pull request Apr 3, 2025

nix: use fetchCargoVendor for cargo-about

a0f97b2

see the previous commit; `fetchCargoTarball` is deprecated in nixpkgs 25.05: NixOS/nixpkgs#394012

rrbutani mentioned this pull request Apr 3, 2025

nix: Fix the cargo-bundle override zed-industries/zed#28061

Merged

rrbutani added a commit to rrbutani/zed that referenced this pull request Apr 3, 2025

nix: use fetchCargoVendor for cargo-about

0251a70

see the previous commit; `fetchCargoTarball` is deprecated in nixpkgs 25.05: NixOS/nixpkgs#394012

FliegendeWurst mentioned this pull request Jun 17, 2025

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

Closed

Uh oh!

Conversation

emilazy commented Mar 28, 2025 • edited by TomaSajt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

Uh oh!

TomaSajt commented Mar 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomaSajt commented Mar 29, 2025

Uh oh!

emilazy commented Mar 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomaSajt commented Mar 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilazy commented Mar 29, 2025

Uh oh!

TomaSajt commented Mar 29, 2025

Uh oh!

emilazy commented Mar 29, 2025

Uh oh!

TomaSajt commented Mar 29, 2025

Uh oh!

emilazy commented Mar 29, 2025

Uh oh!

TomaSajt commented Mar 29, 2025

Uh oh!

emilazy commented Mar 30, 2025

Uh oh!

TomaSajt Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

emilazy Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

TomaSajt Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

emilazy Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

TomaSajt commented Mar 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

emilazy commented Mar 28, 2025 •

edited by TomaSajt

Loading

TomaSajt commented Mar 29, 2025 •

edited

Loading

emilazy commented Mar 29, 2025 •

edited

Loading

TomaSajt commented Mar 29, 2025 •

edited

Loading