Skip to content

fix(system): fix copying closures between remotes#174

Merged
water-sucks merged 1 commit intonix-community:mainfrom
Sporif:system-fix-remote-copy-closure
Feb 12, 2026
Merged

fix(system): fix copying closures between remotes#174
water-sucks merged 1 commit intonix-community:mainfrom
Sporif:system-fix-remote-copy-closure

Conversation

@Sporif
Copy link
Copy Markdown
Collaborator

@Sporif Sporif commented Feb 10, 2026

  • Fix incorrect source address when copying closures between remotes
  • Avoid copying closure when remotes have the same address (see NixOS/nixpkgs@610e1e8d)

 - Fix incorrect source address when copying closures between remotes
 - Avoid copying closure when remotes have the same address
   (see NixOS/nixpkgs@610e1e8d)
@Sporif
Copy link
Copy Markdown
Collaborator Author

Sporif commented Feb 10, 2026

I'm unsure whether to use Address() or address. If two remotes have the same address but a different user and/or port, are they still guaranteed to have same /nix/store?

@water-sucks
Copy link
Copy Markdown
Collaborator

I'm unsure whether to use Address() or address. If two remotes have the same address but a different user and/or port, are they still guaranteed to have same /nix/store?

For most systems, I would imagine this assumption is correct, since most users are running SSH on the same user@address:port everywhere for their targets. However, I can definitely think of a few scenarios where this would not be the case; one being a reverse proxy with multiple SSH ports exposed that each lead to different hosts. So user@home.snare.dev:22 could be proxied to a different machine than user@home.snare.dev:23.

I think it's best to take a conservative approach and use the whole host string with Address(). I do admit that naming the field address is kind of an oversight on my part, since elsewhere it's referred to mostly as a "host". I can address the naming inconsistency in the docs and code in a future PR I would hope.

@Sporif
Copy link
Copy Markdown
Collaborator Author

Sporif commented Feb 11, 2026

Ok, then at least from my side this is fine to merge.

Copy link
Copy Markdown
Collaborator

@water-sucks water-sucks left a comment

Choose a reason for hiding this comment

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

Did some rudimentary testing, seems good to me!

@water-sucks water-sucks merged commit 9a2e97e into nix-community:main Feb 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants