From 0e2c090dc5aec298ef6726bc9d6c103cfc59af63 Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Thu, 26 Aug 2010 22:52:57 -0400 Subject: [PATCH] Dependencies - Added check for circular dependencies. --- src/Task.cpp | 3 ++ src/dependency.cpp | 94 +++++++++++++++++++++++++++++----------- src/main.h | 2 +- src/recur.cpp | 1 + src/tests/dependencies.t | 8 ++-- 5 files changed, 78 insertions(+), 30 deletions(-) diff --git a/src/Task.cpp b/src/Task.cpp index 274cd781a..9d5187503 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -493,6 +493,9 @@ void Task::addDependency (int id) } else set ("depends", uuid); + + if (dependencyIsCircular (*this)) + throw std::string ("Circular dependency detected and disallowed."); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/dependency.cpp b/src/dependency.cpp index a049d7cea..1a4eca69c 100644 --- a/src/dependency.cpp +++ b/src/dependency.cpp @@ -24,7 +24,11 @@ // USA // //////////////////////////////////////////////////////////////////////////////// + +#include +#include #include +#include extern Context context; @@ -34,6 +38,7 @@ extern Context context; // void dependencyRepairChain (); // bool dependencyRepairConfirm (); // void dependencyNag (); +static bool followUpstream (const Task&, const Task&, const std::vector &, std::vector &); //////////////////////////////////////////////////////////////////////////////// // All it takes to be blocked is to depend on another task. @@ -62,35 +67,72 @@ bool dependencyIsBlocking (Task& task) } //////////////////////////////////////////////////////////////////////////////// -// Follow each of the given task's dependencies to the end of the chain, and if -// any duplicates show up, or the chain length exceeds N, stop. - -/* - Linear: - 1->2 - - 1->2->3->4 - `->5->6 - `->7 - - Circular: - 1->1 - - 1->2->1 - - 1->2->3 - `->1 - - Algorithm: - 1. Generate a subset of all task that have dependencies - 2. Find the heads of all the chains - 3. For each unique chain head - 3.1 Walk the chain recording IDs - 3.2 Duplicate ID => circular -*/ +// Terminology: +// --> if a depends on b, then it can be said that a --> b +// Head if a --> b, then b is the head +// Tail if a --> b, then a is the tail +// +// Algorithm: +// Find all tails, ie tasks that have dependencies, with no other tasks that +// are dependent on them. +// +// For each tail: +// follow the chain, recording all linkages, ie a --> b, b --> c. If a +// linkage appears that has already occurred in this chain => circularity. +// bool dependencyIsCircular (Task& task) { + std::vector links; + const std::vector & all = context.tdb.getAllPending (); + return followUpstream (task, task, all, links); +} + +//////////////////////////////////////////////////////////////////////////////// +// To follow dependencies upstream, follow the heads. +static bool followUpstream ( + const Task& task, + const Task& original, + const std::vector & all, + std::vector & links) +{ + if (task.has ("depends")) + { + std::vector uuids; + split (uuids, task.get ("depends"), ','); + + std::vector ::iterator outer; + for (outer = uuids.begin (); outer != uuids.end (); ++outer) + { + // Check that link has not already been seen. + std::string link = task.get ("uuid") + " -> " + *outer; + if (std::find (links.begin (), links.end (), link) != links.end ()) + return true; + + links.push_back (link); + + // Recurse up the chain. + std::vector ::const_iterator inner; + for (inner = all.begin (); inner != all.end (); ++inner) + { + if (*outer == inner->get ("uuid")) + { + if (*outer == original.get ("uuid")) + { + if (followUpstream (task, original, all, links)) + return true; + } + else + { + if (followUpstream (*inner, original, all, links)) + return true; + } + + break; + } + } + } + } return false; } diff --git a/src/main.h b/src/main.h index 4bf60f73f..20a4487f6 100644 --- a/src/main.h +++ b/src/main.h @@ -133,7 +133,7 @@ int handleExportYAML (std::string &); // dependency.cpp bool dependencyIsBlocked (Task&); bool dependencyIsBlocking (Task&); -bool dependencyCheckCircular (Task&); +bool dependencyIsCircular (Task&); // list template /////////////////////////////////////////////////////////////////////////////// diff --git a/src/recur.cpp b/src/recur.cpp index 6e8d33d90..f3fc53a2d 100644 --- a/src/recur.cpp +++ b/src/recur.cpp @@ -24,6 +24,7 @@ // USA // //////////////////////////////////////////////////////////////////////////////// + #include #include #include diff --git a/src/tests/dependencies.t b/src/tests/dependencies.t index 106c0e400..761b96f5c 100755 --- a/src/tests/dependencies.t +++ b/src/tests/dependencies.t @@ -28,7 +28,7 @@ use strict; use warnings; -use Test::More tests => 35; +use Test::More tests => 37; # Create the rc file. if (open my $fh, '>', 'dep.rc') @@ -73,7 +73,8 @@ like ($output, qr/Modified 0 tasks\./, 'dependencies - add already existing depe # t 1 dep:2; t 2 dep:1 => error $output = qx{../task rc:dep.rc 2 dep:1}; -unlike ($output, qr/Modified 1 task\./, 'dependencies - trivial circular'); +like ($output, qr/Circular dependency detected and disallowed\./, 'dependencies - trivial circular'); +unlike ($output, qr/Modified 1 task\./, 'dependencies - trivial circular'); unlink 'pending.data'; ok (!-r 'pending.data', 'Removed pending.data for a fresh start'); @@ -88,7 +89,8 @@ qx{../task rc:dep.rc 5 dep:4; ../task rc:dep.rc 4 dep:3; ../task rc:dep.rc 3 dep # 5 dep 4 dep 3 dep 2 dep 1 dep 5 => error $output = qx{../task rc:dep.rc 1 dep:5}; -unlike ($output, qr/Modified 1 task\./, 'dependencies - nontrivial circular'); +like ($output, qr/Circular dependency detected and disallowed\./, 'dependencies - nontrivial circular'); +unlike ($output, qr/Modified 1 task\./, 'dependencies - nontrivial circular'); unlink 'pending.data'; ok (!-r 'pending.data', 'Removed pending.data for a fresh start');