feat: make cycle errors more verbose

mk/run_test.sh

@@ -18,6 +18,8 @@ fi
 run_test () {
     (cd tests && env ${TESTS_ENVIRONMENT} init.sh 2>/dev/null > /dev/null)
     log="$(cd $(dirname $1) && env ${TESTS_ENVIRONMENT} $(basename $1) 2>&1)"
+    # debug: show output of test
+    echo "$log"
     status=$?
 }
 

src/libexpr/lexer.l

@@ -2,7 +2,6 @@
 %option noyywrap
 %option never-interactive
 %option stack
-%option nodefault
 %option nounput noyy_top_state
 
 

src/libstore/build/find-cycles.hh

@@ -0,0 +1,40 @@
+#pragma once
+
+//#include "hash.hh"
+#include "path.hh"
+
+#include <string>
+#include <deque>
+
+namespace nix {
+
+// see nix/src/libstore/references.hh
+// first pass: fast on success
+//std::pair<StorePathSet, HashResult> scanForReferences(const Path & path, const StorePathSet & refs);
+//StorePathSet scanForReferences(Sink & toTee, const Path & path, const StorePathSet & refs);
+
+//typedef std::pair<std::string, std::string> StoreCycleEdge;
+// need deque to join edges
+typedef std::deque<std::string> StoreCycleEdge;
+typedef std::vector<StoreCycleEdge> StoreCycleEdgeVec;
+
+// second pass: get exact file paths of cycles
+void scanForCycleEdges(
+    const Path & path,
+    const StorePathSet & refs,
+    StoreCycleEdgeVec & edges
+);
+
+void scanForCycleEdges2(
+    std::string path,
+    const StringSet & hashes,
+    StoreCycleEdgeVec & seen,
+    std::string storePrefix
+);
+
+void transformEdgesToMultiedges(
+    StoreCycleEdgeVec & edges,
+    StoreCycleEdgeVec & multiedges
+);
+
+}

src/libstore/build/find-cycles.py

@@ -0,0 +1,77 @@
+#! /usr/bin/env python
+
+# prototype for find-cycles.cc
+
+edges = [
+  # sorted:
+  #[1, 2],
+  #[2, 3],
+  #[3, 1],
+  # unsorted:
+  [1, 2],
+  [3, 1],
+  [2, 3],
+]
+
+edges_yaml = """\
+-
+ - a-to-c.2.txt
+ - c-to-b.2.txt
+-
+ - b-to-a.2.txt
+ - a-to-c.2.txt
+-
+ - c-to-b.2.txt
+ - b-to-a.2.txt
+"""
+
+edges = []
+i = -1
+for line in edges_yaml.splitlines():
+  if line == "-":
+    i += 1
+    continue
+  try:
+    edges[i]
+  except IndexError:
+    edges.append([])
+  val = line[3:]
+  # a: gdmbqa2y7xv2sc0hf6q5c6da3cai5ygw-cyclic-outputs-b/opt/from-b-to-a.2.txt
+  # b: b-to-a
+  #val = val[112:118]
+  edges[i].append(val)
+
+#print(repr(edges)); import sys; sys.exit()
+
+multiedges = []
+
+for edge2 in edges:
+  edge2Joined = False
+  for edge1 in multiedges:
+    print(f"edge2 = {edge2}")
+    print(f"edge1 = {edge1}")
+    if edge1[-1] == edge2[0]: # edge1.back() == edge2.front()
+      # a-b + b-c -> a-b-c
+      print(f"append: edge1 = {edge1} + {edge2[1:]} = {edge1 + edge2[1:]}")
+      #edge1 = edge1 + edge2[1:] # wrong: this creates a new list
+      edge1.append(*edge2[1:])
+      print(f"-> edge1 = {edge1}")
+      edge2Joined = True
+      break
+    if edge2[-1] == edge1[0]: # edge2.back() == edge1.front()
+      # b-c + a-b -> a-b-c
+      print(f"prepend: edge1 = {edge2[:-1]} + {edge1} = {edge2[:-1] + edge1}")
+      #edge1.prepend(*edge2[:-1])
+      edge1.insert(0, *edge2[:-1])
+      print(f"-> edge1 = {edge1}")
+      edge2Joined = True
+      break
+  if not edge2Joined:
+    print(f"init: edge1 = {edge2}")
+    multiedges.append(edge2)
+
+for edge1 in multiedges:
+  print("edge1:")
+  for point in edge1:
+    print(f"  {point}")
+

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

