Merge pull request #13734 from xokdvium/get-uri-correct

libstore: Correct getUri methods for all stores

src/libexpr/eval-profiler-settings.cc

@@ -1,6 +1,5 @@
 #include "nix/expr/eval-profiler-settings.hh"
 #include "nix/util/configuration.hh"
-#include "nix/util/logging.hh" /* Needs to be included before config-impl.hh */
 #include "nix/util/config-impl.hh"
 #include "nix/util/abstract-setting-to-json.hh"
 

src/libstore-tests/legacy-ssh-store.cc

@@ -6,21 +6,27 @@ namespace nix {
 
 TEST(LegacySSHStore, constructConfig)
 {
-    LegacySSHStoreConfig config{
+    initLibStore(/*loadConfig=*/false);
+
+    auto config = make_ref<LegacySSHStoreConfig>(
         "ssh",
-        "localhost",
+        "me@localhost:2222",
         StoreConfig::Params{
             {
                 "remote-program",
                 // TODO #11106, no more split on space
                 "foo bar",
             },
-        }};
+        });
+
     EXPECT_EQ(
-        config.remoteProgram.get(),
+        config->remoteProgram.get(),
         (Strings{
             "foo",
             "bar",
         }));
+
+    auto store = config->openStore();
+    EXPECT_EQ(store->getUri(), "ssh://me@localhost:2222?remote-program=foo%20bar");
 }
 } // namespace nix

src/libstore-tests/local-overlay-store.cc

@@ -1,9 +1,6 @@
-// FIXME: Odd failures for templates that are causing the PR to break
+#include <gtest/gtest.h>
-// for now with discussion with @Ericson2314 to comment out.
-#if 0
-#  include <gtest/gtest.h>
 
-#  include "nix/store/local-overlay-store.hh"
+#include "nix/store/local-overlay-store.hh"
 
 namespace nix {
 
@@ -31,4 +28,3 @@ TEST(LocalOverlayStore, constructConfig_rootPath)
 }
 
 } // namespace nix
-#endif

src/libstore-tests/local-store.cc

@@ -1,15 +1,12 @@
-// FIXME: Odd failures for templates that are causing the PR to break
+#include <gtest/gtest.h>
-// for now with discussion with @Ericson2314 to comment out.
-#if 0
-#  include <gtest/gtest.h>
 
-#  include "nix/store/local-store.hh"
+#include "nix/store/local-store.hh"
 
 // Needed for template specialisations. This is not good! When we
 // overhaul how store configs work, this should be fixed.
-#  include "nix/util/args.hh"
+#include "nix/util/args.hh"
-#  include "nix/util/config-impl.hh"
+#include "nix/util/config-impl.hh"
-#  include "nix/util/abstract-setting-to-json.hh"
+#include "nix/util/abstract-setting-to-json.hh"
 
 namespace nix {
 
@@ -37,4 +34,3 @@ TEST(LocalStore, constructConfig_rootPath)
 }
 
 } // namespace nix
-#endif

src/libstore-tests/nix_api_store.cc

@@ -5,6 +5,7 @@
 
 #include "nix/store/tests/nix_api_store.hh"
 #include "nix/util/tests/string_callback.hh"
+#include "nix/util/url.hh"
 
 #include "store-tests-config.hh"
 
@@ -23,7 +24,13 @@ TEST_F(nix_api_store_test, nix_store_get_uri)
     std::string str;
     auto ret = nix_store_get_uri(ctx, store, OBSERVE_STRING(str));
     ASSERT_EQ(NIX_OK, ret);
-    ASSERT_STREQ("local", str.c_str());
+    auto expectedStoreURI = "local?"
+                            + nix::encodeQuery({
+                                {"log", nixLogDir},
+                                {"state", nixStateDir},
+                                {"store", nixStoreDir},
+                            });
+    ASSERT_EQ(expectedStoreURI, str);
 }
 
 TEST_F(nix_api_util_context, nix_store_get_storedir_default)

src/libstore-tests/ssh-store.cc

@@ -1,32 +1,38 @@
-// FIXME: Odd failures for templates that are causing the PR to break
+#include <gtest/gtest.h>
-// for now with discussion with @Ericson2314 to comment out.
-#if 0
-#  include <gtest/gtest.h>
 
-#  include "nix/store/ssh-store.hh"
+#include "nix/store/ssh-store.hh"
+#include "nix/util/config-impl.hh"
+#include "nix/util/abstract-setting-to-json.hh"
 
 namespace nix {
 
 TEST(SSHStore, constructConfig)
 {
-    SSHStoreConfig config{
+    initLibStore(/*loadConfig=*/false);
-        "ssh",
+
-        "localhost",
+    auto config = make_ref<SSHStoreConfig>(
+        "ssh-ng",
+        "me@localhost:2222",
         StoreConfig::Params{
             {
                 "remote-program",
                 // TODO #11106, no more split on space
                 "foo bar",
             },
-        },
+        });
-    };
 
     EXPECT_EQ(
-        config.remoteProgram.get(),
+        config->remoteProgram.get(),
         (Strings{
             "foo",
             "bar",
         }));
+
+    auto store = config->openStore();
+    EXPECT_EQ(store->getUri(), "ssh-ng://me@localhost:2222?remote-program=foo%20bar");
+    config->resetOverridden();
+    store = config->openStore();
+    EXPECT_EQ(store->getUri(), "ssh-ng://me@localhost:2222");
 }
 
 TEST(MountedSSHStore, constructConfig)
@@ -51,5 +57,4 @@ TEST(MountedSSHStore, constructConfig)
         }));
 }
 
-}
+} // namespace nix
-#endif

