From 3513ab13dc45f9025cebc6f8f694a2963d44556a 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 --- .../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..d00888897 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().to_string(), "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..11e6b04a3 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().to_string(), "daemon"); +} + } // namespace nix diff --git a/src/libstore/include/nix/store/store-reference.hh b/src/libstore/include/nix/store/store-reference.hh index 1df333947..dc34500d9 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 7872d4f93..112d5b14c 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 d96be5965..78d2bbd54 100644 --- a/src/libstore/store-api.cc +++ b/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(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 8b4c19600..2c54e497e 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 += "://"; @@ -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 = - 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