@@ -15,6 +15,7 @@
 #include "topo-sort.hh"
 #include "callback.hh"
 #include "json-utils.hh"
+#include "find-cycles.hh" // scanForCycleEdges transformEdgesToMultiedges
 
 #include <regex>
 #include <queue>
@@ -2192,7 +2193,15 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
         outputStats.insert_or_assign(outputName, std::move(st));
     }
 
-    auto sortedOutputNames = topoSort(outputsToSort,
+    debug("calling topoSort");
+
+    std::vector<std::string> sortedOutputNames;
+
+    try {
+    // TODO indent
+    sortedOutputNames = topoSort(
+    //auto sortedOutputNames = topoSortCycles(
+        outputsToSort,
         {[&](const std::string & name) {
             auto orifu = get(outputReferencesIfUnregistered, name);
             if (!orifu)
@@ -2221,6 +2230,55 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 "cycle detected in build of '%s' in the references of output '%s' from output '%s'",
                 worker.store.printStorePath(drvPath), path, parent);
         }});
+    // TODO indent end
+    }
+    catch (Error & e) {
+        debug(format("catching cycle error: %1%") % e.what());
+
+        for (auto & sp : referenceablePaths) {
+            debug(format("analyze cycle: referenceablePaths[] = %1%") % sp.to_string());
+        }
+
+        // analyze cycle
+        StoreCycleEdgeVec edges;
+        for (auto & [outputName, _] : drv->outputs) {
+            debug(format("analyze cycle: outputName = %1%") % outputName);
+            auto actualPath = toRealPathChroot(worker.store.printStorePath(scratchOutputs.at(outputName)));
+            debug(format("analyze cycle: actualPath = %1%") % actualPath);
+            debug(format("analyze cycle: scanForCycleEdges"));
+            scanForCycleEdges(actualPath, referenceablePaths, edges);
+        }
+
+        debug(format("error: cycles detected. found %i cycle edges:") % edges.size());
+        for (auto edge : edges) {
+            debug(format("-"));
+            for (auto file : edge) {
+                debug(format(" - %s") % file);
+            }
+        }
+        // find paths in directed graph
+        // = connect adjacent edges to multiedges
+        // = transform edges to multiedges
+        // simple implementation with string compare
+        StoreCycleEdgeVec multiedges;
+        debug(format("transforming edges to multiedges"));
+        transformEdgesToMultiedges(edges, multiedges);
+        std::cout << "error: cycle detected. found " << multiedges.size() << " cycle edges:\n";
+        //for (auto multiedge : multiedges) {
+        for (size_t i = 0; i < multiedges.size(); i++) {
+            auto multiedge = multiedges[i];
+            //std::cout << "-\n";
+            std::cout << (i + 1) << ":\n";
+            for (auto file : multiedge) {
+                std::cout << " - " << file << "\n";
+            }
+        }
+        std::cout << std::flush; // "\n" does not flush like std::endl
+
+        throw e; // BuildError: cycle
+    }
+
+    debug(format("no cycles -> continue"));
 
     std::reverse(sortedOutputNames.begin(), sortedOutputNames.end());
 

src/libstore/references.cc

