treewide: Remove getUri and replace with getHumanReadableURI where appropriate

src/libcmd/repl.cc

@@ -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;

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

src/libstore-tests/legacy-ssh-store.cc

@@ -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

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));

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)

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> 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);
 }

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));

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);

src/libstore/http-binary-cache-store.cc

@@ -98,7 +98,7 @@ class HttpBinaryCacheStore : public virtual BinaryCacheStore
         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 @@ class HttpBinaryCacheStore : public virtual BinaryCacheStore
             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 @@ class HttpBinaryCacheStore : public virtual BinaryCacheStore
             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;
         }

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());
     }
 };
 

src/libstore/include/nix/store/store-cast.hh

@@ -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;
 }
 

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.

src/libstore/remote-store.cc

@@ -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);

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