Merge pull request #13911 from xokdvium/store-uri-daemon-local

libstore: Do not normalize daemon -> unix://, local -> local://

src/libstore-tests/data/store-reference/daemon_shorthand.txt

@@ -0,0 +1 @@
+daemon

src/libstore-tests/data/store-reference/local_shorthand_3.txt

@@ -0,0 +1 @@
+local

src/libstore-tests/local-store.cc

@@ -33,4 +33,10 @@ TEST(LocalStore, constructConfig_rootPath)
     EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
 }
 
+TEST(LocalStore, constructConfig_to_string)
+{
+    LocalStoreConfig config{"local", "", {}};
+    EXPECT_EQ(config.getReference().to_string(), "local");
+}
+
 } // namespace nix

src/libstore-tests/store-reference.cc

@@ -107,6 +107,13 @@ URI_TEST_READ(local_shorthand_1, localExample_1)
 
 URI_TEST_READ(local_shorthand_2, localExample_2)
 
+URI_TEST(
+    local_shorthand_3,
+    (StoreReference{
+        .variant = StoreReference::Local{},
+        .params = {},
+    }))
+
 static StoreReference unixExample{
     .variant =
         StoreReference::Specified{
@@ -134,4 +141,11 @@ URI_TEST(
         .params = {},
     }))
 
+URI_TEST(
+    daemon_shorthand,
+    (StoreReference{
+        .variant = StoreReference::Daemon{},
+        .params = {},
+    }))
+
 } // namespace nix

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

@@ -16,4 +16,10 @@ TEST(UDSRemoteStore, constructConfigWrongScheme)
     EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError);
 }
 
+TEST(UDSRemoteStore, constructConfig_to_string)
+{
+    UDSRemoteStoreConfig config{"unix", "", {}};
+    EXPECT_EQ(config.getReference().to_string(), "daemon");
+}
+
 } // namespace nix

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

@@ -64,7 +64,29 @@ struct StoreReference
         auto operator<=>(const Specified & rhs) const = default;
     };
 
-    typedef std::variant<Auto, Specified> Variant;
+    /**
+     * Special case for `daemon` to avoid normalization.
+     */
+    struct Daemon : Specified
+    {
+        Daemon()
+            : Specified({.scheme = "unix"})
+        {
+        }
+    };
+
+    /**
+     * Special case for `local` to avoid normalization.
+     */
+    struct Local : Specified
+    {
+        Local()
+            : Specified({.scheme = "local"})
+        {
+        }
+    };
+
+    typedef std::variant<Auto, Specified, Daemon, Local> Variant;
 
     Variant variant;
 

src/libstore/local-store.cc

@@ -456,12 +456,17 @@ LocalStore::~LocalStore()
 
 StoreReference LocalStoreConfig::getReference() const
 {
+    auto params = getQueryParams();
+    /* Back-compatibility kludge. Tools like nix-output-monitor expect 'local'
+       and can't parse 'local://'. */
+    if (params.empty())
+        return {.variant = StoreReference::Local{}};
     return {
         .variant =
             StoreReference::Specified{
                 .scheme = *uriSchemes().begin(),
             },
-        .params = getQueryParams(),
+        .params = std::move(params),
     };
 }
 

src/libstore/store-api.cc

