rustPlatform.fetchCargoVendor: allow duplicated dependencies

[TomaSajt]

This is a breaking change (even if mostly internal). Because of this, it's not really backportable either.

Reviews are welcome.

Fixes #359340
Fixes #30742

Related:

• buildRustPackage: found duplicate version of package #30742
• rustPlatform.importCargoLock fails with git dependencies of the same package and version #183344
• fetchCargoVendor fails for duplicate versions of same crate #359340
• rustPlatform.importCargoLock: allow duplicated packages with git. #282798

---

This PR contains the simpler but arguably more important part of #282798.
The other PR tries to migrate both importCargoLock and fetchCargoVendor, but this one only does the fetchCargoVendor change.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[nyabinary]

Should target staging considering the amount of rebuilds

[TomaSajt]

Yes I'll point it there soon, but I still have a few TODOs.

[poelzi]

Unfortunately, this does not fix the sui problem. I debugged it by adding "set -x" on the cargo-vendor-dir command:

+++ ln -s /nix/store/a3hq2s4jsrjlk5flzm6bwibc63habljn-tokio-io-timeout-1.2.0 /nix/store/hrw1qsrqh3mfasw72hlapd0lnkd303a1-cargo-vendor-dir/tokio-io-timeout-1.2.0
+++ '[' -e /nix/store/a3hq2s4jsrjlk5flzm6bwibc63habljn-tokio-io-timeout-1.2.0/.cargo-config ']'
+++ for crate in /nix/store/w4njzhl6c2ap84kd6v9c09g3vb3jwpwr-Inflector-0.11.4 /nix/store/61myz4b2wg5bcwrmcgqir98p1kjh0yjf-addchain-0.2.0 /nix/store/dfdynm0vivlyw1cyxabpnkmk0gbfvn6v-add>
++++ basename /nix/store/pbkdlfdqi3gzigz428kbay8fcwvv6z7z-tokio-macros-2.5.0
++++ cut -c 34-
+++ ln -s /nix/store/pbkdlfdqi3gzigz428kbay8fcwvv6z7z-tokio-macros-2.5.0 /nix/store/hrw1qsrqh3mfasw72hlapd0lnkd303a1-cargo-vendor-dir/tokio-macros-2.5.0
+++ '[' -e /nix/store/pbkdlfdqi3gzigz428kbay8fcwvv6z7z-tokio-macros-2.5.0/.cargo-config ']'
+++ for crate in /nix/store/w4njzhl6c2ap84kd6v9c09g3vb3jwpwr-Inflector-0.11.4 /nix/store/61myz4b2wg5bcwrmcgqir98p1kjh0yjf-addchain-0.2.0 /nix/store/dfdynm0vivlyw1cyxabpnkmk0gbfvn6v-add>
++++ basename /nix/store/rk97wgy1a6m8x9z5k05dzw1fmhbqb3wx-tokio-macros-2.5.0
++++ cut -c 34-
+++ ln -s /nix/store/rk97wgy1a6m8x9z5k05dzw1fmhbqb3wx-tokio-macros-2.5.0 /nix/store/hrw1qsrqh3mfasw72hlapd0lnkd303a1-cargo-vendor-dir/tokio-macros-2.5.0
ln: failed to create symbolic link '/nix/store/hrw1qsrqh3mfasw72hlapd0lnkd303a1-cargo-vendor-dir/tokio-macros-2.5.0/rk97wgy1a6m8x9z5k05dzw1fmhbqb3wx-tokio-macros-2.5.0': Permission den>
+ exitHandler
+ exitCode=1
+ set +e
+ '[' -n '' ']'
+ ((  1 != 0  ))
+ runHook failureHook
+ local hookName=failureHook

The problem is triggered by 2 versions of the tokio-macros used by sui

[TomaSajt]

You are using cargoLock.lockFile (backend: importCargoLock)
instead of cargoHash + useFetchCargoVendor = true (backend: fetchCargoVendor).

In this PR I only modified fetchCargoVendor.

I was able to build sui with this:
(do note that you need like over 100GiB of storage to build it...)

{
  lib,
  rustPlatform,
  fetchFromGitHub,
  pkg-config,
  zstd,
}:

rustPlatform.buildRustPackage (finalAttrs: {
  pname = "sui";
  version = "1.43.1";

  src = fetchFromGitHub {
    owner = "MystenLabs";
    repo = "sui";
    tag = "mainnet-v${finalAttrs.version}";
    hash = "sha256-mNljh3HupusGmfT3pXtjqUp7OZHhyWd6jNiFiKlSYpk=";
  };

  useFetchCargoVendor = true;
  cargoHash = "sha256-ckotvpQw3WfvJ2YXR/XKT7LamGj7kLtGwMR/qrXpmYc=";

  nativeBuildInputs = [
    pkg-config
    rustPlatform.bindgenHook
  ];

  buildInputs = [
    zstd
  ];

  env = {
    # if this is not set the build will try to invoke git to get the rev
    GIT_REVISION = "nixpkgs@${finalAttrs.version}";
    ZSTD_SYS_USE_PKG_CONFIG = true;
  };

  # the build takes up more than 60G of space even without the checks
  # my system ran out of space (150G) when having tests enabled
  doCheck = false;

})

[poelzi]

Ahh, I see. I used useFetchCargoVendor = true and cargoLock.
Works now, thanks a lot 👍🏽

