1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-08 19:46:02 +01:00

Rewrite StoreConfig::getUri in terms of new StoreConfig::getReference

Rather than having store implementations return a free-form URI string,
have them return a `StoreReference`. This reflects that fact that this
method is supposed to invert `resolveStoreConfig`, which goes from a
`StoreReference` to some `StoreConfig` concrete derived class (based on
the registry).

`StoreConfig::getUri` is kept only as a convenience for the common case
that we want to immediately render the `StoreReference`.

A few tests were changed to use `local://` not `local`, since
`StoreReference` does not encode the `local` and `daemon` shorthands
(and instead desugars them to `local://` and `unix://` right away). I
think that is fine. `local` and `daemon` still work as input.
This commit is contained in:
John Ericson 2025-08-12 10:50:45 -04:00
parent be3a508b74
commit 3e7879e6df
22 changed files with 181 additions and 60 deletions

View file

@ -8,14 +8,14 @@ TEST(HttpBinaryCacheStore, constructConfig)
{
HttpBinaryCacheStoreConfig config{"http", "foo.bar.baz", {}};
EXPECT_EQ(config.cacheUri, "http://foo.bar.baz");
EXPECT_EQ(config.cacheUri.to_string(), "http://foo.bar.baz");
}
TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash)
{
HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", {}};
EXPECT_EQ(config.cacheUri, "https://foo.bar.baz/a/b");
EXPECT_EQ(config.cacheUri.to_string(), "https://foo.bar.baz/a/b");
}
} // namespace nix

View file

@ -24,7 +24,7 @@ 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);
auto expectedStoreURI = "local?"
auto expectedStoreURI = "local://?"
+ nix::encodeQuery({
{"log", nixLogDir},
{"state", nixStateDir},
@ -104,7 +104,7 @@ TEST_F(nix_api_util_context, nix_store_open_dummy)
nix_libstore_init(ctx);
Store * store = nix_store_open(ctx, "dummy://", nullptr);
ASSERT_EQ(NIX_OK, ctx->last_err_code);
ASSERT_STREQ("dummy", store->ptr->config.getUri().c_str());
ASSERT_STREQ("dummy://", store->ptr->config.getUri().c_str());
std::string str;
nix_store_get_version(ctx, store, OBSERVE_STRING(str));

View file

@ -33,9 +33,14 @@ struct DummyStoreConfig : public std::enable_shared_from_this<DummyStoreConfig>,
ref<Store> openStore() const override;
std::string getUri() const override
StoreReference getReference() const override
{
return *uriSchemes().begin();
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
},
};
}
};

View file

@ -22,13 +22,25 @@ HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig(
std::string_view scheme, std::string_view _cacheUri, const Params & params)
: StoreConfig(params)
, BinaryCacheStoreConfig(params)
, cacheUri(
, cacheUri(parseURL(
std::string{scheme} + "://"
+ (!_cacheUri.empty() ? _cacheUri
: throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme)))
: throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))))
{
while (!cacheUri.empty() && cacheUri.back() == '/')
cacheUri.pop_back();
while (!cacheUri.path.empty() && cacheUri.path.back() == '/')
cacheUri.path.pop_back();
}
StoreReference HttpBinaryCacheStoreConfig::getReference() const
{
return {
.variant =
StoreReference::Specified{
.scheme = cacheUri.scheme,
.authority = (cacheUri.authority ? cacheUri.authority->to_string() : "") + cacheUri.path,
},
.params = cacheUri.query,
};
}
std::string HttpBinaryCacheStoreConfig::doc()
@ -65,16 +77,17 @@ public:
void init() override
{
// FIXME: do this lazily?
if (auto cacheInfo = diskCache->upToDateCacheExists(config->cacheUri)) {
if (auto cacheInfo = diskCache->upToDateCacheExists(config->cacheUri.to_string())) {
config->wantMassQuery.setDefault(cacheInfo->wantMassQuery);
config->priority.setDefault(cacheInfo->priority);
} else {
try {
BinaryCacheStore::init();
} catch (UploadToHTTP &) {
throw Error("'%s' does not appear to be a binary cache", config->cacheUri);
throw Error("'%s' does not appear to be a binary cache", config->cacheUri.to_string());
}
diskCache->createCache(config->cacheUri, config->storeDir, config->wantMassQuery, config->priority);
diskCache->createCache(
config->cacheUri.to_string(), config->storeDir, config->wantMassQuery, config->priority);
}
}
@ -134,16 +147,29 @@ protected:
try {
getFileTransfer()->upload(req);
} catch (FileTransferError & e) {
throw UploadToHTTP("while uploading to HTTP binary cache at '%s': %s", config->cacheUri, e.msg());
throw UploadToHTTP(
"while uploading to HTTP binary cache at '%s': %s", config->cacheUri.to_string(), e.msg());
}
}
FileTransferRequest makeRequest(const std::string & path)
{
/* FIXME path is not a path, but a full relative or absolute
URL, e.g. we've seen in the wild NARINFO files have a URL
field which is
`nar/15f99rdaf26k39knmzry4xd0d97wp6yfpnfk1z9avakis7ipb9yg.nar?hash=zphkqn2wg8mnvbkixnl2aadkbn0rcnfj`
(note the query param) and that gets passed here.
What should actually happen is that we have two parsed URLs
(if we support relative URLs), and then we combined them with
a URL `operator/` which would be like
`std::filesystem::path`'s equivalent operator, which properly
combines the the URLs, whether the right is relative or
absolute. */
return FileTransferRequest(
hasPrefix(path, "https://") || hasPrefix(path, "http://") || hasPrefix(path, "file://")
? path
: config->cacheUri + "/" + path);
: config->cacheUri.to_string() + "/" + path);
}
void getFile(const std::string & path, Sink & sink) override

