From 426a72c9cf0ae513a1254943dc3efd9d71ebb549 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 28 Sep 2025 16:29:12 +0300 Subject: [PATCH] libstore: Make all StoreConfig::getReference implementations return store parameters These stragglers have been accidentally left out when implementing the StoreConfig::getReference. Also HttpBinaryCacheStore::getReference now returns the actual store parameters, not the cacheUri parameters. --- src/libstore-tests/http-binary-cache-store.cc | 16 +++++++++++++++ src/libstore-tests/uds-remote-store.cc | 20 +++++++++++++++++++ src/libstore/http-binary-cache-store.cc | 2 +- src/libstore/include/nix/store/dummy-store.hh | 1 + src/libstore/s3-binary-cache-store.cc | 1 + src/libstore/uds-remote-store.cc | 6 +++++- 6 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/libstore-tests/http-binary-cache-store.cc b/src/libstore-tests/http-binary-cache-store.cc index 0e3be4ced..4b3754a1f 100644 --- a/src/libstore-tests/http-binary-cache-store.cc +++ b/src/libstore-tests/http-binary-cache-store.cc @@ -18,4 +18,20 @@ TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash) EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b"); } +TEST(HttpBinaryCacheStore, constructConfigWithParams) +{ + StoreConfig::Params params{{"compression", "xz"}}; + HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", params}; + EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b"); + EXPECT_EQ(config.getReference().params, params); +} + +TEST(HttpBinaryCacheStore, constructConfigWithParamsAndUrlWithParams) +{ + StoreConfig::Params params{{"compression", "xz"}}; + HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b?some-param=some-value", params}; + EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b?some-param=some-value"); + EXPECT_EQ(config.getReference().params, params); +} + } // namespace nix diff --git a/src/libstore-tests/uds-remote-store.cc b/src/libstore-tests/uds-remote-store.cc index 11e6b04a3..415dfc4ac 100644 --- a/src/libstore-tests/uds-remote-store.cc +++ b/src/libstore-tests/uds-remote-store.cc @@ -22,4 +22,24 @@ TEST(UDSRemoteStore, constructConfig_to_string) EXPECT_EQ(config.getReference().to_string(), "daemon"); } +TEST(UDSRemoteStore, constructConfigWithParams) +{ + StoreConfig::Params params{{"max-connections", "1"}}; + UDSRemoteStoreConfig config{"unix", "/tmp/socket", params}; + auto storeReference = config.getReference(); + EXPECT_EQ(storeReference.to_string(), "unix:///tmp/socket?max-connections=1"); + EXPECT_EQ(storeReference.render(/*withParams=*/false), "unix:///tmp/socket"); + EXPECT_EQ(storeReference.params, params); +} + +TEST(UDSRemoteStore, constructConfigWithParamsNoPath) +{ + StoreConfig::Params params{{"max-connections", "1"}}; + UDSRemoteStoreConfig config{"unix", "", params}; + auto storeReference = config.getReference(); + EXPECT_EQ(storeReference.to_string(), "daemon?max-connections=1"); + EXPECT_EQ(storeReference.render(/*withParams=*/false), "daemon"); + EXPECT_EQ(storeReference.params, params); +} + } // namespace nix diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 7737389a3..6922c0f69 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -39,7 +39,7 @@ StoreReference HttpBinaryCacheStoreConfig::getReference() const .scheme = cacheUri.scheme, .authority = cacheUri.renderAuthorityAndPath(), }, - .params = cacheUri.query, + .params = getQueryParams(), }; } diff --git a/src/libstore/include/nix/store/dummy-store.hh b/src/libstore/include/nix/store/dummy-store.hh index 4898e8a5b..47e3375cd 100644 --- a/src/libstore/include/nix/store/dummy-store.hh +++ b/src/libstore/include/nix/store/dummy-store.hh @@ -48,6 +48,7 @@ struct DummyStoreConfig : public std::enable_shared_from_this, StoreReference::Specified{ .scheme = *uriSchemes().begin(), }, + .params = getQueryParams(), }; } }; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 4ad09aff2..b70f04be7 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -262,6 +262,7 @@ StoreReference S3BinaryCacheStoreConfig::getReference() const .scheme = *uriSchemes().begin(), .authority = bucketName, }, + .params = getQueryParams(), }; } diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 9725fe8a0..6106a99ce 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -61,13 +61,17 @@ StoreReference UDSRemoteStoreConfig::getReference() const * 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::Daemon{}, + .params = getQueryParams(), + }; return { .variant = StoreReference::Specified{ .scheme = *uriSchemes().begin(), .authority = path, }, + .params = getQueryParams(), }; }