[TomaSajt]

New format:

$ ls -A result/
.cargo  source-git-0  source-git-1  source-git-2  source-registry-0  Cargo.lock

$ cat result/.cargo/config.toml

[source._vendored-source-registry-0]
directory = "@vendor@/source-registry-0"

[source.crates-io]
replace-with = "_vendored-source-registry-0"

[source._vendored-source-git-0]
directory = "@vendor@/source-git-0"

[source._original-source-git-0]
git = "https://github.com/biomejs/biome"
rev = "2648fa4201be4afd26f44eca1a4e77aac0a67272"
replace-with = "_vendored-source-git-0"

[source._vendored-source-git-1]
directory = "@vendor@/source-git-1"

[source._original-source-git-1]
git = "https://github.com/lionel-/tower-lsp"
branch = "bugfix/patches"
replace-with = "_vendored-source-git-1"

[source._vendored-source-git-2]
directory = "@vendor@/source-git-2"

[source._original-source-git-2]
git = "https://github.com/r-lib/tree-sitter-r"
rev = "a0d3e3307489c3ca54da8c7b5b4e0c5f5fd6953a"
replace-with = "_vendored-source-git-2"

[DavHau]

I tested this patch today on a project where it was running into the failed to create symbolic link error and it fixed it successfully.

EDIT: Though I'm now running into an error like this:

 error: failed to select a version for the requirement `cfg-if = "^1.0"` (locked to 1.0.0)

Not sure if this is related to the fetching or not

EDIT: forget about it. This seems to be an unrelated issue

[TomaSajt]

I tested this patch today on a project where it was running into the failed to create symbolic link error and it fixed it successfully.

EDIT: Though I'm now running into an error like this:

 error: failed to select a version for the requirement `cfg-if = "^1.0"` (locked to 1.0.0)

Not sure if this is related to the fetching or not

If this is a public project, please share a link to it.
If not, please share some more details/logs.

[DavHau]

@TomaSajt My issue seems to be unrelated. Please don't feel blocked by it

[MultisampledNight]

Packaging Kiesel seems to be blocked on this.

Kiesel uses an in-tree Rust crate for some FFI, which depends partly on crates.io versions, partly on git versions to get the latest fixes. A lot of transitive deps are duplicated in turn, too.
Just replacing them with crates.io is not practical, as the rest of the project also has the exact git commits pinned.
(Current attempt is at MultisampledNight@ec0f801 if anybody is curious, very half-baked.)

[MultisampledNight]

Simple enough impl, impact is bearable.

[MultisampledNight]

Nit: Could maybe refactor this to instead store lists of registry and git sources, then constructing the source_config afterwards. This would avoid bugs due to missing to increment the indices. Works fine as-is though.

[TomaSajt]

Pushed a change factoring out some stuff to slightly de-clutter the create_vendor function and hopefully made some of the logic more readable.
Diff

[gepbird]

What's your opinion on versioning fetchCargoVendor similarly to #422975 ?

The main advantage of it would be that we can make more frequent changes to the fetcher, because it will only affect packages that opt into an update, rather than (pretty much) all rust packages.

For example let's suppose all packages have cargoHashVersion = 1 set, their cargoHash should never change due to changes in fetchCargoVendor. Then we have the issue of packages not building with duplicate versions of the same crate, we could make a quick hotfix for it which affects cargoHashVersion = 2 or greater. People who had this issue will be happy that they can use a fix in a reasonable time. If the implementation contained other bugs, or there's still room for improvement, it's not a big deal: just fix it in cargoHashVersion = 3 and so on.

The downside is adding extra complexity and maintenance burden.

[TomaSajt]

We could have versioning in the future, but this PR currently does not break any hashes.

It only breaks an assumption about the final layout of stuff, not the FOD hash.

Currently, the place I could imagine versioning being needed is if we start supporting multiple non-git registries and also want to support duplicate packages in there (because we currently just have only one "tarballs" directory in the FOD, unlike with git deps)

Be we could still avoid versioning even in that situation by being sneaky and adding special cases.

---

In any case, having this double layered fetcher implementation is very ideal when considering hash stability.
(The cache concern is another story)

---

The annoying thing for this change specifically would be backporting.
Can we really backport this small breaking change, even if it doesn't break the hashes? I don't think we can. Out of tree stuff may also be relying on the layout assumptions.
And if we cant backport, the workaround patches in stable will still be needed for some packages that hacked around the no duplicate packages support.

[MultisampledNight]

re Backporting: Given that 25.11 is coming up we could "just" do it with the stable bump

[poelzi]

Can we land this please. This bug is very annoying for some flakes and clearly wrong behavior on the fetch site.

[poelzi]

@TomaSajt thank you so much - this shit is blocking proper sui packaging for so long.

[TomaSajt]

Holy... this actually got merged.

For the record: since this is not exactly backportable (in its current form), packages in stable still need to use the $cargoDepsCopy/pname-version instead of $cargoDepsCopy/*/pname-version in their patches. So look out for this when backporting changes in the touched packages.

Also, packages that do patching to their lockfiles and also want to backport version updates probably have to carry the patches until 25.11 goes EOL.

I can imagine making a followup PR that adds a stable-branch-only flag to the fetcher that could opt in to the new behaviour on stable, too.
But I'll not work on it unless there is a demand for it.