From bbbb4ce330942723a0ee8731c5ce7ec14e3c5269 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Fri, 5 Sep 2025 02:56:28 +0300 Subject: [PATCH] libstore: Do not normalize daemon -> unix://, local -> local:// This is relied upon (specifically the `local` store) by existing tooling [1] and we broke this in 3e7879e6dfb75d5c39058b8c2fd6619db8df9b95 (which was first released in 2.31). To lessen the scope of the breakage we should not normalize "auto" references and explicitly specified references like "local" or "daemon". It also makes sense to canonicalize local://,daemon:// to be more compatible with prior behavior. [1]: https://github.com/maralorn/nix-output-monitor/blob/05e1b3cba2fa328a1781390a4e4515e9c432229e/lib/NOM/Builds.hs#L60-L64 (cherry picked from commit 3513ab13dc45f9025cebc6f8f694a2963d44556a) --- .../data/store-reference/daemon_shorthand.txt | 1 + .../store-reference/local_shorthand_3.txt | 1 + src/libstore-tests/local-store.cc | 6 +++++ src/libstore-tests/store-reference.cc | 14 +++++++++++ src/libstore-tests/uds-remote-store.cc | 6 +++++ .../include/nix/store/store-reference.hh | 24 ++++++++++++++++++- src/libstore/local-store.cc | 7 +++++- src/libstore/store-api.cc | 8 ++++++- src/libstore/store-reference.cc | 18 +++++++------- src/libstore/uds-remote-store.cc | 11 +++++---- tests/functional/store-info.sh | 18 +++++++++----- 11 files changed, 90 insertions(+), 24 deletions(-) create mode 100644 src/libstore-tests/data/store-reference/daemon_shorthand.txt create mode 100644 src/libstore-tests/data/store-reference/local_shorthand_3.txt diff --git a/src/libstore-tests/data/store-reference/daemon_shorthand.txt b/src/libstore-tests/data/store-reference/daemon_shorthand.txt new file mode 100644 index 000000000..bd8c0f8c4 --- /dev/null +++ b/src/libstore-tests/data/store-reference/daemon_shorthand.txt @@ -0,0 +1 @@ +daemon \ No newline at end of file diff --git a/src/libstore-tests/data/store-reference/local_shorthand_3.txt b/src/libstore-tests/data/store-reference/local_shorthand_3.txt new file mode 100644 index 000000000..c2c027fec --- /dev/null +++ b/src/libstore-tests/data/store-reference/local_shorthand_3.txt @@ -0,0 +1 @@ +local \ No newline at end of file diff --git a/src/libstore-tests/local-store.cc b/src/libstore-tests/local-store.cc index cdbc29b03..0b11b7baf 100644 --- a/src/libstore-tests/local-store.cc +++ b/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().render(), "local"); +} + } // namespace nix diff --git a/src/libstore-tests/store-reference.cc b/src/libstore-tests/store-reference.cc index 01b75f3d2..d9f040ab6 100644 --- a/src/libstore-tests/store-reference.cc +++ b/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 diff --git a/src/libstore-tests/uds-remote-store.cc b/src/libstore-tests/uds-remote-store.cc index c215d6e18..b22feeefe 100644 --- a/src/libstore-tests/uds-remote-store.cc +++ b/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().render(), "daemon"); +} + } // namespace nix diff --git a/src/libstore/include/nix/store/store-reference.hh b/src/libstore/include/nix/store/store-reference.hh index 5cf1e9a11..f0dc48d69 100644 --- a/src/libstore/include/nix/store/store-reference.hh +++ b/src/libstore/include/nix/store/store-reference.hh @@ -64,7 +64,29 @@ struct StoreReference auto operator<=>(const Specified & rhs) const = default; }; - typedef std::variant 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 Variant; Variant variant; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index a66a97866..cc1303ae5 100644 --- a/src/libstore/local-store.cc +++ b/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), }; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index fad79a83e..230ddf77b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -817,7 +817,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(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(); }; diff --git a/src/libstore/store-reference.cc b/src/libstore/store-reference.cc index adc60b391..760d77d56 100644 --- a/src/libstore/store-reference.cc +++ b/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 += "://"; @@ -68,21 +70,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 = - Specified{ - .scheme = "unix", - .authority = "", - }, + .variant = Specified{.scheme = "unix", .authority = ""}, .params = std::move(params), }; } else if (baseURI == "local") { + if (params.empty()) + return {.variant = Local{}}; return { - .variant = - Specified{ - .scheme = "local", - .authority = "", - }, + .variant = Specified{.scheme = "local", .authority = ""}, .params = std::move(params), }; } else if (isNonUriPath(baseURI)) { diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 4871b4913..9725fe8a0 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -57,15 +57,16 @@ UDSRemoteStore::UDSRemoteStore(ref 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 - // 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, + .authority = path, }, }; } diff --git a/tests/functional/store-info.sh b/tests/functional/store-info.sh index 7c9257215..adaee5dfe 100755 --- a/tests/functional/store-info.sh +++ b/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 "daemon")" = "unix://" ]] +[[ "$(normalize_nix_store_url "unix://")" = "daemon" ]] +[[ "$(normalize_nix_store_url "daemon")" = "daemon" ]] [[ "$(normalize_nix_store_url "daemon?x=y")" = "unix://?x=y" ]] # otherwise unchanged