View file

@ -1,3 +1,4 @@
#include "nix/util/url.hh"
#include "nix/store/binary-cache-store.hh"
namespace nix {
@ -11,7 +12,7 @@ struct HttpBinaryCacheStoreConfig : std::enable_shared_from_this<HttpBinaryCache
HttpBinaryCacheStoreConfig(
std::string_view scheme, std::string_view cacheUri, const Store::Config::Params & params);
Path cacheUri;
ParsedURL cacheUri;
static const std::string name()
{
@ -24,10 +25,7 @@ struct HttpBinaryCacheStoreConfig : std::enable_shared_from_this<HttpBinaryCache
ref<Store> openStore() const override;
std::string getUri() const override
{
return cacheUri;
}
StoreReference getReference() const override;
};
} // namespace nix

View file

@ -54,7 +54,7 @@ struct LegacySSHStoreConfig : std::enable_shared_from_this<LegacySSHStoreConfig>
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
struct LegacySSHStore : public virtual Store

View file

@ -27,7 +27,7 @@ struct LocalBinaryCacheStoreConfig : std::enable_shared_from_this<LocalBinaryCac
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
} // namespace nix

View file

@ -88,10 +88,7 @@ struct LocalOverlayStoreConfig : virtual LocalStoreConfig
ref<Store> openStore() const override;
std::string getUri() const override
{
return "local-overlay://";
}
StoreReference getReference() const override;
protected:
/**

View file

@ -112,7 +112,7 @@ struct LocalStoreConfig : std::enable_shared_from_this<LocalStoreConfig>,
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
class LocalStore : public virtual IndirectRootStore, public virtual GcStore

View file

@ -107,7 +107,7 @@ public:
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
struct S3BinaryCacheStore : virtual BinaryCacheStore

View file

@ -34,7 +34,7 @@ struct SSHStoreConfig : std::enable_shared_from_this<SSHStoreConfig>,
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig

View file

@ -199,11 +199,16 @@ struct StoreConfig : public StoreDirConfig
virtual ref<Store> openStore() const = 0;
/**
* Render the config back to a "store URL". It should round-trip
* Render the config back to a `StoreReference`. It should round-trip
* with `resolveStoreConfig` (for stores configs that are
* registered).
*/
virtual std::string getUri() const;
virtual StoreReference getReference() const;
std::string getUri() const
{
return getReference().render();
}
};
/**

View file

@ -45,7 +45,7 @@ struct UDSRemoteStoreConfig : std::enable_shared_from_this<UDSRemoteStoreConfig>
ref<Store> openStore() const override;
std::string getUri() const override;
StoreReference getReference() const override;
};
struct UDSRemoteStore : virtual IndirectRootStore, virtual RemoteStore

View file

@ -88,9 +88,16 @@ ref<LegacySSHStore::Connection> LegacySSHStore::openConnection()
return conn;
};
std::string LegacySSHStoreConfig::getUri() const
StoreReference LegacySSHStoreConfig::getReference() const
{
return ParsedURL{.scheme = *uriSchemes().begin(), .authority = authority, .query = getQueryParams()}.to_string();
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
.authority = authority.to_string(),
},
.params = getQueryParams(),
};
}
std::map<StorePath, UnkeyedValidPathInfo> LegacySSHStore::queryPathInfosUncached(const StorePathSet & paths)

View file

@ -23,9 +23,15 @@ std::string LocalBinaryCacheStoreConfig::doc()
;
}
std::string LocalBinaryCacheStoreConfig::getUri() const
StoreReference LocalBinaryCacheStoreConfig::getReference() const
{
return "file://" + binaryCacheDir;
return {
.variant =
StoreReference::Specified{
.scheme = "file",
.authority = binaryCacheDir,
},
};
}
struct LocalBinaryCacheStore : virtual BinaryCacheStore

View file

@ -23,6 +23,16 @@ ref<Store> LocalOverlayStoreConfig::openStore() const
ref{std::dynamic_pointer_cast<const LocalOverlayStoreConfig>(shared_from_this())});
}
StoreReference LocalOverlayStoreConfig::getReference() const
{
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
},
};
}
Path LocalOverlayStoreConfig::toUpperPath(const StorePath & path) const
{
return upperLayer + "/" + path.to_string();

View file

@ -439,15 +439,15 @@ LocalStore::~LocalStore()
}
}
std::string LocalStoreConfig::getUri() const
StoreReference LocalStoreConfig::getReference() const
{
std::ostringstream oss;
oss << *uriSchemes().begin();
auto queryParams = getQueryParams();
if (!queryParams.empty())
oss << "?";
oss << encodeQuery(queryParams);
return std::move(oss).str();
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
},
.params = getQueryParams(),
};
}
int LocalStore::getSchema()

View file

@ -254,9 +254,15 @@ std::string S3BinaryCacheStoreConfig::doc()
;
}
std::string S3BinaryCacheStoreConfig::getUri() const
StoreReference S3BinaryCacheStoreConfig::getReference() const
{
return "s3://" + bucketName;
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
.authority = bucketName,
},
};
}
struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore

View file

@ -25,9 +25,16 @@ std::string SSHStoreConfig::doc()
;
}
std::string SSHStoreConfig::getUri() const
StoreReference SSHStoreConfig::getReference() const
{
return ParsedURL{.scheme = *uriSchemes().begin(), .authority = authority, .query = getQueryParams()}.to_string();
return {
.variant =
StoreReference::Specified{
.scheme = *uriSchemes().begin(),
.authority = authority.to_string(),
},
.params = getQueryParams(),
};
}
struct SSHStore : virtual RemoteStore

View file

@ -300,9 +300,9 @@ Store::Store(const Store::Config & config)
assertLibStoreInitialized();
}
std::string StoreConfig::getUri() const
StoreReference StoreConfig::getReference() const
{
return "";
return {.variant = StoreReference::Auto{}};
}
bool Store::PathInfoCacheValue::isKnownNow()

View file

@ -54,14 +54,19 @@ UDSRemoteStore::UDSRemoteStore(ref<const Config> config)
{
}
std::string UDSRemoteStoreConfig::getUri() const
StoreReference UDSRemoteStoreConfig::getReference() const
{
return path == settings.nixDaemonSocketFile ? // FIXME: Not clear why we return daemon here and not default
// to settings.nixDaemonSocketFile
//
// unix:// with no path also works. Change what we return?
"daemon"
: std::string(*uriSchemes().begin()) + "://" + path;
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,
},
};
}
void UDSRemoteStore::Connection::closeWrite()

View file

@ -2,12 +2,61 @@
source common.sh
# Different versions of the Nix daemon normalize or don't normalize
# store URLs, plus NIX_REMOTE (per the test suite) might not be using on
# store URL in normal form, so the easiest thing to do is normalize URLs
# after the fact before comparing them for equality.
normalize_nix_store_url () {
local url="$1"
case "$url" in
'auto' )
# Need to actually ask Nix in this case
echo "$defaultStore"
;;
'local://'* )
# To not be captured by next pattern
echo "$url"
;;
local | 'local?'* )
echo "local://${url#local}"
;;
daemon | 'daemon?'* )
echo "unix://${url#daemon}"
;;
* )
echo "$url"
;;
esac
}
STORE_INFO=$(nix store info 2>&1)
LEGACY_STORE_INFO=$(nix store ping 2>&1) # alias to nix store info
STORE_INFO_JSON=$(nix store info --json)
echo "$STORE_INFO" | grep "Store URL: ${NIX_REMOTE}"
echo "$LEGACY_STORE_INFO" | grep "Store URL: ${NIX_REMOTE}"
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?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 "daemon?x=y")" = "unix://?x=y" ]]
# otherwise unchanged
[[ "$(normalize_nix_store_url "https://site")" = "https://site" ]]
nixRemoteOrDefault=$(normalize_nix_store_url "${NIX_REMOTE:-"auto"}")
check_human_readable () {
[[ "$(normalize_nix_store_url "$(echo "$1" | grep 'Store URL:' | sed 's^Store URL: ^^')")" = "${nixRemoteOrDefault}" ]]
}
check_human_readable "$STORE_INFO"
check_human_readable "$LEGACY_STORE_INFO"
if [[ -v NIX_DAEMON_PACKAGE ]] && isDaemonNewer "2.7.0pre20220126"; then
DAEMON_VERSION=$("$NIX_DAEMON_PACKAGE"/bin/nix daemon --version | cut -d' ' -f3)
@ -21,4 +70,4 @@ expect 127 NIX_REMOTE=unix:"$PWD"/store nix store info || \
TODO_NixOS
[[ "$(echo "$STORE_INFO_JSON" | jq -r ".url")" == "${NIX_REMOTE:-local}" ]]
[[ "$(normalize_nix_store_url "$(echo "$STORE_INFO_JSON" | jq -r ".url")")" == "${nixRemoteOrDefault}" ]]