@@ -12,6 +12,7 @@ namespace nix {
 
 
 static size_t refLength = 32; /* characters */
+// TODO rename to hashLength?
 
 
 static void search(
@@ -83,6 +84,8 @@ StorePathSet scanForReferences(
     const StorePathSet & refs)
 {
     StringSet hashes;
+
+    // backMap: map from hash to storepath
     std::map<std::string, StorePath> backMap;
 
     for (auto & i : refs) {
@@ -108,7 +111,6 @@ StorePathSet scanForReferences(
     return found;
 }
 
-
 RewritingSink::RewritingSink(const std::string & from, const std::string & to, Sink & nextSink)
     : from(from), to(to), nextSink(nextSink)
 {

src/libstore/references.hh

@@ -3,12 +3,38 @@
 #include "hash.hh"
 #include "path.hh"
 
+#include <deque>
+
 namespace nix {
 
+// first pass: fast on success
 std::pair<StorePathSet, HashResult> scanForReferences(const Path & path, const StorePathSet & refs);
-
 StorePathSet scanForReferences(Sink & toTee, const Path & path, const StorePathSet & refs);
 
+// moved to nix/src/libstore/build/find-cycles.hh
+/*
+
+//typedef std::pair<std::string, std::string> StoreCycleEdge;
+// need deque to join edges
+typedef std::deque<std::string> StoreCycleEdge;
+typedef std::vector<StoreCycleEdge> StoreCycleEdgeVec;
+
+// second pass: get exact file paths of cycles
+void scanForCycleEdges(
+    const Path & path,
+    const StorePathSet & refs,
+    StoreCycleEdgeVec & edges
+);
+
+void scanForCycleEdges2(
+    std::string path,
+    const StringSet & hashes,
+    StoreCycleEdgeVec & seen,
+    std::string storePrefix
+);
+
+*/
+
 class RefScanSink : public Sink
 {
     StringSet hashes;

src/libutil/topo-sort.hh

@@ -39,4 +39,89 @@ std::vector<T> topoSort(std::set<T> items,
     return sorted;
 }
 
+
+
+template<typename T>
+std::vector<T> topoSortCycles(std::set<T> items,
+        std::function<std::set<T>(const T &)> getChildren,
+        std::function<void(const T &, const std::vector<T> *)> handleCycle)
+{
+    std::vector<T> sorted;
+    std::set<T> visited, parents;
+
+    std::function<void(const T & path, const T * parent)> dfsVisit;
+
+    dfsVisit = [&](const T & path, const T * parent) {
+        debug(format("topoSortCycles: path = %1%") % path);
+        if (parent != nullptr) {
+            debug(format("topoSortCycles: args: *parent = %1%") % *parent);
+        }
+        else {
+            debug(format("topoSortCycles: args: parent = nullptr") % *parent);
+        }
+        debug(format("topoSortCycles: state: sorted = %1%") % *parent);
+
+        if (parents.count(path)) {
+            debug(format("topoSortCycles: found cycle"));
+            for (auto & i : parents) {
+                debug(format("topoSortCycles: parents[] = %1%") % i);
+            }
+            for (auto & i : sorted) {
+                debug(format("topoSortCycles: sorted[] = %1%") % i);
+            }
+            debug(format("topoSortCycles: calling handleCycle"));
+            // NOTE sorted can be larger than necessary.
+            // ex:
+            // sorted: a b c
+            // path: b
+            // -> cycle b c b
+            // -> a is not in cycle
+            // -> end of sorted is in cycle
+            std::vector<T> * sortedPtr = &sorted;
+            handleCycle(path, const_cast<const std::vector<T> *>(sortedPtr));
+        }
+
+        if (!visited.insert(path).second) {
+            debug(format("topoSortCycles: !visited.insert(path).second == true -> return"));
+            return;
+        }
+        debug(format("topoSortCycles: !visited.insert(path).second == false -> continue"));
+
+        parents.insert(path);
+        sorted.push_back(path);
+
+        debug(format("topoSortCycles: calling getChildren"));
+        std::set<T> references = getChildren(path);
+
+        for (auto & i : references) {
+            debug(format("topoSortCycles: references[] = %1%") % i);
+        }
+
+        // recursion
+        for (auto & i : references)
+            /* Don't traverse into items that don't exist in our starting set. */
+            if (i != path && items.count(i)) {
+                debug(format("topoSortCycles: dfsVisit? yes"));
+                dfsVisit(i, &path);
+            }
+            else {
+                debug(format("topoSortCycles: dfsVisit? no"));
+            }
+
+        debug(format("topoSortCycles: done path: %1%") % path);
+        //sorted.push_back(path); // moved before recursion, so we have sorted in handleCycle
+        parents.erase(path);
+        for (auto & i : sorted) {
+            debug(format("topoSortCycles: done: sorted[] = %1%") % i);
+        }
+    };
+
+    for (auto & i : items)
+        dfsVisit(i, nullptr);
+
+    std::reverse(sorted.begin(), sorted.end());
+
+    return sorted;
+}
+
 }