@@ -818,7 +818,13 @@ makeCopyPathMessage(const StoreConfig & srcCfg, const StoreConfig & dstCfg, std:
 
     auto isShorthand = [](const StoreReference & ref) {
         /* At this point StoreReference **must** be resolved. */
-        const auto & specified = std::get<StoreReference::Specified>(ref.variant);
+        const auto & specified = std::visit(
+            overloaded{
+                [](const StoreReference::Auto &) -> const StoreReference::Specified & { unreachable(); },
+                [](const StoreReference::Specified & specified) -> const StoreReference::Specified & {
+                    return specified;
+                }},
+            ref.variant);
         const auto & scheme = specified.scheme;
         return (scheme == "local" || scheme == "unix") && specified.authority.empty();
     };

src/libstore/store-reference.cc

@@ -25,6 +25,8 @@ std::string StoreReference::render(bool withParams) const
     std::visit(
         overloaded{
             [&](const StoreReference::Auto &) { res = "auto"; },
+            [&](const StoreReference::Daemon &) { res = "daemon"; },
+            [&](const StoreReference::Local &) { res = "local"; },
             [&](const StoreReference::Specified & g) {
                 res = g.scheme;
                 res += "://";
@@ -66,21 +68,17 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen
                 .params = std::move(params),
             };
         } else if (baseURI == "daemon") {
+            if (params.empty())
+                return {.variant = Daemon{}};
             return {
-                .variant =
+                .variant = Specified{.scheme = "unix", .authority = ""},
-                    Specified{
-                        .scheme = "unix",
-                        .authority = "",
-                    },
                 .params = std::move(params),
             };
         } else if (baseURI == "local") {
+            if (params.empty())
+                return {.variant = Local{}};
             return {
-                .variant =
+                .variant = Specified{.scheme = "local", .authority = ""},
-                    Specified{
-                        .scheme = "local",
-                        .authority = "",
-                    },
                 .params = std::move(params),
             };
         } else if (isNonUriPath(baseURI)) {

src/libstore/uds-remote-store.cc

@@ -57,15 +57,16 @@ UDSRemoteStore::UDSRemoteStore(ref<const Config> config)
 
 StoreReference UDSRemoteStoreConfig::getReference() const
 {
+    /* We specifically return "daemon" here instead of "unix://" or "unix://${path}"
+     * to be more compatible with older versions of nix. Some tooling out there
+     * tries hard to parse store references and it might not be able to handle "unix://". */
+    if (path == settings.nixDaemonSocketFile)
+        return {.variant = StoreReference::Daemon{}};
     return {
         .variant =
             StoreReference::Specified{
                 .scheme = *uriSchemes().begin(),
-                // We return the empty string when the path looks like the
+                .authority = path,
-                // default path, but we could also just return the path
-                // verbatim always, to be robust to overall config changes
-                // at the cost of some verbosity.
-                .authority = path == settings.nixDaemonSocketFile ? "" : path,
             },
     };
 }

tests/functional/store-info.sh

@@ -13,14 +13,20 @@ normalize_nix_store_url () {
             # Need to actually ask Nix in this case
             echo "$defaultStore"
             ;;
+        local | 'local://' )
+            echo 'local'
+            ;;
+        daemon | 'unix://' )
+            echo 'daemon'
+            ;;
         'local://'* )
             # To not be captured by next pattern
             echo "$url"
             ;;
-        local | 'local?'* )
+        'local?'* )
             echo "local://${url#local}"
             ;;
-        daemon | 'daemon?'* )
+        'daemon?'* )
             echo "unix://${url#daemon}"
             ;;
         * )
@@ -38,13 +44,13 @@ defaultStore="$(normalize_nix_store_url "$(echo "$STORE_INFO_JSON" | jq -r ".url
 # Test cases for `normalize_nix_store_url` itself
 
 # Normalize local store
-[[ "$(normalize_nix_store_url "local://")" = "local://" ]]
+[[ "$(normalize_nix_store_url "local://")" = "local" ]]
-[[ "$(normalize_nix_store_url "local")" = "local://" ]]
+[[ "$(normalize_nix_store_url "local")" = "local" ]]
 [[ "$(normalize_nix_store_url "local?foo=bar")" = "local://?foo=bar" ]]
 
 # Normalize unix domain socket remote store
-[[ "$(normalize_nix_store_url "unix://")" = "unix://" ]]
+[[ "$(normalize_nix_store_url "unix://")" = "daemon" ]]
-[[ "$(normalize_nix_store_url "daemon")" = "unix://" ]]
+[[ "$(normalize_nix_store_url "daemon")" = "daemon" ]]
 [[ "$(normalize_nix_store_url "daemon?x=y")" = "unix://?x=y" ]]
 
 # otherwise unchanged