Implement SSHStore on top of nix-daemon served over ssh #1053

shlevy · 2016-09-02T18:34:51Z

Fixes #986

…gisterStoreImplementation

shlevy · 2016-09-02T18:35:02Z

Replaces #997

shlevy · 2016-09-02T18:35:39Z

@edolstra I will port over build-remote and nix-copy-closure to this once merged

shlevy · 2016-09-09T15:16:15Z

@edolstra ping

edolstra · 2016-09-12T11:51:35Z

src/libstore/ssh-store.cc

+                else
+                    execlp("ssh", "ssh", "-N", "-M", "-S", socketPath.c_str(), "-i", key.c_str(), uri.c_str(), NULL);
+                throw SysError("starting ssh master");
+      }))


It would probably be good to start the SSH master lazily. Otherwise, if you have an SSH store in your binary-caches setting, the SSH master will be started for every Nix command, even when the SSH store isn't used.

How is the SSH master cleaned up? On Linux, dieWithParent is implicit in startProcess(), but what would happen on OS X?

sshMaster is a Pid https://github.com/shlevy/nix/blob/584f8a62de117ade154ec8208d939dc194782936/src/libutil/util.cc#L693

OK, will implement laziness.

edolstra · 2016-09-12T12:04:36Z

src/nix-daemon/nix-daemon.cc

+
+                auto socketDir = dirOf(socketPath);
+                if (chdir(socketDir.c_str()) == -1)
+                    throw SysError(format("changing to socket directory %1%") % socketDir);


Missing quotes around %1%.

edolstra · 2016-09-12T12:11:11Z

Looks good to me.

However: Regarding nix-copy-closure, it will need to be backwards compatible. Otherwise, for instance, a NixOps on Nix 1.12 wouldn't be able to copy closures to a pre-1.12 system. From that perspective, it would actually make sense to implement SSHStore on top of the nix-store --worker protocol.

shlevy · 2016-09-12T12:16:51Z

@edolstra I don't think it makes sense to implement the entire store API in nix-store --serve, but perhaps a fallback for just the operations already implemented makes sense. Does that sound OK?

shlevy · 2016-09-12T12:19:19Z

Ugh, there's really no clean way to do that. Maybe it's better to just implement nix-copy-closure on top of the serve API directly and have nix copy be the way of the future.

shlevy · 2016-09-12T12:30:03Z

@edolstra Alternatively, could we port the nix-daemon changes to 1.11.5 and have a milder compatibility break that way?

shlevy · 2016-09-12T12:30:13Z

(thanks to @domenkozar for the idea)

domenkozar · 2016-09-12T12:32:05Z

The other alternative I see is that we:

ship 1.12 with perl only for nix-copy-closure, so it supports both protocols
really remove perl in 1.13 (maybe 2.0)

I hate this option, but it's better than rewriting some software for one release.

shlevy · 2016-09-12T12:36:07Z

Actually, we could also bundle nix-copy-closure.pl with the perl bindings package and have nixops hard-code the path to that one, so main nix is still perl-free.

shlevy · 2016-09-12T12:38:03Z

OK my preferred solution:

remove nix-copy-closure from main nix altogether. nix copy is the way for the future
Have nix-perl contain nix-copy-closure (perl version), and include it by default (maybe with a deprecation warning) in user environments, hard-coded in nixops, etc. until it seems safe to not do so, but give users a way to be perl free if they want.

domenkozar · 2016-09-13T19:11:00Z

I like that. It's the same as my previous suggestion, but with better separation of concerns.

Can't really judge how much that spares work over reimplementing it in C++.

shlevy · 2016-09-21T21:02:11Z

@edolstra ?

edolstra · 2016-09-23T10:41:19Z

My preferred solution would be to have a minimal SSHStore that uses nix-store --serve. That would give the least headaches with backward compatibility. We can keep the full-featured SSHStore, renaming it to SSHStoreNG or something like that.

A more complicated solution would be to have a full-featured, backward-compatible SSHStore that uses nix-store --serve, by extending the serve protocol to upgrade to the Nix daemon protocol if both sides support it. So:

nix-store --serve sends a server version of 0x203. (Nix 1.11 is 0x202.)
If the client sees a server version of 0x203, it sends a client version of 0x203.
If the server sees a client version of 0x203, it then switches over to the Nix daemon protocol for the rest of the connection. This would require moving most of nix-daemon into a library so it can be used by nix-store --serve.
If the server sees a client version < 0x203, it uses the old --serve protocol.
If the client sees a server version < 0x203, it sends a client version of 0x202 and uses the old --serve protocol. Thus SSHStore will only be able to provide a few operations like copying paths.

shlevy · 2016-09-23T10:53:52Z

Why is it worth a decent chunk of work (implementing a C++ nix-store --serve client, rewriting nix-copy-closure again) for features we already know in advance are going away eventually (nix-copy-closure for nix copy, serve-store and 1.11 compat for sshstore) when we already have perfectly good backwards compatibility options in keeping nix-copy-closure perl and distributing it either with nix itself or with a separate package that is bundled by default?

shlevy · 2016-09-23T10:54:42Z

In either case, are there concerns with this PR? It would be good to have it merged before too much chance for drift happens.

shlevy · 2016-09-23T10:56:57Z

