1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-09 12:06:01 +01:00

Merge pull request #13755 from xokdvium/concise-uri-logs

treewide: Remove getUri and replace with getHumanReadableURI where appropriate
This commit is contained in:
John Ericson 2025-08-14 15:39:09 -04:00 committed by GitHub
commit 4b4895e750
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 118 additions and 69 deletions

View file

@ -574,14 +574,15 @@ ProcessLineResult NixRepl::processLine(std::string line)
for (auto & sub : subs) {
auto * logSubP = dynamic_cast<LogStore *>(&*sub);
if (!logSubP) {
printInfo("Skipped '%s' which does not support retrieving build logs", sub->config.getUri());
printInfo(
"Skipped '%s' which does not support retrieving build logs", sub->config.getHumanReadableURI());
continue;
}
auto & logSub = *logSubP;
auto log = logSub.getBuildLog(drvPath);
if (log) {
printInfo("got build log for '%s' from '%s'", drvPathRaw, logSub.config.getUri());
printInfo("got build log for '%s' from '%s'", drvPathRaw, logSub.config.getHumanReadableURI());
logger->writeToStdout(*log);
foundLog = true;
break;

View file

@ -62,7 +62,7 @@ nix_err nix_store_get_uri(nix_c_context * context, Store * store, nix_get_string
if (context)
context->last_err_code = NIX_OK;
try {
auto res = store->ptr->config.getUri();
auto res = store->ptr->config.getReference().render(/*withParams=*/true);
return call_nix_get_string_callback(res, callback, user_data);
}
NIXC_CATCH_ERRS

View file

@ -6,9 +6,7 @@ namespace nix {
TEST(LegacySSHStore, constructConfig)
{
initLibStore(/*loadConfig=*/false);
auto config = make_ref<LegacySSHStoreConfig>(
LegacySSHStoreConfig config(
"ssh",
"me@localhost:2222",
StoreConfig::Params{
@ -20,13 +18,13 @@ TEST(LegacySSHStore, constructConfig)
});
EXPECT_EQ(
config->remoteProgram.get(),
config.remoteProgram.get(),
(Strings{
"foo",
"bar",
}));
auto store = config->openStore();
EXPECT_EQ(store->config.getUri(), "ssh://me@localhost:2222?remote-program=foo%20bar");
EXPECT_EQ(config.getReference().render(/*withParams=*/true), "ssh://me@localhost:2222?remote-program=foo%20bar");
EXPECT_EQ(config.getReference().render(/*withParams=*/false), "ssh://me@localhost:2222");
}
} // namespace nix

View file

@ -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.getReference().render(/*withParams=*/true).c_str());
std::string str;
nix_store_get_version(ctx, store, OBSERVE_STRING(str));

View file

@ -27,9 +27,9 @@ TEST(SSHStore, constructConfig)
"bar",
}));
EXPECT_EQ(config.getUri(), "ssh-ng://me@localhost:2222?remote-program=foo%20bar");
EXPECT_EQ(config.getReference().render(/*withParams=*/true), "ssh-ng://me@localhost:2222?remote-program=foo%20bar");
config.resetOverridden();
EXPECT_EQ(config.getUri(), "ssh-ng://me@localhost:2222");
EXPECT_EQ(config.getReference().render(/*withParams=*/true), "ssh-ng://me@localhost:2222");
}
TEST(MountedSSHStore, constructConfig)

View file

@ -59,7 +59,7 @@ void BinaryCacheStore::init()
if (value != storeDir)
throw Error(
"binary cache '%s' is for Nix stores with prefix '%s', not '%s'",
config.getUri(),
config.getHumanReadableURI(),
value,
storeDir);
} else if (name == "WantMassQuery") {
@ -133,7 +133,9 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
if (diskCache)
diskCache->upsertNarInfo(
config.getUri(), std::string(narInfo->path.hashPart()), std::shared_ptr<NarInfo>(narInfo));
config.getReference().render(/*FIXME withParams=*/false),
std::string(narInfo->path.hashPart()),
std::shared_ptr<NarInfo>(narInfo));
}
ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
@ -431,7 +433,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink)
void BinaryCacheStore::queryPathInfoUncached(
const StorePath & storePath, Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept
{
auto uri = config.getUri();
auto uri = config.getReference().render(/*FIXME withParams=*/false);
auto storePathS = printStorePath(storePath);
auto act = std::make_shared<Activity>(
*logger,
@ -531,7 +533,7 @@ void BinaryCacheStore::queryRealisationUncached(
void BinaryCacheStore::registerDrvOutput(const Realisation & info)
{
if (diskCache)
diskCache->upsertRealisation(config.getUri(), info);
diskCache->upsertRealisation(config.getReference().render(/*FIXME withParams=*/false), info);
auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi";
upsertFile(filePath, info.toJSON().dump(), "application/json");
}
@ -559,7 +561,7 @@ std::optional<std::string> BinaryCacheStore::getBuildLogExact(const StorePath &
{
auto logPath = "log/" + std::string(baseNameOf(printStorePath(path)));
debug("fetching build log from binary cache '%s/%s'", config.getUri(), logPath);
debug("fetching build log from binary cache '%s/%s'", config.getHumanReadableURI(), logPath);
return getFile(logPath);
}

View file

@ -98,7 +98,7 @@ Goal::Co DrvOutputSubstitutionGoal::init()
"substituter '%s' has an incompatible realisation for '%s', ignoring.\n"
"Local: %s\n"
"Remote: %s",
sub->config.getUri(),
sub->config.getHumanReadableURI(),
depId.to_string(),
worker.store.printStorePath(localOutputInfo->outPath),
worker.store.printStorePath(depPath));

View file

@ -101,7 +101,7 @@ Goal::Co PathSubstitutionGoal::init()
} else {
printError(
"asked '%s' for '%s' but got '%s'",
sub->config.getUri(),
sub->config.getHumanReadableURI(),
worker.store.printStorePath(storePath),
sub->printStorePath(info->path));
continue;
@ -127,7 +127,7 @@ Goal::Co PathSubstitutionGoal::init()
warn(
"ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'",
worker.store.printStorePath(storePath),
sub->config.getUri());
sub->config.getHumanReadableURI());
continue;
}
@ -218,7 +218,9 @@ Goal::Co PathSubstitutionGoal::tryToRun(
Finally updateStats([this]() { outPipe.writeSide.close(); });
Activity act(
*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->config.getUri()});
*logger,
actSubstitute,
Logger::Fields{worker.store.printStorePath(storePath), sub->config.getHumanReadableURI()});
PushActivity pact(act.id);
copyStorePath(*sub, worker.store, subPath, repair, sub->config.isTrusted ? NoCheckSigs : CheckSigs);

View file

@ -98,7 +98,7 @@ protected:
auto state(_state.lock());
if (state->enabled && settings.tryFallback) {
int t = 60;
printError("disabling binary cache '%s' for %s seconds", config->getUri(), t);
printError("disabling binary cache '%s' for %s seconds", config->getHumanReadableURI(), t);
state->enabled = false;
state->disabledUntil = std::chrono::steady_clock::now() + std::chrono::seconds(t);
}
@ -111,10 +111,10 @@ protected:
return;
if (std::chrono::steady_clock::now() > state->disabledUntil) {
state->enabled = true;
debug("re-enabling binary cache '%s'", config->getUri());
debug("re-enabling binary cache '%s'", config->getHumanReadableURI());
return;
}
throw SubstituterDisabled("substituter '%s' is disabled", config->getUri());
throw SubstituterDisabled("substituter '%s' is disabled", config->getHumanReadableURI());
}
bool fileExists(const std::string & path) override
@ -180,7 +180,8 @@ protected:
getFileTransfer()->download(std::move(request), sink);
} catch (FileTransferError & e) {
if (e.error == FileTransfer::NotFound || e.error == FileTransfer::Forbidden)
throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache '%s'", path, config->getUri());
throw NoSuchBinaryCacheFile(
"file '%s' does not exist in binary cache '%s'", path, config->getHumanReadableURI());
maybeDisable();
throw;
}

View file

@ -205,9 +205,20 @@ struct StoreConfig : public StoreDirConfig
*/
virtual StoreReference getReference() const;
std::string getUri() const
/**
* Get a textual representation of the store reference.
*
* @warning This is only suitable for logging or error messages.
* This will not roundtrip when parsed as a StoreReference.
* Must NOT be used as a cache key or otherwise be relied upon to
* be stable.
*
* Can be implemented by subclasses to make the URI more legible,
* e.g. when some query parameters are necessary to make sense of the URI.
*/
virtual std::string getHumanReadableURI() const
{
return getReference().render();
return getReference().render(/*withParams=*/false);
}
};
@ -878,7 +889,7 @@ protected:
*/
[[noreturn]] void unsupported(const std::string & op)
{
throw Unsupported("operation '%s' is not supported by store '%s'", op, config.getUri());
throw Unsupported("operation '%s' is not supported by store '%s'", op, config.getHumanReadableURI());
}
};

View file

@ -17,7 +17,7 @@ T & require(Store & store)
{
auto * castedStore = dynamic_cast<T *>(&store);
if (!castedStore)
throw UsageError("%s not supported by store '%s'", T::operationName, store.config.getUri());
throw UsageError("%s not supported by store '%s'", T::operationName, store.config.getHumanReadableURI());
return *castedStore;
}

View file

@ -73,9 +73,9 @@ struct StoreReference
bool operator==(const StoreReference & rhs) const = default;
/**
* Render the whole store reference as a URI, including parameters.
* Render the whole store reference as a URI, optionally including parameters.
*/
std::string render() const;
std::string render(bool withParams = true) const;
/**
* Parse a URI into a store reference.

View file

@ -53,7 +53,7 @@ RemoteStore::RemoteStore(const Config & config)
ref<RemoteStore::Connection> RemoteStore::openConnectionWrapper()
{
if (failed)
throw Error("opening a connection to remote store '%s' previously failed", config.getUri());
throw Error("opening a connection to remote store '%s' previously failed", config.getHumanReadableURI());
try {
return openConnection();
} catch (...) {
@ -95,7 +95,7 @@ void RemoteStore::initConnection(Connection & conn)
if (ex)
std::rethrow_exception(ex);
} catch (Error & e) {
throw Error("cannot open connection to remote store '%s': %s", config.getUri(), e.what());
throw Error("cannot open connection to remote store '%s': %s", config.getHumanReadableURI(), e.what());
}
setOptions(conn);

View file

@ -282,12 +282,15 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore
void init() override
{
if (auto cacheInfo = diskCache->upToDateCacheExists(config->getUri())) {
/* FIXME: The URI (when used as a cache key) must have several parameters rendered (e.g. the endpoint).
This must be represented as a separate opaque string (probably a URI) that has the right query parameters. */
auto cacheUri = config->getReference().render(/*withParams=*/false);
if (auto cacheInfo = diskCache->upToDateCacheExists(cacheUri)) {
config->wantMassQuery.setDefault(cacheInfo->wantMassQuery);
config->priority.setDefault(cacheInfo->priority);
} else {
BinaryCacheStore::init();
diskCache->createCache(config->getUri(), config->storeDir, config->wantMassQuery, config->priority);
diskCache->createCache(cacheUri, config->storeDir, config->wantMassQuery, config->priority);
}
}
@ -525,7 +528,8 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore
sink(*res.data);
} else
throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache '%s'", path, config->getUri());
throw NoSuchBinaryCacheFile(
"file '%s' does not exist in binary cache '%s'", path, config->getHumanReadableURI());
}
StorePathSet queryAllValidPaths() override

View file

@ -395,11 +395,14 @@ void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, Substituta
"replaced path '%s' with '%s' for substituter '%s'",
printStorePath(path.first),
sub->printStorePath(subPath),
sub->config.getUri());
sub->config.getHumanReadableURI());
} else if (sub->storeDir != storeDir)
continue;
debug("checking substituter '%s' for path '%s'", sub->config.getUri(), sub->printStorePath(subPath));
debug(
"checking substituter '%s' for path '%s'",
sub->config.getHumanReadableURI(),
sub->printStorePath(subPath));
try {
auto info = sub->queryPathInfo(subPath);
@ -439,7 +442,8 @@ bool Store::isValidPath(const StorePath & storePath)
}
if (diskCache) {
auto res = diskCache->lookupNarInfo(config.getUri(), std::string(storePath.hashPart()));
auto res = diskCache->lookupNarInfo(
config.getReference().render(/*FIXME withParams=*/false), std::string(storePath.hashPart()));
if (res.first != NarInfoDiskCache::oUnknown) {
stats.narInfoReadAverted++;
auto state_(state.lock());
@ -455,7 +459,8 @@ bool Store::isValidPath(const StorePath & storePath)
if (diskCache && !valid)
// FIXME: handle valid = true case.
diskCache->upsertNarInfo(config.getUri(), std::string(storePath.hashPart()), 0);
diskCache->upsertNarInfo(
config.getReference().render(/*FIXME withParams=*/false), std::string(storePath.hashPart()), 0);
return valid;
}
@ -509,7 +514,7 @@ std::optional<std::shared_ptr<const ValidPathInfo>> Store::queryPathInfoFromClie
}
if (diskCache) {
auto res = diskCache->lookupNarInfo(config.getUri(), hashPart);
auto res = diskCache->lookupNarInfo(config.getReference().render(/*FIXME withParams=*/false), hashPart);
if (res.first != NarInfoDiskCache::oUnknown) {
stats.narInfoReadAverted++;
{
@ -554,7 +559,7 @@ void Store::queryPathInfo(const StorePath & storePath, Callback<ref<const ValidP
auto info = fut.get();
if (diskCache)
diskCache->upsertNarInfo(config.getUri(), hashPart, info);
diskCache->upsertNarInfo(config.getReference().render(/*FIXME withParams=*/false), hashPart, info);
{
auto state_(state.lock());
@ -578,7 +583,8 @@ void Store::queryRealisation(const DrvOutput & id, Callback<std::shared_ptr<cons
try {
if (diskCache) {
auto [cacheOutcome, maybeCachedRealisation] = diskCache->lookupRealisation(config.getUri(), id);
auto [cacheOutcome, maybeCachedRealisation] =
diskCache->lookupRealisation(config.getReference().render(/*FIXME: withParams=*/false), id);
switch (cacheOutcome) {
case NarInfoDiskCache::oValid:
debug("Returning a cached realisation for %s", id.to_string());
@ -604,9 +610,11 @@ void Store::queryRealisation(const DrvOutput & id, Callback<std::shared_ptr<cons
if (diskCache) {
if (info)
diskCache->upsertRealisation(config.getUri(), *info);
diskCache->upsertRealisation(
config.getReference().render(/*FIXME withParams=*/false), *info);
else
diskCache->upsertAbsentRealisation(config.getUri(), id);
diskCache->upsertAbsentRealisation(
config.getReference().render(/*FIXME withParams=*/false), id);
}
(*callbackPtr)(std::shared_ptr<const Realisation>(info));
@ -785,12 +793,27 @@ const Store::Stats & Store::getStats()
return stats;
}
static std::string makeCopyPathMessage(std::string_view srcUri, std::string_view dstUri, std::string_view storePath)
static std::string
makeCopyPathMessage(const StoreConfig & srcCfg, const StoreConfig & dstCfg, std::string_view storePath)
{
return srcUri == "local" || srcUri == "daemon" ? fmt("copying path '%s' to '%s'", storePath, dstUri)
: dstUri == "local" || dstUri == "daemon"
? fmt("copying path '%s' from '%s'", storePath, srcUri)
: fmt("copying path '%s' from '%s' to '%s'", storePath, srcUri, dstUri);
auto src = srcCfg.getReference();
auto dst = dstCfg.getReference();
auto isShorthand = [](const StoreReference & ref) {
/* At this point StoreReference **must** be resolved. */
const auto & specified = std::get<StoreReference::Specified>(ref.variant);
const auto & scheme = specified.scheme;
return (scheme == "local" || scheme == "unix") && specified.authority.empty() && ref.params.empty();
};
if (isShorthand(src))
return fmt("copying path '%s' to '%s'", storePath, dstCfg.getHumanReadableURI());
if (isShorthand(dst))
return fmt("copying path '%s' from '%s'", storePath, srcCfg.getHumanReadableURI());
return fmt(
"copying path '%s' from '%s' to '%s'", storePath, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI());
}
void copyStorePath(
@ -801,11 +824,15 @@ void copyStorePath(
if (!repair && dstStore.isValidPath(storePath))
return;
auto srcUri = srcStore.config.getUri();
auto dstUri = dstStore.config.getUri();
const auto & srcCfg = srcStore.config;
const auto & dstCfg = dstStore.config;
auto storePathS = srcStore.printStorePath(storePath);
Activity act(
*logger, lvlInfo, actCopyPath, makeCopyPathMessage(srcUri, dstUri, storePathS), {storePathS, srcUri, dstUri});
*logger,
lvlInfo,
actCopyPath,
makeCopyPathMessage(srcCfg, dstCfg, storePathS),
{storePathS, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI()});
PushActivity pact(act.id);
auto info = srcStore.queryPathInfo(storePath);
@ -841,7 +868,7 @@ void copyStorePath(
throw EndOfFile(
"NAR for '%s' fetched from '%s' is incomplete",
srcStore.printStorePath(storePath),
srcStore.config.getUri());
srcStore.config.getHumanReadableURI());
});
dstStore.addToStore(*info, *source, repair, checkSigs);
@ -939,7 +966,7 @@ std::map<StorePath, StorePath> copyPaths(
"replaced path '%s' to '%s' for substituter '%s'",
srcStore.printStorePath(storePathForSrc),
dstStore.printStorePath(storePathForDst),
dstStore.config.getUri());
dstStore.config.getHumanReadableURI());
}
return storePathForDst;
};
@ -957,15 +984,15 @@ std::map<StorePath, StorePath> copyPaths(
// We can reasonably assume that the copy will happen whenever we
// read the path, so log something about that at that point
uint64_t total = 0;
auto srcUri = srcStore.config.getUri();
auto dstUri = dstStore.config.getUri();
const auto & srcCfg = srcStore.config;
const auto & dstCfg = dstStore.config;
auto storePathS = srcStore.printStorePath(missingPath);
Activity act(
*logger,
lvlInfo,
actCopyPath,
makeCopyPathMessage(srcUri, dstUri, storePathS),
{storePathS, srcUri, dstUri});
makeCopyPathMessage(srcCfg, dstCfg, storePathS),
{storePathS, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI()});
PushActivity pact(act.id);
LambdaSink progressSink([&](std::string_view data) {

View file

@ -18,7 +18,7 @@ static bool isNonUriPath(const std::string & spec)
&& spec.find("/") != std::string::npos;
}
std::string StoreReference::render() const
std::string StoreReference::render(bool withParams) const
{
std::string res;
@ -33,7 +33,7 @@ std::string StoreReference::render() const
},
variant);
if (!params.empty()) {
if (withParams && !params.empty()) {
res += "?";
res += encodeQuery(params);
}

View file

@ -71,7 +71,7 @@ struct CmdConfigCheck : StoreCommand
void run(ref<Store> store) override
{
logger->log("Running checks against store uri: " + store->config.getUri());
logger->log("Running checks against store uri: " + store->config.getHumanReadableURI());
if (store.dynamic_pointer_cast<LocalFSStore>()) {
success &= checkNixInPath();
@ -171,9 +171,9 @@ struct CmdConfigCheck : StoreCommand
{
if (auto trustedMay = store->isTrustedClient()) {
std::string_view trusted = trustedMay.value() ? "trusted" : "not trusted";
checkInfo(fmt("You are %s by store uri: %s", trusted, store->config.getUri()));
checkInfo(fmt("You are %s by store uri: %s", trusted, store->config.getHumanReadableURI()));
} else {
checkInfo(fmt("Store uri: %s doesn't have a notion of trusted user", store->config.getUri()));
checkInfo(fmt("Store uri: %s doesn't have a notion of trusted user", store->config.getHumanReadableURI()));
}
}
};

View file

@ -48,7 +48,8 @@ struct CmdLog : InstallableCommand
for (auto & sub : subs) {
auto * logSubP = dynamic_cast<LogStore *>(&*sub);
if (!logSubP) {
printInfo("Skipped '%s' which does not support retrieving build logs", sub->config.getUri());
printInfo(
"Skipped '%s' which does not support retrieving build logs", sub->config.getHumanReadableURI());
continue;
}
auto & logSub = *logSubP;
@ -57,7 +58,7 @@ struct CmdLog : InstallableCommand
if (!log)
continue;
logger->stop();
printInfo("got build log for '%s' from '%s'", installable->what(), logSub.config.getUri());
printInfo("got build log for '%s' from '%s'", installable->what(), logSub.config.getHumanReadableURI());
writeFull(getStandardOutput(), *log);
return;
}

View file

@ -77,7 +77,9 @@ void execProgramInStore(
auto store2 = store.dynamic_pointer_cast<LocalFSStore>();
if (!store2)
throw Error("store '%s' is not a local store so it does not support command execution", store->config.getUri());
throw Error(
"store '%s' is not a local store so it does not support command execution",
store->config.getHumanReadableURI());
if (store->storeDir != store2->getRealStoreDir()) {
Strings helperArgs = {

View file

@ -24,7 +24,7 @@ struct CmdInfoStore : StoreCommand, MixJSON
void run(ref<Store> store) override
{
if (!json) {
notice("Store URL: %s", store->config.getUri());
notice("Store URL: %s", store->config.getReference().render(/*withParams=*/true));
store->connect();
if (auto version = store->getVersion())
notice("Version: %s", *version);
@ -34,7 +34,7 @@ struct CmdInfoStore : StoreCommand, MixJSON
nlohmann::json res;
Finally printRes([&]() { printJSON(res); });
res["url"] = store->config.getUri();
res["url"] = store->config.getReference().render(/*withParams=*/true);
store->connect();
if (auto version = store->getVersion())
res["version"] = *version;