Ensure we can construct remote store configs in isolation

Progress towards NixOS#10766 I thought that NixOS#10768 achieved, but when I went to use this stuff (in Hydra), turns out it did not. (Those `using FooConfig;` lines were not working --- they are so finicky!) This PR gets the job done, and adds some trivial unit tests to make sure I did what I intended. I had to add add a header to expose `SSHStoreConfig`, after which the preexisting `ssh-store-config.*` were very confusingly named files, so I renamed them to `common-ssh-store-config.hh` to match the type defined therein.
obsidiansystems · Jul 15, 2024 · 808082e · 808082e
1 parent 846869d
commit 808082e
Show file tree

Hide file tree

Showing 11 changed files with 120 additions and 26 deletions.
diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix
@@ -143,6 +143,7 @@
             ''^src/libstore/common-protocol-impl\.hh$''
             ''^src/libstore/common-protocol\.cc$''
             ''^src/libstore/common-protocol\.hh$''
+            ''^src/libstore/common-ssh-store-config\.hh$''
             ''^src/libstore/content-address\.cc$''
             ''^src/libstore/content-address\.hh$''
             ''^src/libstore/daemon\.cc$''
@@ -215,7 +216,6 @@
             ''^src/libstore/serve-protocol\.hh$''
             ''^src/libstore/sqlite\.cc$''
             ''^src/libstore/sqlite\.hh$''
-            ''^src/libstore/ssh-store-config\.hh$''
             ''^src/libstore/ssh-store\.cc$''
             ''^src/libstore/ssh\.cc$''
             ''^src/libstore/ssh\.hh$''

diff --git a/src/libstore/ssh-store-config.cc → src/libstore/common-ssh-store-config.cc b/src/libstore/ssh-store-config.cc → src/libstore/common-ssh-store-config.cc
@@ -1,6 +1,6 @@
 #include <regex>
 
