Merge 14c70d0807 into 479b6b73a9

src/libstore/local-store.cc

@@ -989,19 +989,22 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
            error if a cycle is detected and roll back the
            transaction.  Cycles can only occur when a derivation
            has multiple outputs. */
-        topoSort(
+        auto topoSortResult = topoSort(paths, {[&](const StorePath & path) {
-            paths,
-            {[&](const StorePath & path) {
                                            auto i = infos.find(path);
                                            return i == infos.end() ? StorePathSet() : i->second.references;
-            }},
+                                       }});
-            {[&](const StorePath & path, const StorePath & parent) {
+
-                return BuildError(
+        std::visit(
+            overloaded{
+                [&](Cycle<StorePath> & cycle) {
+                    throw BuildError(
                         BuildResult::Failure::OutputRejected,
                         "cycle detected in the references of '%s' from '%s'",
-                    printStorePath(path),
+                        printStorePath(cycle.path),
-                    printStorePath(parent));
+                        printStorePath(cycle.parent));
-            }});
+                },
+                [](auto &) { /* Success, continue */ }},
+            topoSortResult);
 
         txn.commit();
     });

src/libstore/misc.cc

@@ -311,22 +311,25 @@ MissingPaths Store::queryMissing(const std::vector<DerivedPath> & targets)
 
 StorePaths Store::topoSortPaths(const StorePathSet & paths)
 {
-    return topoSort(
+    auto result = topoSort(paths, {[&](const StorePath & path) {
-        paths,
-        {[&](const StorePath & path) {
                                try {
                                    return queryPathInfo(path)->references;
                                } catch (InvalidPath &) {
                                    return StorePathSet();
                                }
-        }},
+                           }});
-        {[&](const StorePath & path, const StorePath & parent) {
+
-            return BuildError(
+    return std::visit(
+        overloaded{
+            [&](Cycle<StorePath> & cycle) -> StorePaths {
+                throw BuildError(
                     BuildResult::Failure::OutputRejected,
                     "cycle detected in the references of '%s' from '%s'",
-                printStorePath(path),
+                    printStorePath(cycle.path),
-                printStorePath(parent));
+                    printStorePath(cycle.parent));
-        }});
+            },
+            [](auto & sorted) { return sorted; }},
+        result);
 }
 
 std::map<DrvOutput, StorePath>

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

@@ -1473,9 +1473,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         outputStats.insert_or_assign(outputName, std::move(st));
     }
 
-    auto sortedOutputNames = topoSort(
+    auto topoSortResult = topoSort(outputsToSort, {[&](const std::string & name) {
-        outputsToSort,
-        {[&](const std::string & name) {
                                        auto orifu = get(outputReferencesIfUnregistered, name);
                                        if (!orifu)
                                            throw BuildError(
@@ -1500,16 +1498,21 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                                                },
                                            },
                                            *orifu);
-        }},
+                                   }});
-        {[&](const std::string & path, const std::string & parent) {
+
+    auto sortedOutputNames = std::visit(
+        overloaded{
+            [&](Cycle<std::string> & cycle) -> std::vector<std::string> {
                 // TODO with more -vvvv also show the temporary paths for manual inspection.
-            return BuildError(
+                throw BuildError(
                     BuildResult::Failure::OutputRejected,
                     "cycle detected in build of '%s' in the references of output '%s' from output '%s'",
                     store.printStorePath(drvPath),
-                path,
+                    cycle.path,
-                parent);
+                    cycle.parent);
-        }});
+            },
+            [](auto & sorted) { return sorted; }},
+        topoSortResult);
 
     std::reverse(sortedOutputNames.begin(), sortedOutputNames.end());
 

src/libutil/include/nix/util/topo-sort.hh

@@ -2,39 +2,61 @@
 ///@file
 
 #include "nix/util/error.hh"
+#include <variant>
 
 namespace nix {
 
+template<typename T>
+struct Cycle
+{
+    T path;
+    T parent;
+};
+
+template<typename T>
+using TopoSortResult = std::variant<std::vector<T>, Cycle<T>>;
+
 template<typename T, typename Compare>
-std::vector<T> topoSort(
+TopoSortResult<T> topoSort(std::set<T, Compare> items, std::function<std::set<T, Compare>(const T &)> getChildren)
-    std::set<T, Compare> items,
-    std::function<std::set<T, Compare>(const T &)> getChildren,
-    std::function<Error(const T &, const T &)> makeCycleError)
 {
     std::vector<T> sorted;
     decltype(items) visited, parents;
 
-    auto dfsVisit = [&](this auto & dfsVisit, const T & path, const T * parent) {
+    std::function<std::optional<Cycle<T>>(const T & path, const T * parent)> dfsVisit;
-        if (parents.count(path))
-            throw makeCycleError(path, *parent);
 
-        if (!visited.insert(path).second)
+    dfsVisit = [&](const T & path, const T * parent) -> std::optional<Cycle<T>> {
-            return;
+        if (parents.count(path)) {
+            return Cycle{path, *parent};
+        }
+
+        if (!visited.insert(path).second) {
+            return std::nullopt;
+        }
         parents.insert(path);
 
         auto references = getChildren(path);
 
         for (auto & i : references)
             /* Don't traverse into items that don't exist in our starting set. */
-            if (i != path && items.count(i))
+            if (i != path && items.count(i)) {
-                dfsVisit(i, &path);
+                auto result = dfsVisit(i, &path);
+                if (result.has_value()) {
+                    return result;
+                }
+            }
 
         sorted.push_back(path);
         parents.erase(path);
+
+        return std::nullopt;
     };
 
-    for (auto & i : items)
+    for (auto & i : items) {
-        dfsVisit(i, nullptr);
+        auto cycle = dfsVisit(i, nullptr);
+        if (cycle.has_value()) {
+            return *cycle;
+        }
+    }
 
     std::reverse(sorted.begin(), sorted.end());