Skip to content

rustPlatform.fetchCargoVendor: de-duplicate git sources by selector#501979

Merged
TomaSajt merged 2 commits into
NixOS:stagingfrom
CathalMullan:fetch-cargo-vendor
Jun 5, 2026
Merged

rustPlatform.fetchCargoVendor: de-duplicate git sources by selector#501979
TomaSajt merged 2 commits into
NixOS:stagingfrom
CathalMullan:fetch-cargo-vendor

Conversation

@CathalMullan

@CathalMullan CathalMullan commented Mar 21, 2026

Copy link
Copy Markdown
Member

If a Cargo project contains multiple bare git sources (no explicit rev, branch or tag) for the same repo, the lockfile entry looks like:

[[package]]
name = "cosmic-settings-daemon"
version = "0.1.0"
source = "git+https://github.com/pop-os/dbus-settings-bindings#0fa672f8dadb884001ef9a251b149ed432879629"

[[package]]
name = "cosmic-dbus-a11y"
version = "0.1.0"
source = "git+https://github.com/pop-os/dbus-settings-bindings#3b86984332be2c930a3536ab714b843c851fa8ca"

The new vendor code (#387337) treats each unique source string as a separate source:

[source.original-source-git-5]
git = "https://github.com/pop-os/dbus-settings-bindings"
replace-with = "vendored-source-git-5"

[source.original-source-git-7]
git = "https://github.com/pop-os/dbus-settings-bindings"
replace-with = "vendored-source-git-7"
error: source `original-source-git-7` defines source
  https://github.com/pop-os/dbus-settings-bindings,
  but that source is already defined by `original-source-git-5`
note: Sources are not allowed to be defined multiple times.

This change de-duplicates by selector, matching what cargo vendor does.

Encountered this issue while trying to build cosmic-settings-daemon.

cc: @TomaSajt

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci Bot requested a review from TomaSajt March 21, 2026 16:26
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. labels Mar 21, 2026
@CathalMullan

CathalMullan commented Mar 21, 2026

Copy link
Copy Markdown
Member Author

Hmm, looking at this more closely, I don't think this is the right fix if the aim is to match cargo vendor.

cargo vendor deduplicates to:

[source."git+https://github.com/pop-os/dbus-settings-bindings"]
git = "https://github.com/pop-os/dbus-settings-bindings"
replace-with = "vendored-sources"

I'll switch to a draft for now.

@CathalMullan CathalMullan marked this pull request as draft March 21, 2026 17:12
@CathalMullan CathalMullan changed the title rustPlatform.fetchCargoVendor: include rev in bare git sources rustPlatform.fetchCargoVendor: de-duplicate git sources by selector Mar 21, 2026
@CathalMullan CathalMullan force-pushed the fetch-cargo-vendor branch 2 times, most recently from fc4f2fd to beefe04 Compare March 21, 2026 17:55
@CathalMullan

Copy link
Copy Markdown
Member Author

Okay, new approach is to de-dupe git sources by selector, which matches upstream cargo vendor behavior.

@CathalMullan CathalMullan marked this pull request as ready for review March 21, 2026 18:05
@Pandapip1

Copy link
Copy Markdown
Member

Probably worth targeting staging-next instead of staging as this is a fix.

@TomaSajt

Copy link
Copy Markdown
Contributor

Sure, we can automatically deduplicate, but is that what we really want? Why should we give one of the revisions priority over the other? They are not the same.

Also: did this work before #387337 ? I fail to imagine how it could have been broken because of it.

@CathalMullan

CathalMullan commented Mar 23, 2026

Copy link
Copy Markdown
Member Author

Agree this isn't exactly a great solution, but I don't think there's any other option here.

Latest cosmic-settings-daemon update was merged a month ago and built fine: e0e2946

Only started failing once the new vendor code was merged in from staging.

The old vendor code chose 0fa672f8 as the winning rev:

> git checkout e0e2946b5372d187ad3d60acfc5e5ddebead212c
> nix build .#cosmic-settings-daemon
...
cosmic-settings-daemon>    Compiling cosmic-settings-daemon v0.1.0 (https://github.com/pop-os/dbus-settings-bindings#0fa672f8)
cosmic-settings-daemon>    Compiling cosmic-dbus-a11y v0.1.0 (https://github.com/pop-os/dbus-settings-bindings#0fa672f8)

@TomaSajt

Copy link
Copy Markdown
Contributor

Okay, I see why it was working before, and why it's not working now.
I thought the package was also a duplicate, but no, it's only the selector that's duplicated.

I really don't see a pretty way of solving this.
Your fix seems to do its job in this case.
However, if we had both a duplicate package and a duplicate selector, even your fix wouldn't work, because we'd be back to the situation that #387337 aimed to solve.

I'll look into whether your approach could be improved.

In the meantime you could patch out one of the revisions in the lockfile over to the other revision. It's not guaranteed to be correct, but it might be fine as a hotfix.

@CathalMullan

Copy link
Copy Markdown
Member Author

The affected COSMIC packages have been patched already: #502494

AFAIK those were the only 2 packages affected.

@nyabinary

Copy link
Copy Markdown
Contributor

The affected COSMIC packages have been patched already: #502494

AFAIK those were the only 2 packages affected.

Yeah but this issue is still worth fixing since it could happen to other packages as well.

@thefossguy thefossguy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied this commit on top of nixos-unstable-small (6c9a78c09ff4d6c21d0319114873508a6ec01655) and built an ISO with COSMIC. I performed the usual testing (that is done when I bump packages) by booting in the ISO and nothing seemed broken there.

I also ran the nixosTests.cosmic* tests.
ALL tests PASS (4/4).

The ISO is on the nix-community x86 builder build01 if anyone wants to verify, at path /nix/store/07c989d20qbywbm4nfa8cmfw57ijvi7f-nixos-cosmic-26.05pre56789.gfedcba-x86_64-linux.iso/iso/nixos-cosmic-26.05pre56789.gfedcba-x86_64-linux.iso.

@thefossguy

Copy link
Copy Markdown
Member

Would love a review from @NixOS/rust btw

@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 24, 2026
@AL-33

AL-33 commented Mar 25, 2026

Copy link
Copy Markdown

Hi,
I'm getting this error that's blocking me from making changes/updating my system.
Can you tell me how to fix or work around it?

@thefossguy

Copy link
Copy Markdown
Member

@AL-33 what error do you get?

@AL-33

AL-33 commented Mar 26, 2026

Copy link
Copy Markdown

I was getting this kind of error:

Cargo.toml error: source `original-source-git-7` defines source
  https://github.com/pop-os/dbus-settings-bindings,
  but that source is already defined by `original-source-git-5`
note: Sources are not allowed to be defined multiple times.

when running sudo nixos-rebuild boot --upgrade for the past three days.
It was failing to build 'cosmic-settings-daemon' due to 'dbus-setting-bindings'.

Today, the problem has disappeared, and I was able to update without any
issues.

@Eveeifyeve Eveeifyeve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not from the rust team, I think this would be a great idea to add this and from code wise I see no issues.

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Mar 27, 2026
@TomaSajt TomaSajt force-pushed the fetch-cargo-vendor branch from beefe04 to 1c9d43e Compare May 5, 2026 15:42
@nixpkgs-ci nixpkgs-ci Bot added the 10.rebuild-nixos-tests This PR causes rebuilds for all NixOS tests and should normally target the staging branches. label May 5, 2026
@TomaSajt

TomaSajt commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

For any new people stumbling across this PR, please do leave a comment so I can see how common this issue is.

@TomaSajt TomaSajt force-pushed the fetch-cargo-vendor branch from ac3c41d to 100e233 Compare June 2, 2026 13:26
@thefossguy

Copy link
Copy Markdown
Member

We encountered this again in bumping the COSMIC DE packages: #527825 (comment)

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Jun 5, 2026
@TomaSajt TomaSajt added this pull request to the merge queue Jun 5, 2026
@TomaSajt TomaSajt added the backport staging-26.05 Backport PR automatically label Jun 5, 2026
Merged via the queue into NixOS:staging with commit 67363ee Jun 5, 2026
32 of 34 checks passed
@nixpkgs-ci

nixpkgs-ci Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Backport failed for staging-26.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin staging-26.05
git worktree add -d .worktree/backport-501979-to-staging-26.05 origin/staging-26.05
cd .worktree/backport-501979-to-staging-26.05
git switch --create backport-501979-to-staging-26.05
git cherry-pick -x 55b813b5d77c96dce549930e1449e902839a1bad 100e23324c871a3b9210bd50c59d4f22e6cc452c

@TomaSajt

TomaSajt commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Opened backport #528591

@CathalMullan CathalMullan deleted the fetch-cargo-vendor branch June 6, 2026 22:03
@thefossguy thefossguy mentioned this pull request Jun 10, 2026
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 8.has: failed backport 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-nixos-tests This PR causes rebuilds for all NixOS tests and should normally target the staging branches. 12.approvals: 3+ This PR was reviewed and approved by three or more persons. backport staging-26.05 Backport PR automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants