Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a unix remote store without the filesystem required mounted #9429

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ericson2314
Copy link
Member

Motivation

This allows creating a proxy to access some other sort of remote store.

For example, I talked to @kolloch about using this to talk to a daemon running as a regular user which needs to do some extra auth steps access a binary cache. The credentials should be owned by that user not root / the daemon user, and it might need to e.g. spawn some sort of "did you authorize this?" GUI.

Context

A very natural follow-up from #7912 --- now we have all four combinations of

  • ssh, unix domain socket
  • mounted/unmounted on OS file system

TODO needs some tests

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store labels Nov 21, 2023
@tomberek
Copy link
Contributor

Could this be used to implement a "prefix rewriting store"? It would keep the existing store, but then also do path rewriting for local usage.

@Ericson2314
Copy link
Member Author

Perhaps? I am not quire sure what you mean.

This works just like the (unmounted) ssh-ng:// store, except it uses a Unix domain socket instead of an ssh connection.


This store allows full access to a Nix store on a remote machine and additionally requires that the store be mounted in the local filesystem.

- [`unmounted-unix://`](@docroot@/command-ref/new-cli/nix3-help-stores.md)
Copy link
Member

@roberth roberth Nov 24, 2023

Choose a reason for hiding this comment

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

Getting a sense of impending combinatorial explosion here.
Mounted or not seems orthogonal to the protocol used.
Inheritance isn't suitable for arbitrary composition of orthogonal behaviors, so this suggests that the Store hierarchy should be refactored instead, to use value composition instead of inheritance for this purpose.
As far as URIs, that could exhibit as a more regular format such as

ssh-ng://foo
ssh-ng://foo?mounted=true
unix://?mounted=false
unix://

The only remaining irregularity is in the defaults, but I feel like that's expected and part of the domain.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps the mount could be considered part of the protocol, and in a way it is, but then it should be part of the scheme, where we don't have a syntax for removing a protocol. E.g. ssh-ng+mount: would be ok, but unix-mount: makes no sense out of context.

Main point is the explosion of subclasses, not the syntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

I totally agree: we're exposing the deficiency in the current architecture. (And we're proving once again why OOP is bad / a failure --- this is supposed to be what it is good at!)

Still, I think this is useful functionality to have. And I think having a little bit of explosion to remind us that we really need to fix the arch is OK. We discussed this briefly and @roberth said then we should document that this syntax is an ugly stop-gap and better not be stabilized. Sounds great to me!

Copy link
Member

Choose a reason for hiding this comment

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

Well-documented, explicitly experimental stop gap is ok.

@Ericson2314 Ericson2314 force-pushed the uds-remote-without-local-fs branch 2 times, most recently from 5422ed3 to 7a60270 Compare November 24, 2023 15:26
@roberth

This comment was marked as resolved.

@roberth roberth dismissed their stale review November 27, 2023 23:45

Combinatoral class explosion postponed through experimental flag

@Ericson2314 Ericson2314 mentioned this pull request Dec 6, 2023
We now have mounted/unmounted variations for both remote stores, and
they both should have a better solution for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants