From 1b7ffa53af168aab255c9ee4c1ca5a192c269738 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 14 Aug 2025 16:47:05 +0300 Subject: [PATCH] treewide: Remove getUri and replace with getHumanReadableURI where appropriate The problem with old code was that it used getUri for both the `diskCache` as well as logging. This is really bad because it mixes the textual human readable representation with the caching. Also using getUri for the cache key is really problematic for the S3 store, since it doesn't include the `endpoint` in the cache key, so it's totally broken. This starts separating the logging / cache concerns by introducing a `getHumanReadableURI` that should only be used for logging. The caching logic now instead uses `getReference().render(/*withParams=*/false)` exclusively. This would need to be fixed in follow-ups, because that's really fragile and broken for some store types (but it was already broken before). --- src/libcmd/repl.cc | 5 +- src/libstore-c/nix_api_store.cc | 2 +- src/libstore-tests/legacy-ssh-store.cc | 10 ++- src/libstore-tests/nix_api_store.cc | 2 +- src/libstore-tests/ssh-store.cc | 4 +- src/libstore/binary-cache-store.cc | 12 ++-- .../build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/substitution-goal.cc | 8 ++- src/libstore/http-binary-cache-store.cc | 9 +-- src/libstore/include/nix/store/store-api.hh | 17 ++++- src/libstore/include/nix/store/store-cast.hh | 2 +- .../include/nix/store/store-reference.hh | 4 +- src/libstore/remote-store.cc | 4 +- src/libstore/s3-binary-cache-store.cc | 10 ++- src/libstore/store-api.cc | 67 +++++++++++-------- src/libstore/store-reference.cc | 4 +- src/nix/config-check.cc | 6 +- src/nix/log.cc | 5 +- src/nix/run.cc | 4 +- src/nix/store-info.cc | 4 +- 20 files changed, 107 insertions(+), 74 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 863de05ed..01d786deb 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -574,14 +574,15 @@ ProcessLineResult NixRepl::processLine(std::string line) for (auto & sub : subs) { auto * logSubP = dynamic_cast(&*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; diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index 705d8153f..4f91f5332 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -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 diff --git a/src/libstore-tests/legacy-ssh-store.cc b/src/libstore-tests/legacy-ssh-store.cc index 04c3763ec..d60ecc424 100644 --- a/src/libstore-tests/legacy-ssh-store.cc +++ b/src/libstore-tests/legacy-ssh-store.cc @@ -6,9 +6,7 @@ namespace nix { TEST(LegacySSHStore, constructConfig) { - initLibStore(/*loadConfig=*/false); - - auto config = make_ref( + 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 diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index 2310c4395..c7146f977 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -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)); diff --git a/src/libstore-tests/ssh-store.cc b/src/libstore-tests/ssh-store.cc index 335e4ae85..a156da52b 100644 --- a/src/libstore-tests/ssh-store.cc +++ b/src/libstore-tests/ssh-store.cc @@ -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) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index c55239413..0a44b0cf0 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -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) if (diskCache) diskCache->upsertNarInfo( - config.getUri(), std::string(narInfo->path.hashPart()), std::shared_ptr(narInfo)); + config.getReference().render(/*FIXME withParams=*/false), + std::string(narInfo->path.hashPart()), + std::shared_ptr(narInfo)); } ref BinaryCacheStore::addToStoreCommon( @@ -431,7 +433,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) void BinaryCacheStore::queryPathInfoUncached( const StorePath & storePath, Callback> callback) noexcept { - auto uri = config.getUri(); + auto uri = config.getReference().render(/*FIXME withParams=*/false); auto storePathS = printStorePath(storePath); auto act = std::make_shared( *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 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); } diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 3f4b787f7..222cd8618 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -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)); diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index e46ad2007..3c0e96152 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -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); diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index fc19bc1f8..2777b8827 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -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; } diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 3e32c49a3..ba80c18f2 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -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()); } }; diff --git a/src/libstore/include/nix/store/store-cast.hh b/src/libstore/include/nix/store/store-cast.hh index 0d7257602..1e0b14914 100644 --- a/src/libstore/include/nix/store/store-cast.hh +++ b/src/libstore/include/nix/store/store-cast.hh @@ -17,7 +17,7 @@ T & require(Store & store) { auto * castedStore = dynamic_cast(&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; } diff --git a/src/libstore/include/nix/store/store-reference.hh b/src/libstore/include/nix/store/store-reference.hh index fff3b5c5c..5cf1e9a11 100644 --- a/src/libstore/include/nix/store/store-reference.hh +++ b/src/libstore/include/nix/store/store-reference.hh @@ -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. diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 3eff339e1..d3446093d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -53,7 +53,7 @@ RemoteStore::RemoteStore(const Config & config) ref 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); diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 84eb63f7f..4ad09aff2 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -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 diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 298b23e5e..493137361 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -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> 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, CallbackupsertNarInfo(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, CallbacklookupRealisation(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, CallbackupsertRealisation(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(info)); @@ -786,23 +794,26 @@ const Store::Stats & Store::getStats() } static std::string -makeCopyPathMessage(const StoreReference & src, const StoreReference & dst, std::string_view storePath) +makeCopyPathMessage(const StoreConfig & srcCfg, const StoreConfig & dstCfg, std::string_view storePath) { + auto src = srcCfg.getReference(); + auto dst = dstCfg.getReference(); + auto isShorthand = [](const StoreReference & ref) { - if (const auto * specified = std::get_if(&ref.variant)) { - const auto & scheme = specified->scheme; - return (scheme == "local" || scheme == "unix") && specified->authority.empty() && ref.params.empty(); - } - return false; + /* At this point StoreReference **must** be resolved. */ + const auto & specified = std::get(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, dst.render()); + return fmt("copying path '%s' to '%s'", storePath, dstCfg.getHumanReadableURI()); if (isShorthand(dst)) - return fmt("copying path '%s' from '%s'", storePath, src.render()); + return fmt("copying path '%s' from '%s'", storePath, srcCfg.getHumanReadableURI()); - return fmt("copying path '%s' from '%s' to '%s'", storePath, src.render(), dst.render()); + return fmt( + "copying path '%s' from '%s' to '%s'", storePath, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI()); } void copyStorePath( @@ -813,15 +824,15 @@ void copyStorePath( if (!repair && dstStore.isValidPath(storePath)) return; - auto srcRef = srcStore.config.getReference(); - auto dstRef = dstStore.config.getReference(); + const auto & srcCfg = srcStore.config; + const auto & dstCfg = dstStore.config; auto storePathS = srcStore.printStorePath(storePath); Activity act( *logger, lvlInfo, actCopyPath, - makeCopyPathMessage(srcRef, dstRef, storePathS), - {storePathS, srcRef.render(), dstRef.render()}); + makeCopyPathMessage(srcCfg, dstCfg, storePathS), + {storePathS, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI()}); PushActivity pact(act.id); auto info = srcStore.queryPathInfo(storePath); @@ -857,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); @@ -955,7 +966,7 @@ std::map copyPaths( "replaced path '%s' to '%s' for substituter '%s'", srcStore.printStorePath(storePathForSrc), dstStore.printStorePath(storePathForDst), - dstStore.config.getUri()); + dstStore.config.getHumanReadableURI()); } return storePathForDst; }; @@ -973,15 +984,15 @@ std::map 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 srcRef = srcStore.config.getReference(); - auto dstRef = dstStore.config.getReference(); + const auto & srcCfg = srcStore.config; + const auto & dstCfg = dstStore.config; auto storePathS = srcStore.printStorePath(missingPath); Activity act( *logger, lvlInfo, actCopyPath, - makeCopyPathMessage(srcRef, dstRef, storePathS), - {storePathS, srcRef.render(), dstRef.render()}); + makeCopyPathMessage(srcCfg, dstCfg, storePathS), + {storePathS, srcCfg.getHumanReadableURI(), dstCfg.getHumanReadableURI()}); PushActivity pact(act.id); LambdaSink progressSink([&](std::string_view data) { diff --git a/src/libstore/store-reference.cc b/src/libstore/store-reference.cc index 13feeae3e..2b8305072 100644 --- a/src/libstore/store-reference.cc +++ b/src/libstore/store-reference.cc @@ -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); } diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 685795487..dc6453e27 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -71,7 +71,7 @@ struct CmdConfigCheck : StoreCommand void run(ref 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()) { 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())); } } }; diff --git a/src/nix/log.cc b/src/nix/log.cc index 2b697c609..cabe611fa 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -48,7 +48,8 @@ struct CmdLog : InstallableCommand for (auto & sub : subs) { auto * logSubP = dynamic_cast(&*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; } diff --git a/src/nix/run.cc b/src/nix/run.cc index cd7784cee..c3d416a6e 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -77,7 +77,9 @@ void execProgramInStore( auto store2 = store.dynamic_pointer_cast(); 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 = { diff --git a/src/nix/store-info.cc b/src/nix/store-info.cc index 92fcef663..4526d9cda 100644 --- a/src/nix/store-info.cc +++ b/src/nix/store-info.cc @@ -24,7 +24,7 @@ struct CmdInfoStore : StoreCommand, MixJSON void run(ref 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;