Skip to content

nix: Fix the cargo-bundle override#28061

Merged
P1n3appl3 merged 2 commits intozed-industries:mainfrom
rrbutani:fix/nix-cargo-about
Apr 3, 2025
Merged

nix: Fix the cargo-bundle override#28061
P1n3appl3 merged 2 commits intozed-industries:mainfrom
rrbutani:fix/nix-cargo-about

Conversation

@rrbutani
Copy link
Contributor

@rrbutani rrbutani commented Apr 3, 2025

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 no longer works:

zed/nix/build.nix

Lines 100 to 116 in c6e2d20

(cargo-bundle.overrideAttrs (
new: old: {
version = "0.6.1-zed";
src = fetchFromGitHub {
owner = "zed-industries";
repo = "cargo-bundle";
rev = "2be2669972dff3ddd4daf89a2cb29d2d06cad7c7";
hash = "sha256-cSvW0ND148AGdIGWg/ku0yIacVgW+9f1Nsi+kAQxVrI=";
};
# https://nixos.asia/en/buildRustPackage
cargoDeps = old.cargoDeps.overrideAttrs ({
inherit src;
name = "${new.pname}-${new.version}-vendor.tar.gz";
outputHash = "sha256-Q49FnXNHWhvbH1LtMUpXFcvGKu9VHwqOXXd+MjswO64=";
});
}
))

fetchCargoTarball produced a single derivation but fetchCargoVendor produces two:

  • ${name}-vendor-staging (inner; FoD)
  • ${name}-vendor (outer)

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:

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 instead. This change has us just do the call to fetchCargoTarball manually instead of using overrides.


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 since before the nixpkgs bump in 50ad71a)


Note

eventually we'll be able to just have .overrideAttrs (_: { cargoHash = "..."; }) work as expected 1


Release Notes:

  • N/A

Footnotes

  1. now that buildRustPackage uses lib.extendMkDerivation (NixOS/nixpkgs/#234651) the groundwork is in place; a follow PR needs to use cargoHash and friends from finalAttrs

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)
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Apr 3, 2025
@maxdeviant maxdeviant changed the title nix: fix the cargo-bundle override nix: Fix the cargo-bundle override Apr 3, 2025
@P1n3appl3 P1n3appl3 added nix run-nix Configures PR to run nix CI labels Apr 3, 2025
@P1n3appl3 P1n3appl3 self-assigned this Apr 3, 2025
see the previous commit; `fetchCargoTarball` is deprecated in nixpkgs
25.05: NixOS/nixpkgs#394012
@rrbutani rrbutani force-pushed the fix/nix-cargo-about branch from a0f97b2 to 0251a70 Compare April 3, 2025 23:03
@P1n3appl3
Copy link
Contributor

P1n3appl3 commented Apr 3, 2025

Thanks for the fix and the clear/detailed explanation!

@P1n3appl3 P1n3appl3 enabled auto-merge (squash) April 3, 2025 23:14
@P1n3appl3 P1n3appl3 merged commit c04c581 into zed-industries:main Apr 3, 2025
16 of 18 checks passed
@P1n3appl3 P1n3appl3 mentioned this pull request Apr 3, 2025
@rrbutani rrbutani deleted the fix/nix-cargo-about branch April 3, 2025 23:36
tmickleydoyle pushed a commit that referenced this pull request Apr 6, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement run-nix Configures PR to run nix CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants