diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c6aeaf0d2..aaee54366 100644 --- a/src/libstore/local-store.cc +++ b/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( - 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( - BuildResult::Failure::OutputRejected, - "cycle detected in the references of '%s' from '%s'", - printStorePath(path), - printStorePath(parent)); - }}); + auto topoSortResult = topoSort(paths, {[&](const StorePath & path) { + auto i = infos.find(path); + return i == infos.end() ? StorePathSet() : i->second.references; + }}); + + std::visit( + overloaded{ + [&](Cycle & cycle) { + throw BuildError( + BuildResult::Failure::OutputRejected, + "cycle detected in the references of '%s' from '%s'", + printStorePath(cycle.path), + printStorePath(cycle.parent)); + }, + [](auto &) { /* Success, continue */ }}, + topoSortResult); txn.commit(); }); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 8b2a7287e..6f6b2354d 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -311,22 +311,25 @@ MissingPaths Store::queryMissing(const std::vector & targets) StorePaths Store::topoSortPaths(const StorePathSet & paths) { - return topoSort( - paths, - {[&](const StorePath & path) { - try { - return queryPathInfo(path)->references; - } catch (InvalidPath &) { - return StorePathSet(); - } - }}, - {[&](const StorePath & path, const StorePath & parent) { - return BuildError( - BuildResult::Failure::OutputRejected, - "cycle detected in the references of '%s' from '%s'", - printStorePath(path), - printStorePath(parent)); - }}); + auto result = topoSort(paths, {[&](const StorePath & path) { + try { + return queryPathInfo(path)->references; + } catch (InvalidPath &) { + return StorePathSet(); + } + }}); + + return std::visit( + overloaded{ + [&](Cycle & cycle) -> StorePaths { + throw BuildError( + BuildResult::Failure::OutputRejected, + "cycle detected in the references of '%s' from '%s'", + printStorePath(cycle.path), + printStorePath(cycle.parent)); + }, + [](auto & sorted) { return sorted; }}, + result); } std::map diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index c2ef730dc..6eedc5fa2 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1473,43 +1473,46 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() outputStats.insert_or_assign(outputName, std::move(st)); } - auto sortedOutputNames = topoSort( - outputsToSort, - {[&](const std::string & name) { - auto orifu = get(outputReferencesIfUnregistered, name); - if (!orifu) + auto topoSortResult = topoSort(outputsToSort, {[&](const std::string & name) { + auto orifu = get(outputReferencesIfUnregistered, name); + if (!orifu) + throw BuildError( + BuildResult::Failure::OutputRejected, + "no output reference for '%s' in build of '%s'", + name, + store.printStorePath(drvPath)); + return std::visit( + overloaded{ + /* Since we'll use the already installed versions of these, we + can treat them as leaves and ignore any references they + have. */ + [&](const AlreadyRegistered &) { return StringSet{}; }, + [&](const PerhapsNeedToRegister & refs) { + StringSet referencedOutputs; + /* FIXME build inverted map up front so no quadratic waste here */ + for (auto & r : refs.refs) + for (auto & [o, p] : scratchOutputs) + if (r == p) + referencedOutputs.insert(o); + return referencedOutputs; + }, + }, + *orifu); + }}); + + auto sortedOutputNames = std::visit( + overloaded{ + [&](Cycle & cycle) -> std::vector { + // TODO with more -vvvv also show the temporary paths for manual inspection. throw BuildError( BuildResult::Failure::OutputRejected, - "no output reference for '%s' in build of '%s'", - name, - store.printStorePath(drvPath)); - return std::visit( - overloaded{ - /* Since we'll use the already installed versions of these, we - can treat them as leaves and ignore any references they - have. */ - [&](const AlreadyRegistered &) { return StringSet{}; }, - [&](const PerhapsNeedToRegister & refs) { - StringSet referencedOutputs; - /* FIXME build inverted map up front so no quadratic waste here */ - for (auto & r : refs.refs) - for (auto & [o, p] : scratchOutputs) - if (r == p) - referencedOutputs.insert(o); - return referencedOutputs; - }, - }, - *orifu); - }}, - {[&](const std::string & path, const std::string & parent) { - // TODO with more -vvvv also show the temporary paths for manual inspection. - return BuildError( - BuildResult::Failure::OutputRejected, - "cycle detected in build of '%s' in the references of output '%s' from output '%s'", - store.printStorePath(drvPath), - path, - parent); - }}); + "cycle detected in build of '%s' in the references of output '%s' from output '%s'", + store.printStorePath(drvPath), + cycle.path, + cycle.parent); + }, + [](auto & sorted) { return sorted; }}, + topoSortResult); std::reverse(sortedOutputNames.begin(), sortedOutputNames.end()); diff --git a/src/libutil/include/nix/util/topo-sort.hh b/src/libutil/include/nix/util/topo-sort.hh index aaf5dff16..285c34316 100644 --- a/src/libutil/include/nix/util/topo-sort.hh +++ b/src/libutil/include/nix/util/topo-sort.hh @@ -2,39 +2,61 @@ ///@file #include "nix/util/error.hh" +#include namespace nix { +template +struct Cycle +{ + T path; + T parent; +}; + +template +using TopoSortResult = std::variant, Cycle>; + template -std::vector topoSort( - std::set items, - std::function(const T &)> getChildren, - std::function makeCycleError) +TopoSortResult topoSort(std::set items, std::function(const T &)> getChildren) { std::vector sorted; decltype(items) visited, parents; - auto dfsVisit = [&](this auto & dfsVisit, const T & path, const T * parent) { - if (parents.count(path)) - throw makeCycleError(path, *parent); + std::function>(const T & path, const T * parent)> dfsVisit; - if (!visited.insert(path).second) - return; + dfsVisit = [&](const T & path, const T * parent) -> std::optional> { + 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)) - dfsVisit(i, &path); + if (i != path && items.count(i)) { + 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) - dfsVisit(i, nullptr); + for (auto & i : items) { + auto cycle = dfsVisit(i, nullptr); + if (cycle.has_value()) { + return *cycle; + } + } std::reverse(sorted.begin(), sorted.end());