From 56dbca9a98103d3cf5db6640360023c94fbe934a Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Tue, 28 Oct 2025 05:53:52 +0000 Subject: [PATCH] feat(libstore): add findCycles() to DependencyGraph Adds cycle detection to DependencyGraph using DFS with back-edge detection. This will be used by the cycle detection feature for build errors. Each cycle is represented as a path that starts and ends at the same node, e.g., [A, B, C, A]. --- src/libstore-tests/dependency-graph.cc | 60 +++++++++++++++++++ .../nix/store/dependency-graph-impl.hh | 56 +++++++++++++++++ .../include/nix/store/dependency-graph.hh | 7 +++ 3 files changed, 123 insertions(+) diff --git a/src/libstore-tests/dependency-graph.cc b/src/libstore-tests/dependency-graph.cc index 60dbb1717..eecaf14e4 100644 --- a/src/libstore-tests/dependency-graph.cc +++ b/src/libstore-tests/dependency-graph.cc @@ -94,4 +94,64 @@ TEST(DependencyGraph, EmptyGraph) EXPECT_EQ(depGraph.getAllNodes().size(), 0); } +/** + * Parameters for cycle detection tests + */ +struct FindCyclesParams +{ + std::string description; + std::vector> inputEdges; + std::vector> expectedCycles; +}; + +class FindCyclesTest : public ::testing::TestWithParam +{}; + +namespace { +bool compareCycles(const std::vector & a, const std::vector & b) +{ + if (a.size() != b.size()) + return a.size() < b.size(); + return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end()); +} +} // namespace + +TEST_P(FindCyclesTest, FindCycles) +{ + const auto & params = GetParam(); + + FilePathGraph depGraph; + for (const auto & [from, to] : params.inputEdges) { + depGraph.addEdge(from, to); + } + + auto actualCycles = depGraph.findCycles(); + EXPECT_EQ(actualCycles.size(), params.expectedCycles.size()); + + std::ranges::sort(actualCycles, compareCycles); + auto expectedCycles = params.expectedCycles; + std::ranges::sort(expectedCycles, compareCycles); + + EXPECT_EQ(actualCycles, expectedCycles); +} + +INSTANTIATE_TEST_CASE_P( + FindCycles, + FindCyclesTest, + ::testing::Values( + FindCyclesParams{"empty input", {}, {}}, + FindCyclesParams{"single edge no cycle", {{"a", "b"}}, {}}, + FindCyclesParams{"simple cycle", {{"a", "b"}, {"b", "a"}}, {{"a", "b", "a"}}}, + FindCyclesParams{"three node cycle", {{"a", "b"}, {"b", "c"}, {"c", "a"}}, {{"a", "b", "c", "a"}}}, + FindCyclesParams{ + "four node cycle", {{"a", "b"}, {"b", "c"}, {"c", "d"}, {"d", "a"}}, {{"a", "b", "c", "d", "a"}}}, + FindCyclesParams{ + "multiple disjoint cycles", + {{"a", "b"}, {"b", "a"}, {"c", "d"}, {"d", "c"}}, + {{"a", "b", "a"}, {"c", "d", "c"}}}, + FindCyclesParams{"cycle with extra edges", {{"a", "b"}, {"b", "a"}, {"c", "d"}}, {{"a", "b", "a"}}}, + FindCyclesParams{"self-loop", {{"a", "a"}}, {{"a", "a"}}}, + FindCyclesParams{"chain no cycle", {{"a", "b"}, {"b", "c"}, {"c", "d"}}, {}}, + FindCyclesParams{"cycle with tail", {{"x", "a"}, {"a", "b"}, {"b", "c"}, {"c", "a"}}, {{"a", "b", "c", "a"}}})); + } // namespace nix diff --git a/src/libstore/include/nix/store/dependency-graph-impl.hh b/src/libstore/include/nix/store/dependency-graph-impl.hh index df5b03303..4e94db29a 100644 --- a/src/libstore/include/nix/store/dependency-graph-impl.hh +++ b/src/libstore/include/nix/store/dependency-graph-impl.hh @@ -17,6 +17,7 @@ #include "nix/util/error.hh" #include +#include #include #include @@ -222,6 +223,61 @@ DependencyGraph::getEdgeProperty(const NodeId & from, cons return graph[edge]; } +template +std::vector> DependencyGraph::findCycles() const +{ + using vertex_descriptor = typename boost::graph_traits::vertex_descriptor; + using edge_descriptor = typename boost::graph_traits::edge_descriptor; + + std::vector> cycleDescriptors; + std::vector dfsPath; + + // Custom DFS visitor to detect back edges and extract cycles + class CycleFinder : public boost::default_dfs_visitor + { + public: + std::vector> & cycles; + std::vector & dfsPath; + + CycleFinder(std::vector> & cycles, std::vector & dfsPath) + : cycles(cycles) + , dfsPath(dfsPath) + { + } + + void discover_vertex(vertex_descriptor v, const Graph & g) + { + dfsPath.push_back(v); + } + + void finish_vertex(vertex_descriptor v, const Graph & g) + { + if (!dfsPath.empty() && dfsPath.back() == v) { + dfsPath.pop_back(); + } + } + + void back_edge(edge_descriptor e, const Graph & g) + { + auto target = boost::target(e, g); + auto cycleStart = std::ranges::find(dfsPath, target); + std::vector cycle(cycleStart, dfsPath.end()); + cycle.push_back(target); + cycles.push_back(std::move(cycle)); + } + }; + + CycleFinder visitor(cycleDescriptors, dfsPath); + boost::depth_first_search(graph, boost::visitor(visitor)); + + // Convert vertex_descriptors to NodeIds using ranges + return cycleDescriptors | std::views::transform([&](const auto & cycleVerts) { + return cycleVerts | std::views::transform([&](auto v) { return getNodeId(v); }) + | std::ranges::to>(); + }) + | std::ranges::to(); +} + template std::vector DependencyGraph::getAllNodes() const { diff --git a/src/libstore/include/nix/store/dependency-graph.hh b/src/libstore/include/nix/store/dependency-graph.hh index fb203c6e2..e32be07ce 100644 --- a/src/libstore/include/nix/store/dependency-graph.hh +++ b/src/libstore/include/nix/store/dependency-graph.hh @@ -5,6 +5,7 @@ #include "nix/util/canon-path.hh" #include +#include #include #include @@ -137,6 +138,12 @@ public: std::vector getAllNodes() const; + /** + * Find all cycles in the graph using DFS. + * Returns vector of cycles, each represented as a path that starts and ends at the same node. + */ + std::vector> findCycles() const; + size_t numVertices() const { return boost::num_vertices(graph);