Skip to content

Commit 3e9c373

Browse files
committed
Create CommonSSHStoreConfig::createSSHMaster
By moving `host` to the config, we can do a lot further cleanups and dedups. This anticipates a world where we always go `StoreReference` -> `*StoreConfig` -> `Store*` rather than skipping the middle step too. Progress on #10766 Progress on NixOS/hydra#1164
1 parent 1796444 commit 3e9c373

7 files changed

Lines changed: 58 additions & 55 deletions

File tree

src/libstore/legacy-ssh-store.cc

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,32 +32,19 @@ LegacySSHStore::LegacySSHStore(
3232
std::string_view scheme,
3333
std::string_view host,
3434
const Params & params)
35-
: LegacySSHStore{scheme, LegacySSHStoreConfig::extractConnStr(scheme, host), params}
36-
{
37-
}
38-
39-
LegacySSHStore::LegacySSHStore(
40-
std::string_view scheme,
41-
std::string host,
42-
const Params & params)
4335
: StoreConfig(params)
44-
, CommonSSHStoreConfig(params)
36+
, CommonSSHStoreConfig(scheme, host, params)
4537
, LegacySSHStoreConfig(params)
4638
, Store(params)
47-
, host(host)
4839
, connections(make_ref<Pool<Connection>>(
4940
std::max(1, (int) maxConnections),
5041
[this]() { return openConnection(); },
5142
[](const ref<Connection> & r) { return r->good; }
5243
))
53-
, master(
54-
host,
55-
sshKey.get(),
56-
sshPublicHostKey.get(),
44+
, master(createSSHMaster(
5745
// Use SSH master only if using more than 1 connection.
5846
connections->capacity() > 1,
59-
compress,
60-
logFD)
47+
logFD))
6148
{
6249
}
6350

src/libstore/legacy-ssh-store.hh

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
3333

3434
struct Connection;
3535

36-
std::string host;
37-
3836
ref<Pool<Connection>> connections;
3937

4038
SSHMaster master;
@@ -46,13 +44,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
4644
std::string_view host,
4745
const Params & params);
4846

49-
private:
50-
LegacySSHStore(
51-
std::string_view scheme,
52-
std::string host,
53-
const Params & params);
54-
public:
55-
5647
ref<Connection> openConnection();
5748

5849
std::string getUri() override;

src/libstore/ssh-store-config.cc

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
#include <regex>
22

33
#include "ssh-store-config.hh"
4+
#include "ssh.hh"
45

56
namespace nix {
67

7-
std::string CommonSSHStoreConfig::extractConnStr(std::string_view scheme, std::string_view _connStr)
8+
static std::string extractConnStr(std::string_view scheme, std::string_view _connStr)
89
{
910
if (_connStr.empty())
1011
throw UsageError("`%s` store requires a valid SSH host as the authority part in Store URI", scheme);
@@ -21,4 +22,22 @@ std::string CommonSSHStoreConfig::extractConnStr(std::string_view scheme, std::s
2122
return connStr;
2223
}
2324

25+
CommonSSHStoreConfig::CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params)
26+
: StoreConfig(params)
27+
, host(extractConnStr(scheme, host))
28+
{
29+
}
30+
31+
SSHMaster CommonSSHStoreConfig::createSSHMaster(bool useMaster, Descriptor logFD)
32+
{
33+
return {
34+
host,
35+
sshKey.get(),
36+
sshPublicHostKey.get(),
37+
useMaster,
38+
compress,
39+
logFD,
40+
};
41+
}
42+
2443
}

src/libstore/ssh-store-config.hh

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55