We could have a non-default --without-perl configure flag, and have --with-perl bring in nix-perl during the compliation process. So people building from source get backwards compat by default, and people building in nixpkgs we can do the bundling however we want (either in separate packages automatically included together or or in a single derivation), and people who are ready to switch to nix copy can do so.

shlevy · 2016-10-02T01:33:26Z

@edolstra ping

shlevy · 2016-10-07T14:22:45Z

@edolstra ping

shlevy · 2016-10-12T14:10:13Z

@edolstra I will do a dumb rewrite of nix-copy-closure like I did with nix-build if you insist on the backwards compatibility, but what is your thought on this PR (and the build-remote that will be based on it)?

shlevy · 2016-10-20T21:58:25Z

@edolstra ping

Ericson2314 · 2016-10-20T22:35:48Z

...I have to say, while its no news things can move slowly around here, I'd hope things could be better the sake of @shlevy getting his pay and those that chipped in getting their Perl-freedom in a timely manner.

edolstra · 2016-10-21T18:40:22Z

Sorry about the delay, will try to make some progress on this the next few days.

copumpkin · 2016-11-02T16:29:52Z

Friendly bump 😄 I'd hate for this to die/bitrot

Ericson2314 · 2016-11-09T19:02:27Z

🎉

shlevy added 8 commits September 2, 2016 06:35

Merge openStore and openStoreAt with default arguments

a91954f

Factor a function to get the store type from a URI out of the main Re…

53b27dd

…gisterStoreImplementation

nix-daemon: add --stdio flag for handling connections over stdin/stdout

7d4ccd9

Factor out the unix domain socket-specific code from RemoteStore

0f39633

Factor a general remote FS accessor out of BinaryCacheStore

a705e8c

Add narFromPath op to nix daemon

b4b5e9c

Add ssh store implementation

ecba88d

Implement nar-based addToStore for remote-store

584f8a6

shlevy mentioned this pull request Sep 2, 2016

Implement SSHStore on top of nix-daemon served over ssh #997

Closed

shlevy added this to the perl-to-c++ milestone Sep 2, 2016

edolstra reviewed Sep 12, 2016
View reviewed changes

Inline ssh-store.hh into ssh-store.cc

ab31f99

edolstra reviewed Sep 12, 2016
View reviewed changes

shlevy added 2 commits September 12, 2016 08:07

ssh-store: Start master on-demand

196815f

nix-daemon: Fix error message

b99c6e0

edolstra mentioned this pull request Oct 12, 2016

Builds abort with RemoteStore::addToStore() not implemented NixOS/hydra#398

Closed

edolstra merged commit b99c6e0 into NixOS:master Nov 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SSHStore on top of nix-daemon served over ssh #1053

Implement SSHStore on top of nix-daemon served over ssh #1053

shlevy commented Sep 2, 2016

shlevy commented Sep 2, 2016

shlevy commented Sep 2, 2016

shlevy commented Sep 9, 2016

edolstra Sep 12, 2016 •

edited

Loading

edolstra Sep 12, 2016

shlevy Sep 12, 2016

shlevy Sep 12, 2016

shlevy Sep 12, 2016

edolstra Sep 12, 2016

shlevy Sep 12, 2016

edolstra commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

domenkozar commented Sep 12, 2016 •

edited

Loading

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

domenkozar commented Sep 13, 2016

shlevy commented Sep 21, 2016

edolstra commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Oct 2, 2016

shlevy commented Oct 7, 2016

shlevy commented Oct 12, 2016

shlevy commented Oct 20, 2016

Ericson2314 commented Oct 20, 2016

edolstra commented Oct 21, 2016

copumpkin commented Nov 2, 2016

Ericson2314 commented Nov 9, 2016

Implement SSHStore on top of nix-daemon served over ssh #1053

Implement SSHStore on top of nix-daemon served over ssh #1053

Conversation

shlevy commented Sep 2, 2016

shlevy commented Sep 2, 2016

shlevy commented Sep 2, 2016

shlevy commented Sep 9, 2016

edolstra Sep 12, 2016 • edited Loading

Choose a reason for hiding this comment

edolstra Sep 12, 2016

Choose a reason for hiding this comment

shlevy Sep 12, 2016

Choose a reason for hiding this comment

shlevy Sep 12, 2016

Choose a reason for hiding this comment

shlevy Sep 12, 2016

Choose a reason for hiding this comment

edolstra Sep 12, 2016

Choose a reason for hiding this comment

shlevy Sep 12, 2016

Choose a reason for hiding this comment

edolstra commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

domenkozar commented Sep 12, 2016 • edited Loading

shlevy commented Sep 12, 2016

shlevy commented Sep 12, 2016

domenkozar commented Sep 13, 2016

shlevy commented Sep 21, 2016

edolstra commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Sep 23, 2016

shlevy commented Oct 2, 2016

shlevy commented Oct 7, 2016

shlevy commented Oct 12, 2016

shlevy commented Oct 20, 2016

Ericson2314 commented Oct 20, 2016

edolstra commented Oct 21, 2016

copumpkin commented Nov 2, 2016

Ericson2314 commented Nov 9, 2016

edolstra Sep 12, 2016 •

edited

Loading

domenkozar commented Sep 12, 2016 •

edited

Loading