Introduce ReferrersStore::comptuteFSCoClosure

As remarked in the previous commit, it is bad to call `require` except
for on the boundary of the code base, yet we were in
`Store::computeFSClosure.`

The solution is to have more methods instead of the `flipDirection
parameter. The referrers not reference methods are moved to
`ReferrersStore`. This solves the issue.

perl/lib/Nix/CopyClosure.pm

@@ -15,7 +15,7 @@ sub copyToOpen {
     $useSubstitutes = 0 if $dryRun || !defined $useSubstitutes;
 
     # Get the closure of this path.
-    my @closure = reverse(topoSortPaths(computeFSClosure(0, $includeOutputs,
+    my @closure = reverse(topoSortPaths(computeFSClosure($includeOutputs,
         map { followLinksToStorePath $_ } @{$storePaths})));
 
     # Send the "query valid paths" command with the "lock" option

perl/lib/Nix/Store.xs

@@ -144,12 +144,12 @@ SV * queryPathFromHashPart(char * hashPart)
         }
 
 
-SV * computeFSClosure(int flipDirection, int includeOutputs, ...)
+SV * computeFSClosure(int includeOutputs, ...)
     PPCODE:
         try {
             StorePathSet paths;
             for (int n = 2; n < items; ++n)
-                store()->computeFSClosure(store()->parseStorePath(SvPV_nolen(ST(n))), paths, flipDirection, includeOutputs);
+                store()->computeFSClosure(store()->parseStorePath(SvPV_nolen(ST(n))), paths, includeOutputs);
             for (auto & i : paths)
                 XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(i).c_str(), 0)));
         } catch (Error & e) {

src/libstore/gc.cc

@@ -710,7 +710,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
                 try {
                     StorePathSet closure;
                     computeFSClosure(*path, closure,
-                        /* flipDirection */ false, gcKeepOutputs, gcKeepDerivations);
+                        gcKeepOutputs, gcKeepDerivations);
                     for (auto & p : closure)
                         alive.insert(p);
                 } catch (InvalidPath &) { }

src/libstore/legacy-ssh-store.cc

@@ -303,11 +303,11 @@ void LegacySSHStore::buildPaths(const std::vector<DerivedPath> & drvPaths, Build
 
 
 void LegacySSHStore::computeFSClosure(const StorePathSet & paths,
-    StorePathSet & out, bool flipDirection,
+    StorePathSet & out,
     bool includeOutputs, bool includeDerivers)
 {
-    if (flipDirection || includeDerivers) {
-        Store::computeFSClosure(paths, out, flipDirection, includeOutputs, includeDerivers);
+    if (includeDerivers) {
+        Store::computeFSClosure(paths, out, includeOutputs, includeDerivers);
         return;
     }
 

src/libstore/legacy-ssh-store.hh

@@ -98,7 +98,7 @@ public:
     { unsupported("repairPath"); }
 
     void computeFSClosure(const StorePathSet & paths,
-        StorePathSet & out, bool flipDirection = false,
+        StorePathSet & out,
         bool includeOutputs = false, bool includeDerivers = false) override;
 
     StorePathSet queryValidPaths(const StorePathSet & paths,

src/libstore/misc.cc

@@ -14,56 +14,18 @@
 
 namespace nix {
 
-void Store::computeFSClosure(const StorePathSet & startPaths,
-    StorePathSet & paths_, bool flipDirection, bool includeOutputs, bool includeDerivers)
-{
-    std::function<std::set<StorePath>(const StorePath & path, std::future<ref<const ValidPathInfo>> &)> queryDeps;
-    if (flipDirection) {
-        // FIXME It is not good to do partial downcasts "deep" in the
-        // code, we should only do this in "boundary" code like the CLI
-        // or protocol handlers.
-        auto & referrersStore = require<ReferrersStore>(*this);
-        queryDeps = [&](const StorePath& path,
-                        std::future<ref<const ValidPathInfo>> & fut) {
-            StorePathSet res;
-            StorePathSet referrers;
-            referrersStore.queryReferrers(path, referrers);
-            for (auto& ref : referrers)
-                if (ref != path)
-                    res.insert(ref);
-
-            if (includeOutputs)
-                for (auto& i : referrersStore.queryValidDerivers(path))
-                    res.insert(i);
-
-            if (includeDerivers && path.isDerivation())
-                for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path))
-                    if (maybeOutPath && isValidPath(*maybeOutPath))
-                        res.insert(*maybeOutPath);
-            return res;
-        };
-    } else
-        queryDeps = [&](const StorePath& path,
-                        std::future<ref<const ValidPathInfo>> & fut) {
-            StorePathSet res;
-            auto info = fut.get();
-            for (auto& ref : info->references)
-                if (ref != path)
-                    res.insert(ref);
-
-            if (includeOutputs && path.isDerivation())
-                for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path))
-                    if (maybeOutPath && isValidPath(*maybeOutPath))
-                        res.insert(*maybeOutPath);
-
-            if (includeDerivers && info->deriver && isValidPath(*info->deriver))
-                res.insert(*info->deriver);
-            return res;
-        };
+typedef std::set<StorePath> QueryDeps(const StorePath & path, std::future<ref<const ValidPathInfo>> &);
 
+static void computeClosure(
+    Store & store,
+    std::function<QueryDeps> queryDeps,
+    const StorePathSet & startPaths,
+    StorePathSet & paths_,
+    bool includeOutputs, bool includeDerivers)
+{
     computeClosure<StorePath>(
         startPaths, paths_,
-        [&](const StorePath& path,
+        [&](const StorePath & path,
             std::function<void(std::promise<std::set<StorePath>>&)>
                 processEdges) {
             std::promise<std::set<StorePath>> promise;
@@ -76,17 +38,75 @@ void Store::computeFSClosure(const StorePathSet & startPaths,
                             promise.set_exception(std::current_exception());
                         }
                     };
-            queryPathInfo(path, getDependencies);
+            store.queryPathInfo(path, getDependencies);
             processEdges(promise);
         });
 }
 
+void ReferrersStore::computeFSCoClosure(const StorePathSet & startPaths,
+    StorePathSet & paths_, bool includeOutputs, bool includeDerivers)
+{
+    std::function<QueryDeps> queryDeps;
+    queryDeps = [&](const StorePath & path,
+                    std::future<ref<const ValidPathInfo>> & fut) {
+        StorePathSet res;
+        StorePathSet referrers;
+        queryReferrers(path, referrers);
+        for (auto& ref : referrers)
+            if (ref != path)
+                res.insert(ref);
+
+        if (includeOutputs)
+            for (auto& i : queryValidDerivers(path))
+                res.insert(i);
+
+        if (includeDerivers && path.isDerivation())
+            for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path))
+                if (maybeOutPath && isValidPath(*maybeOutPath))
+                    res.insert(*maybeOutPath);
+        return res;
+    };
+    computeClosure(*this, queryDeps, startPaths, paths_, includeOutputs, includeDerivers);
+}
+
+void Store::computeFSClosure(const StorePathSet & startPaths,
+    StorePathSet & paths_, bool includeOutputs, bool includeDerivers)
+{
+    std::function<QueryDeps> queryDeps;
+    queryDeps = [&](const StorePath& path,
+                    std::future<ref<const ValidPathInfo>> & fut) {
+        StorePathSet res;
+        auto info = fut.get();
+        for (auto& ref : info->references)
+            if (ref != path)
+                res.insert(ref);
+
+        if (includeOutputs && path.isDerivation())
+            for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path))
+                if (maybeOutPath && isValidPath(*maybeOutPath))
+                    res.insert(*maybeOutPath);
+
+        if (includeDerivers && info->deriver && isValidPath(*info->deriver))
+            res.insert(*info->deriver);
+        return res;
+    };
+    computeClosure(*this, queryDeps, startPaths, paths_, includeOutputs, includeDerivers);
+}
+
+void ReferrersStore::computeFSCoClosure(const StorePath & startPath,
+    StorePathSet & paths_, bool includeOutputs, bool includeDerivers)
+{
+    StorePathSet paths;
+    paths.insert(startPath);
+    computeFSCoClosure(paths, paths_, includeOutputs, includeDerivers);
+}
+
 void Store::computeFSClosure(const StorePath & startPath,
-    StorePathSet & paths_, bool flipDirection, bool includeOutputs, bool includeDerivers)
+    StorePathSet & paths_, bool includeOutputs, bool includeDerivers)
 {
     StorePathSet paths;
     paths.insert(startPath);
-    computeFSClosure(paths, paths_, flipDirection, includeOutputs, includeDerivers);
+    computeFSClosure(paths, paths_, includeOutputs, includeDerivers);
 }
 
 

src/libstore/referrers-store.hh

@@ -33,6 +33,24 @@ struct ReferrersStore : virtual VisibleStore
      * of `path`.
      */
     virtual void queryReferrers(const StorePath & path, StorePathSet & referrers) = 0;
+
+    /**
+     * @param [out] out Place in here the set of all store paths in the
+     * file system co-closure of `storePath`; that is, all paths than
+     * directly or indirectly refer from it. `out` is not cleared.
+     *
+     * Whereas `Store::computeFSClosure` uses the `references` relation,
+     * this function uses the dual of it which is the `referrers`
+     * relation.
+     */
+    virtual void computeFSCoClosure(const StorePathSet & paths,
+        StorePathSet & out,
+        bool includeOutputs = false, bool includeDerivers = false);
+
+    void computeFSCoClosure(const StorePath & path,
+        StorePathSet & out,
+        bool includeOutputs = false, bool includeDerivers = false);
+
 };
 
 }

src/libstore/store-api.hh

@@ -615,11 +615,11 @@ public:
      * returned.
      */
     virtual void computeFSClosure(const StorePathSet & paths,
-        StorePathSet & out, bool flipDirection = false,
+        StorePathSet & out,
         bool includeOutputs = false, bool includeDerivers = false);
 
     void computeFSClosure(const StorePath & path,
-        StorePathSet & out, bool flipDirection = false,
+        StorePathSet & out,
         bool includeOutputs = false, bool includeDerivers = false);
 
     /**

src/nix-store/nix-store.cc

@@ -356,7 +356,7 @@ static void opQuery(Strings opFlags, Strings opArgs)
             for (auto & i : opArgs) {
                 auto ps = maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise);
                 for (auto & j : ps) {
-                    if (query == qRequisites) store->computeFSClosure(j, paths, false, includeOutputs);
+                    if (query == qRequisites) store->computeFSClosure(j, paths, includeOutputs);
                     else if (query == qReferences) {
                         for (auto & p : store->queryPathInfo(j)->references)
                             paths.insert(p);
@@ -368,7 +368,10 @@ static void opQuery(Strings opFlags, Strings opArgs)
                         for (auto & i : tmp)
                             paths.insert(i);
                     }
-                    else if (query == qReferrersClosure) store->computeFSClosure(j, paths, true);
+                    else if (query == qReferrersClosure) {
+                        auto & referrersStore = require<ReferrersStore>(*store);
+                        referrersStore.computeFSCoClosure(j, paths);
+                    }
                 }
             }
             auto sorted = store->topoSortPaths(paths);
@@ -458,16 +461,17 @@ static void opQuery(Strings opFlags, Strings opArgs)
         }
 
         case qRoots: {
+            auto & referrersStore = require<ReferrersStore>(*store);
+            auto & gcStore = require<GcStore>(*store);
             StorePathSet args;
             for (auto & i : opArgs)
                 for (auto & p : maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise))
                     args.insert(p);
 
             StorePathSet referrers;
-            store->computeFSClosure(
-                args, referrers, true, settings.gcKeepOutputs, settings.gcKeepDerivations);
+            referrersStore.computeFSCoClosure(
+                args, referrers, settings.gcKeepOutputs, settings.gcKeepDerivations);
 
-            auto & gcStore = require<GcStore>(*store);
             Roots roots = gcStore.findRoots(false);
             for (auto & [target, links] : roots)
                 if (referrers.find(target) != referrers.end())