66
namespace nix {
77

8+
class SSHMaster;
9+
810
struct CommonSSHStoreConfig : virtual StoreConfig
911
{
1012
using StoreConfig::StoreConfig;
1113

14+
CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params);
15+
1216
const Setting<Path> sshKey{this, "", "ssh-key",
1317
"Path to the SSH private key used to authenticate to the remote machine."};
1418

@@ -30,9 +34,7 @@ struct CommonSSHStoreConfig : virtual StoreConfig
3034
* RFC2732, but also pure addresses. The latter one is needed here to
3135
* connect to a remote store via SSH (it's possible to do e.g. `ssh root@::1`).
3236
*
33-
* This function now ensures that a usable connection string is available:
34-
*
35-
* - If the store to be opened is not an SSH store, nothing will be done.
37+
* When initialized, the following adjustments are made:
3638
*
3739
* - If the URL looks like `root@[::1]` (which is allowed by the URL parser and probably
3840
* needed to pass further flags), it
@@ -44,9 +46,17 @@ struct CommonSSHStoreConfig : virtual StoreConfig
4446
*
4547
* Will throw an error if `connStr` is empty too.
4648
*/
47-
static std::string extractConnStr(
48-
std::string_view scheme,
49-
std::string_view connStr);
49+
std::string host;
50+
51+
/**
52+
* Small wrapper around `SSHMaster::SSHMaster` that gets most
53+
* arguments from this configuration.
54+
*
55+
* See that constructor for details on the remaining two arguments.
56+
*/
57+
SSHMaster createSSHMaster(
58+
bool useMaster,
59+
Descriptor logFD = INVALID_DESCRIPTOR);
5060
};
5161

5262
}

src/libstore/ssh-store.cc

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,34 +32,21 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
3232

3333
class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
3434
{
35+
public:
36+
3537
SSHStore(
3638
std::string_view scheme,
37-
std::string host,
39+
std::string_view host,
3840
const Params & params)
3941
: StoreConfig(params)
4042
, RemoteStoreConfig(params)
41-
, CommonSSHStoreConfig(params)
43+
, CommonSSHStoreConfig(scheme, host, params)
4244
, SSHStoreConfig(params)
4345
, Store(params)
4446
, RemoteStore(params)
45-
, host(host)
46-
, master(
47-
host,
48-
sshKey.get(),
49-
sshPublicHostKey.get(),
47+
, master(createSSHMaster(
5048
// Use SSH master only if using more than 1 connection.
51-
connections->capacity() > 1,
52-
compress)
53-
{
54-
}
55-
56-
public:
57-
58-
SSHStore(
59-
std::string_view scheme,
60-
std::string_view host,
61-
const Params & params)
62-
: SSHStore{scheme, SSHStoreConfig::extractConnStr(scheme, host), params}
49+
connections->capacity() > 1))
6350
{
6451
}
6552

@@ -119,6 +106,15 @@ struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfi
119106
{
120107
}
121108

109+
MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params)
110+
: StoreConfig(params)
111+
, RemoteStoreConfig(params)
112+
, CommonSSHStoreConfig(scheme, host, params)
113+
, SSHStoreConfig(params)
114+
, LocalFSStoreConfig(params)
115+
{
116+
}
117+
122118
const std::string name() override { return "Experimental SSH Store with filesystem mounted"; }
123119

124120
std::string doc() override
@@ -158,7 +154,7 @@ class MountedSSHStore : public virtual MountedSSHStoreConfig, public virtual SSH
158154
const Params & params)
159155
: StoreConfig(params)
160156
, RemoteStoreConfig(params)
161-
, CommonSSHStoreConfig(params)
157+
, CommonSSHStoreConfig(scheme, host, params)
162158
, SSHStoreConfig(params)
163159
, LocalFSStoreConfig(params)
164160
, MountedSSHStoreConfig(params)

src/libstore/ssh.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ SSHMaster::SSHMaster(
1010
std::string_view host,
1111
std::string_view keyFile,
1212
std::string_view sshPublicHostKey,
13-
bool useMaster, bool compress, int logFD)
13+
bool useMaster, bool compress, Descriptor logFD)
1414
: host(host)
1515
, fakeSSH(host == "localhost")
1616
, keyFile(keyFile)

src/libstore/ssh.hh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private:
1717
const std::string sshPublicHostKey;
1818
const bool useMaster;
1919
const bool compress;
20-
const int logFD;
20+
const Descriptor logFD;
2121

2222
struct State
2323
{
@@ -43,7 +43,7 @@ public:
4343
std::string_view host,
4444
std::string_view keyFile,
4545
std::string_view sshPublicHostKey,
46-
bool useMaster, bool compress, int logFD = -1);
46+
bool useMaster, bool compress, Descriptor logFD = INVALID_DESCRIPTOR);
4747

4848
struct Connection
4949
{

0 commit comments

Comments
 (0)