Relegate toRealPath to LocalFSStore

Fix #14480

This method is not well-defined for arbitrary stores, which do not have
a notion of a "real path" -- it is only well-defined for local file
systems stores, which do have exactly that notion, and so it is moved to
that sub-interface instead.

Some call-sites had to be fixed up for this, but in all cases the
changes are positive. Using `getFSSourceAccessor` allows for more other
stores to work properly. `nix-channel` was straight-up wrong in the case
of redirected local stores. And the building logic with remote building
and a non-local store is also fixed, properly gating some deletions on
store type.

Co-authored-by: Robert Hensing <robert@roberthensing.nl>

src/libfetchers/include/nix/fetchers/registry.hh

@@ -2,6 +2,7 @@
 ///@file
 
 #include "nix/util/types.hh"
+#include "nix/util/source-path.hh"
 #include "nix/fetchers/fetchers.hh"
 
 namespace nix {
@@ -39,7 +40,7 @@ struct Registry
     {
     }
 
-    static std::shared_ptr<Registry> read(const Settings & settings, const Path & path, RegistryType type);
+    static std::shared_ptr<Registry> read(const Settings & settings, const SourcePath & path, RegistryType type);
 
     void write(const Path & path);
 

src/libfetchers/registry.cc

@@ -10,18 +10,18 @@
 
 namespace nix::fetchers {
 
-std::shared_ptr<Registry> Registry::read(const Settings & settings, const Path & path, RegistryType type)
+std::shared_ptr<Registry> Registry::read(const Settings & settings, const SourcePath & path, RegistryType type)
 {
     debug("reading registry '%s'", path);
 
     auto registry = std::make_shared<Registry>(settings, type);
 
-    if (!pathExists(path))
+    if (!path.pathExists())
         return std::make_shared<Registry>(settings, type);
 
     try {
 
-        auto json = nlohmann::json::parse(readFile(path));
+        auto json = nlohmann::json::parse(path.readFile());
 
         auto version = json.value("version", 0);
 
@@ -97,7 +97,10 @@ static Path getSystemRegistryPath()
 
 static std::shared_ptr<Registry> getSystemRegistry(const Settings & settings)
 {
-    static auto systemRegistry = Registry::read(settings, getSystemRegistryPath(), Registry::System);
+    static auto systemRegistry = Registry::read(
+        settings,
+        SourcePath{getFSSourceAccessor(), CanonPath{getSystemRegistryPath()}}.resolveSymlinks(),
+        Registry::System);
     return systemRegistry;
 }
 
@@ -108,13 +111,17 @@ Path getUserRegistryPath()
 
 std::shared_ptr<Registry> getUserRegistry(const Settings & settings)
 {
-    static auto userRegistry = Registry::read(settings, getUserRegistryPath(), Registry::User);
+    static auto userRegistry = Registry::read(
+        settings,
+        SourcePath{getFSSourceAccessor(), CanonPath{getUserRegistryPath()}}.resolveSymlinks(),
+        Registry::User);
     return userRegistry;
 }
 
 std::shared_ptr<Registry> getCustomRegistry(const Settings & settings, const Path & p)
 {
-    static auto customRegistry = Registry::read(settings, p, Registry::Custom);
+    static auto customRegistry =
+        Registry::read(settings, SourcePath{getFSSourceAccessor(), CanonPath{p}}.resolveSymlinks(), Registry::Custom);
     return customRegistry;
 }
 
@@ -137,14 +144,19 @@ static std::shared_ptr<Registry> getGlobalRegistry(const Settings & settings, re
             return std::make_shared<Registry>(settings, Registry::Global); // empty registry
         }
 
-        if (!isAbsolute(path)) {
+        return Registry::read(
-            auto storePath = downloadFile(store, settings, path, "flake-registry.json").storePath;
+            settings,
-            if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>())
+            [&] -> SourcePath {
-                store2->addPermRoot(storePath, getCacheDir() + "/flake-registry.json");
+                if (!isAbsolute(path)) {
-            path = store->toRealPath(storePath);
+                    auto storePath = downloadFile(store, settings, path, "flake-registry.json").storePath;
-        }
+                    if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>())
-
+                        store2->addPermRoot(storePath, getCacheDir() + "/flake-registry.json");
-        return Registry::read(settings, path, Registry::Global);
+                    return {store->requireStoreObjectAccessor(storePath)};
+                } else {
+                    return SourcePath{getFSSourceAccessor(), CanonPath{path}}.resolveSymlinks();
+                }
+            }(),
+            Registry::Global);
     }();
 
     return reg;

src/libstore-c/nix_api_store.cc

@@ -7,6 +7,7 @@
 #include "nix/store/store-api.hh"
 #include "nix/store/store-open.hh"
 #include "nix/store/build-result.hh"
+#include "nix/store/local-fs-store.hh"
 
 #include "nix/store/globals.hh"
 
@@ -109,7 +110,8 @@ nix_err nix_store_real_path(
     if (context)
         context->last_err_code = NIX_OK;
     try {
-        auto res = store->ptr->toRealPath(path->path);
+        auto store2 = store->ptr.dynamic_pointer_cast<nix::LocalFSStore>();
+        auto res = store2 ? store2->toRealPath(path->path) : store->ptr->printStorePath(path->path);
         return call_nix_get_string_callback(res, callback, user_data);
     }
     NIXC_CATCH_ERRS

src/libstore/build/derivation-building-goal.cc

@@ -286,7 +286,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
         PathSet lockFiles;
         /* FIXME: Should lock something like the drv itself so we don't build same
            CA drv concurrently */
-        if (dynamic_cast<LocalStore *>(&worker.store)) {
+        if (auto * localStore = dynamic_cast<LocalStore *>(&worker.store)) {
             /* If we aren't a local store, we might need to use the local store as
                a build remote, but that would cause a deadlock. */
             /* FIXME: Make it so we can use ourselves as a build remote even if we
@@ -296,9 +296,9 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                */
             for (auto & i : drv->outputsAndOptPaths(worker.store)) {
                 if (i.second.second)
-                    lockFiles.insert(worker.store.Store::toRealPath(*i.second.second));
+                    lockFiles.insert(localStore->toRealPath(*i.second.second));
                 else
-                    lockFiles.insert(worker.store.Store::toRealPath(drvPath) + "." + i.first);
+                    lockFiles.insert(localStore->toRealPath(drvPath) + "." + i.first);
             }
         }
 
@@ -331,12 +331,14 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
 
         /* If any of the outputs already exist but are not valid, delete
            them. */
-        for (auto & [_, status] : initialOutputs) {
+        if (auto * localStore = dynamic_cast<LocalFSStore *>(&worker.store)) {
-            if (!status.known || status.known->isValid())
+            for (auto & [_, status] : initialOutputs) {
-                continue;
+                if (!status.known || status.known->isValid())
-            auto storePath = status.known->path;
+                    continue;
-            debug("removing invalid path '%s'", worker.store.printStorePath(status.known->path));
+                auto storePath = status.known->path;
-            deletePath(worker.store.Store::toRealPath(storePath));
+                debug("removing invalid path '%s'", worker.store.printStorePath(status.known->path));
+                deletePath(localStore->toRealPath(storePath));
+            }
         }
 
         /* Don't do a remote build if the derivation has the attribute

src/libstore/daemon.cc

@@ -896,7 +896,7 @@ static void performOp(
         auto path = WorkerProto::Serialise<StorePath>::read(*store, rconn);
         logger->startWork();
         logger->stopWork();
-        dumpPath(store->toRealPath(path), conn.to);
+        store->narFromPath(path, conn.to);
         break;
     }
 

src/libstore/include/nix/store/local-fs-store.hh

@@ -102,7 +102,12 @@ struct LocalFSStore : virtual Store, virtual GcStore, virtual LogStore
         return config.realStoreDir;
     }
 
-    Path toRealPath(const Path & storePath) override
+    Path toRealPath(const StorePath & storePath)
+    {
+        return toRealPath(printStorePath(storePath));
+    }
+
+    Path toRealPath(const Path & storePath)
     {
         assert(isInStore(storePath));
         return getRealStoreDir() + "/" + std::string(storePath, storeDir.size() + 1);

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

@@ -895,16 +895,6 @@ public:
      */
     virtual std::optional<TrustedFlag> isTrustedClient() = 0;
 
-    virtual Path toRealPath(const Path & storePath)
-    {
-        return storePath;
-    }
-
-    Path toRealPath(const StorePath & storePath)
-    {
-        return toRealPath(printStorePath(storePath));
-    }
-
     /**
      * Synchronises the options of the client with those of the daemon
      * (a no-op when there’s no daemon)

src/libstore/local-overlay-store.cc

@@ -246,7 +246,7 @@ void LocalOverlayStore::optimiseStore()
         if (lowerStore->isValidPath(path)) {
             uint64_t bytesFreed = 0;
             // Deduplicate store path
-            deleteStorePath(Store::toRealPath(path), bytesFreed);
+            deleteStorePath(toRealPath(path), bytesFreed);
         }
         done++;
         act.progress(done, paths.size());

src/libstore/local-store.cc

@@ -1063,7 +1063,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, RepairF
 
             PathLocks outputLock;
 
-            auto realPath = Store::toRealPath(info.path);
+            auto realPath = toRealPath(info.path);
 
             /* Lock the output path.  But don't lock if we're being called
             from a build hook (whose parent process already acquired a
@@ -1262,7 +1262,7 @@ StorePath LocalStore::addToStoreFromDump(
         /* The first check above is an optimisation to prevent
            unnecessary lock acquisition. */
 
-        auto realPath = Store::toRealPath(dstPath);
+        auto realPath = toRealPath(dstPath);
 
         PathLocks outputLock({realPath});
 
@@ -1413,7 +1413,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
                 auto hashSink = HashSink(info->narHash.algo);
 
-                dumpPath(Store::toRealPath(i), hashSink);
+                dumpPath(toRealPath(i), hashSink);
                 auto current = hashSink.finish();
 
                 if (info->narHash != nullHash && info->narHash != current.hash) {

src/libstore/unix/build/chroot-derivation-builder.cc

@@ -58,7 +58,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
            environment using bind-mounts.  We put it in the Nix store
            so that the build outputs can be moved efficiently from the
            chroot to their final location. */
-        auto chrootParentDir = store.Store::toRealPath(drvPath) + ".chroot";
+        auto chrootParentDir = store.toRealPath(drvPath) + ".chroot";
         deletePath(chrootParentDir);
 
         /* Clean up the chroot directory automatically. */
@@ -171,7 +171,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
                     continue;
                 if (buildMode != bmCheck && status.known->isValid())
                     continue;
-                auto p = store.Store::toRealPath(status.known->path);
+                auto p = store.toRealPath(status.known->path);
                 if (pathExists(chrootRootDir + p))
                     std::filesystem::rename((chrootRootDir + p), p);
             }
@@ -185,7 +185,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
 
         debug("materialising '%s' in the sandbox", store.printStorePath(path));
 
-        Path source = store.Store::toRealPath(path);
+        Path source = store.toRealPath(path);
         Path target = chrootRootDir + store.printStorePath(path);
 
         if (pathExists(target)) {

src/libstore/unix/build/derivation-builder.cc

@@ -1887,7 +1887,7 @@ void DerivationBuilderImpl::cleanupBuild(bool force)
     if (force) {
         /* Delete unused redirected outputs (when doing hash rewriting). */
         for (auto & i : redirectedOutputs)
-            deletePath(store.Store::toRealPath(i.second));
+            deletePath(store.toRealPath(i.second));
     }
 
     if (topTmpDir != "") {

src/nix/nix-channel/nix-channel.cc

@@ -122,37 +122,33 @@ static void update(const StringSet & channelNames)
             // got redirected in the process, so that we can grab the various parts of a nix channel
             // definition from a consistent location if the redirect changes mid-download.
             auto result = fetchers::downloadFile(store, fetchSettings, url, std::string(baseNameOf(url)));
-            auto filename = store->toRealPath(result.storePath);
             url = result.effectiveUrl;
 
             bool unpacked = false;
-            if (std::regex_search(filename, std::regex("\\.tar\\.(gz|bz2|xz)$"))) {
+            if (std::regex_search(std::string{result.storePath.to_string()}, std::regex("\\.tar\\.(gz|bz2|xz)$"))) {
                 runProgram(
                     getNixBin("nix-build").string(),
                     false,
                     {"--no-out-link",
                      "--expr",
                      "import " + unpackChannelPath + "{ name = \"" + cname + "\"; channelName = \"" + name
-                         + "\"; src = builtins.storePath \"" + filename + "\"; }"});
+                         + "\"; src = builtins.storePath \"" + store->printStorePath(result.storePath) + "\"; }"});
                 unpacked = true;
             }
 
             if (!unpacked) {
                 // Download the channel tarball.
                 try {
-                    filename = store->toRealPath(
+                    result = fetchers::downloadFile(store, fetchSettings, url + "/nixexprs.tar.xz", "nixexprs.tar.xz");
-                        fetchers::downloadFile(store, fetchSettings, url + "/nixexprs.tar.xz", "nixexprs.tar.xz")
-                            .storePath);
                 } catch (FileTransferError & e) {
-                    filename = store->toRealPath(
+                    result =
-                        fetchers::downloadFile(store, fetchSettings, url + "/nixexprs.tar.bz2", "nixexprs.tar.bz2")
+                        fetchers::downloadFile(store, fetchSettings, url + "/nixexprs.tar.bz2", "nixexprs.tar.bz2");
-                            .storePath);
                 }
             }
             // Regardless of where it came from, add the expression representing this channel to accumulated expression
             exprs.push_back(
                 "f: f { name = \"" + cname + "\"; channelName = \"" + name + "\"; src = builtins.storePath \""
-                + filename + "\"; " + extraAttrs + " }");
+                + store->printStorePath(result.storePath) + "\"; " + extraAttrs + " }");
         }
     }