src/libstore-tests/uds-remote-store.cc

@@ -1,9 +1,6 @@
-// FIXME: Odd failures for templates that are causing the PR to break
+#include <gtest/gtest.h>
-// for now with discussion with @Ericson2314 to comment out.
-#if 0
-#  include <gtest/gtest.h>
 
-#  include "nix/store/uds-remote-store.hh"
+#include "nix/store/uds-remote-store.hh"
 
 namespace nix {
 
@@ -20,4 +17,3 @@ TEST(UDSRemoteStore, constructConfigWrongScheme)
 }
 
 } // namespace nix
-#endif

src/libstore/include/nix/store/globals.hh

@@ -18,6 +18,11 @@ namespace nix {
 
 typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode;
 
+template<>
+SandboxMode BaseSetting<SandboxMode>::parse(const std::string & str) const;
+template<>
+std::string BaseSetting<SandboxMode>::to_string() const;
+
 struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
 {
     MaxBuildJobsSetting(

src/libstore/include/nix/store/store-api.hh

@@ -126,6 +126,19 @@ struct StoreConfig : public StoreDirConfig
         return "";
     }
 
+    /**
+     * Get overridden store reference query parameters.
+     */
+    StringMap getQueryParams() const
+    {
+        auto queryParams = std::map<std::string, AbstractConfig::SettingInfo>{};
+        getSettings(queryParams, /*overriddenOnly=*/true);
+        StringMap res;
+        for (const auto & [name, info] : queryParams)
+            res.insert({name, info.value});
+        return res;
+    }
+
     /**
      * An experimental feature this type store is gated, if it is to be
      * experimental.

src/libstore/legacy-ssh-store.cc

@@ -90,7 +90,9 @@ ref<LegacySSHStore::Connection> LegacySSHStore::openConnection()
 
 std::string LegacySSHStore::getUri()
 {
-    return *Config::uriSchemes().begin() + "://" + config->authority.to_string();
+    return ParsedURL{
+        .scheme = *Config::uriSchemes().begin(), .authority = config->authority, .query = config->getQueryParams()}
+        .to_string();
 }
 
 std::map<StorePath, UnkeyedValidPathInfo> LegacySSHStore::queryPathInfosUncached(const StorePathSet & paths)

src/libstore/local-store.cc

@@ -16,6 +16,7 @@
 #include "nix/store/posix-fs-canonicalise.hh"
 #include "nix/util/posix-source-accessor.hh"
 #include "nix/store/keys.hh"
+#include "nix/util/url.hh"
 #include "nix/util/users.hh"
 #include "nix/store/store-open.hh"
 #include "nix/store/store-registration.hh"
@@ -440,7 +441,13 @@ LocalStore::~LocalStore()
 
 std::string LocalStore::getUri()
 {
-    return "local";
+    std::ostringstream oss;
+    oss << *config->uriSchemes().begin();
+    auto queryParams = config->getQueryParams();
+    if (!queryParams.empty())
+        oss << "?";
+    oss << encodeQuery(queryParams);
+    return std::move(oss).str();
 }
 
 int LocalStore::getSchema()

src/libstore/ssh-store.cc

@@ -43,7 +43,9 @@ struct SSHStore : virtual RemoteStore
 
     std::string getUri() override
     {
-        return *Config::uriSchemes().begin() + "://" + host;
+        return ParsedURL{
+            .scheme = *Config::uriSchemes().begin(), .authority = config->authority, .query = config->getQueryParams()}
+            .to_string();
     }
 
     // FIXME extend daemon protocol, move implementation to RemoteStore
@@ -66,8 +68,6 @@ protected:
 
     ref<RemoteStore::Connection> openConnection() override;
 
-    std::string host;
-
     std::vector<std::string> extraRemoteProgramArgs;
 
     SSHMaster master;

src/libutil/config-global.cc

@@ -15,7 +15,7 @@ bool GlobalConfig::set(const std::string & name, const std::string & value)
     return false;
 }
 
-void GlobalConfig::getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly)
+void GlobalConfig::getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly) const
 {
     for (auto & config : configRegistrations())
         config->getSettings(res, overriddenOnly);

src/libutil/configuration.cc

@@ -85,7 +85,7 @@ void AbstractConfig::reapplyUnknownSettings()
         set(s.first, s.second);
 }
 
-void Config::getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly)
+void Config::getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly) const
 {
     for (const auto & opt : _settings)
         if (!opt.second.isAlias && (!overriddenOnly || opt.second.setting->overridden)

src/libutil/include/nix/util/config-global.hh

@@ -17,7 +17,7 @@ struct GlobalConfig : public AbstractConfig
 
     bool set(const std::string & name, const std::string & value) override;
 
-    void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) override;
+    void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) const override;
 
     void resetOverridden() override;
 

src/libutil/include/nix/util/config-impl.hh

@@ -12,8 +12,10 @@
  * instantiation.
  */
 
+#include "nix/util/util.hh"
 #include "nix/util/configuration.hh"
 #include "nix/util/args.hh"
+#include "nix/util/logging.hh"
 
 namespace nix {
 

src/libutil/include/nix/util/configuration.hh

@@ -73,7 +73,7 @@ public:
      * - res: map to store settings in
      * - overriddenOnly: when set to true only overridden settings will be added to `res`
      */
-    virtual void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) = 0;
+    virtual void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) const = 0;
 
     /**
      * Parses the configuration in `contents` and applies it
@@ -160,7 +160,7 @@ public:
 
     void addSetting(AbstractSetting * setting);
 
-    void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) override;
+    void getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly = false) const override;
 
     void resetOverridden() override;