-#include "ssh-store-config.hh"
+#include "common-ssh-store-config.hh"
 #include "ssh.hh"
 
 namespace nix {

diff --git a/src/libstore/ssh-store-config.hh → src/libstore/common-ssh-store-config.hh b/src/libstore/ssh-store-config.hh → src/libstore/common-ssh-store-config.hh
diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
@@ -1,5 +1,5 @@
 #include "legacy-ssh-store.hh"
-#include "ssh-store-config.hh"
+#include "common-ssh-store-config.hh"
 #include "archive.hh"
 #include "pool.hh"
 #include "remote-store.hh"
@@ -15,6 +15,15 @@
 
 namespace nix {
 
+LegacySSHStoreConfig::LegacySSHStoreConfig(
+    std::string_view scheme,
+    std::string_view authority,
+    const Params & params)
+    : StoreConfig(params)
+    , CommonSSHStoreConfig(scheme, authority, params)
+{
+}
+
 std::string LegacySSHStoreConfig::doc()
 {
     return
@@ -35,7 +44,7 @@ LegacySSHStore::LegacySSHStore(
     const Params & params)
     : StoreConfig(params)
     , CommonSSHStoreConfig(scheme, host, params)
-    , LegacySSHStoreConfig(params)
+    , LegacySSHStoreConfig(scheme, host, params)
     , Store(params)
     , connections(make_ref<Pool<Connection>>(
         std::max(1, (int) maxConnections),

diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh
@@ -1,7 +1,7 @@
 #pragma once
 ///@file
 
-#include "ssh-store-config.hh"
+#include "common-ssh-store-config.hh"
 #include "store-api.hh"
 #include "ssh.hh"
 #include "callback.hh"
@@ -13,6 +13,11 @@ struct LegacySSHStoreConfig : virtual CommonSSHStoreConfig
 {
     using CommonSSHStoreConfig::CommonSSHStoreConfig;
 
+    LegacySSHStoreConfig(
+        std::string_view scheme,
+        std::string_view authority,
+        const Params & params);
+
     const Setting<Strings> remoteProgram{this, {"nix-store"}, "remote-program",
         "Path to the `nix-store` executable on the remote machine."};
 

diff --git a/src/libstore/meson.build b/src/libstore/meson.build
@@ -162,6 +162,7 @@ sources = files(
   'builtins/fetchurl.cc',
   'builtins/unpack-channel.cc',
   'common-protocol.cc',
+  'common-ssh-store-config.cc',
   'content-address.cc',
   'daemon.cc',
   'derivations.cc',
@@ -206,7 +207,6 @@ sources = files(
   'serve-protocol-connection.cc',
   'serve-protocol.cc',
   'sqlite.cc',
-  'ssh-store-config.cc',
   'ssh-store.cc',
   'ssh.cc',
   'store-api.cc',
@@ -233,6 +233,7 @@ headers = [config_h] + files(
   'builtins/buildenv.hh',
   'common-protocol-impl.hh',
   'common-protocol.hh',
+  'common-ssh-store-config.hh',
   'content-address.hh',
   'daemon.hh',
   'derivations.hh',
@@ -272,11 +273,11 @@ headers = [config_h] + files(
   'remote-store.hh',
   's3-binary-cache-store.hh',
   's3.hh',
+  'ssh-store.hh',
   'serve-protocol-connection.hh',
   'serve-protocol-impl.hh',
   'serve-protocol.hh',
   'sqlite.hh',
-  'ssh-store-config.hh',
   'ssh.hh',
   'store-api.hh',
   'store-cast.hh',

diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
@@ -1,7 +1,5 @@
-#include "ssh-store-config.hh"
-#include "store-api.hh"
+#include "ssh-store.hh"
 #include "local-fs-store.hh"
-#include "remote-store.hh"
 #include "remote-store-connection.hh"
 #include "source-accessor.hh"
 #include "archive.hh"
@@ -12,23 +10,22 @@
 
 namespace nix {
 
-struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
+SSHStoreConfig::SSHStoreConfig(
+    std::string_view scheme,
+    std::string_view authority,
+    const Params & params)
+    : StoreConfig(params)
+    , RemoteStoreConfig(params)
+    , CommonSSHStoreConfig(scheme, authority, params)
 {
-    using RemoteStoreConfig::RemoteStoreConfig;
-    using CommonSSHStoreConfig::CommonSSHStoreConfig;
-
-    const Setting<Strings> remoteProgram{this, {"nix-daemon"}, "remote-program",
-        "Path to the `nix-daemon` executable on the remote machine."};
-
-    const std::string name() override { return "Experimental SSH Store"; }
+}
 
-    std::string doc() override
-    {
-        return
-          #include "ssh-store.md"
-          ;
-    }
-};
+std::string SSHStoreConfig::doc()
+{
+    return
+      #include "ssh-store.md"
+      ;
+}
 
 class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
 {
@@ -41,7 +38,7 @@ class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
         : StoreConfig(params)
         , RemoteStoreConfig(params)
         , CommonSSHStoreConfig(scheme, host, params)
-        , SSHStoreConfig(params)
+        , SSHStoreConfig(scheme, host, params)
         , Store(params)
         , RemoteStore(params)
         , master(createSSHMaster(

diff --git a/src/libstore/ssh-store.hh b/src/libstore/ssh-store.hh
@@ -0,0 +1,28 @@
+#pragma once
+///@file
+
+#include "common-ssh-store-config.hh"
+#include "store-api.hh"
+#include "remote-store.hh"
+
+namespace nix {
+
+struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
+{
+    using CommonSSHStoreConfig::CommonSSHStoreConfig;
+    using RemoteStoreConfig::RemoteStoreConfig;
+
+    SSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params);
+
+    const Setting<Strings> remoteProgram{
+        this, {"nix-daemon"}, "remote-program", "Path to the `nix-daemon` executable on the remote machine."};
+
+    const std::string name() override
+    {
+        return "Experimental SSH Store";
+    }
+
+    std::string doc() override;
+};
+
+}
diff --git a/tests/unit/libstore/legacy-ssh-store.cc b/tests/unit/libstore/legacy-ssh-store.cc
@@ -0,0 +1,26 @@
+#include <gtest/gtest.h>
+
+#include "legacy-ssh-store.hh"
+
+namespace nix {
+
+TEST(LegacySSHStore, constructConfig)
+{
+    LegacySSHStoreConfig config{
+        "ssh",
+        "localhost",
+        StoreConfig::Params{
+            {
+                "remote-program",
+                // TODO #11106, no more split on space
+                "foo bar",
+            },
+        }};
+    EXPECT_EQ(
+        config.remoteProgram.get(),
+        (Strings{
+            "foo",
+            "bar",
+        }));
+}
+}
diff --git a/tests/unit/libstore/meson.build b/tests/unit/libstore/meson.build
@@ -58,6 +58,7 @@ sources = files(
   'derivation.cc',
   'derived-path.cc',
   'downstream-placeholder.cc',
+  'legacy-ssh-store.cc',
   'machines.cc',
   'nar-info-disk-cache.cc',
   'nar-info.cc',
@@ -67,6 +68,7 @@ sources = files(
   'path.cc',
   'references.cc',
   'serve-protocol.cc',
+  'ssh-store.cc',
   'store-reference.cc',
   'worker-protocol.cc',
 )

diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc
@@ -0,0 +1,26 @@
+#include <gtest/gtest.h>
+
+#include "ssh-store.hh"
+
+namespace nix {
+
+TEST(SSHStore, constructConfig)
+{
+    SSHStoreConfig config{
+        "ssh",
+        "localhost",
+        StoreConfig::Params{
+            {
+                "remote-program",
+                // TODO #11106, no more split on space
+                "foo bar",
+            },
+        }};
+    EXPECT_EQ(
+        config.remoteProgram.get(),
+        (Strings{
+            "foo",
+            "bar",
+        }));
+}
+}