Dependencies

- Improved error message when entering "task 1 dep:2; task 1 dep:2".
- Documented circularity checking.
- Stubbed dependencyChainBroken ().
- Stubbed dependencyNag ().
- Improved existing unit tests, added more.

src/Task.cpp

@@ -477,6 +477,7 @@ void Task::addDependency (int id)
   if (id == this->id)
     throw std::string ("A task cannot be dependent on itself.");
 
+  // Check for extant dependency.
   std::string uuid = context.tdb.uuid (id);
   if (uuid == "")
   {
@@ -485,15 +486,23 @@ void Task::addDependency (int id)
     throw s.str ();
   }
 
+  // Store the dependency.
   std::string depends = get ("depends");
   if (depends.length ())
   {
     if (depends.find (uuid) == std::string::npos)
       set ("depends", depends + "," + uuid);
+    else
+    {
+      std::stringstream out;
+      out << "Task " << this->id << " already depends on task " << id << ".";
+      throw out.str ();
+    }
   }
   else
     set ("depends", uuid);
 
+  // Prevent circular dependencies.
   if (dependencyIsCircular (*this))
     throw std::string ("Circular dependency detected and disallowed.");
 }

src/dependency.cpp

@@ -33,11 +33,6 @@
 extern Context context;
 
 ////////////////////////////////////////////////////////////////////////////////
-// void dependencyCheckDangling ();
-// bool dependencyRepairNeeded ();
-// void dependencyRepairChain ();
-// bool dependencyRepairConfirm ();
-// void dependencyNag ();
 static bool followUpstream (const Task&, const Task&, const std::vector <Task>&, std::vector <std::string>&);
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -73,13 +68,9 @@ bool dependencyIsBlocking (Task& task)
 //   Tail   if a --> b, then a is the tail
 //
 // Algorithm:
-//   Find all tails, ie tasks that have dependencies, with no other tasks that
+//   Keep walking the chain, recording the links (a --> b, b --> c, ...) until
-//   are dependent on them.
+//   either the end of the chain is found (therefore not circular), or the chain
-//
+//   loops and a repeat link is spotted (therefore circular).
-//   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 <std::string> links;
@@ -89,7 +80,8 @@ bool dependencyIsCircular (Task& task)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// To follow dependencies upstream, follow the heads.
+// Returns true if a task is encountered twice in a chain walk, and therefore
+// indicates circularity.  Recursive.
 static bool followUpstream (
   const Task& task,
   const Task& original,
@@ -105,6 +97,9 @@ static bool followUpstream (
     for (outer = uuids.begin (); outer != uuids.end (); ++outer)
     {
       // Check that link has not already been seen.
+
+      // This is the actual circularity check - the rest of this function is
+      // just chain-walking.
       std::string link = task.get ("uuid") + " -> " + *outer;
       if (std::find (links.begin (), links.end (), link) != links.end ())
         return true;
@@ -117,6 +112,8 @@ static bool followUpstream (
       {
         if (*outer == inner->get ("uuid"))
         {
+          // Use the newly modified "task", not "*inner", which is the old
+          // unmodified version.
           if (*outer == original.get ("uuid"))
           {
             if (followUpstream (task, original, all, links))
@@ -138,3 +135,28 @@ static bool followUpstream (
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// Determine whether a dependency chain is being broken, assuming that 'task' is
+// either completed or deleted.
+bool dependencyChainBroken (Task& task)
+{
+  if (task.has ("depends"))
+  {
+    std::cout << "# chain broken\n";
+    return true;
+  }
+
+  return false;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Generate a nag message if a dependency chain is being violated.
+void dependencyNag (Task& task)
+{
+  std::cout << "# dependencyNag "
+            << task.id
+            << " "
+            << task.get ("uuid")
+            << "\n";
+}
+
+////////////////////////////////////////////////////////////////////////////////

src/main.h

@@ -134,6 +134,8 @@ int handleExportYAML (std::string &);
 bool dependencyIsBlocked (Task&);
 bool dependencyIsBlocking (Task&);
 bool dependencyIsCircular (Task&);
+bool dependencyChainBroken (Task&);
+void dependencyNag (Task&);
 
 // list template
 ///////////////////////////////////////////////////////////////////////////////

src/tests/dependencies.t

@@ -28,7 +28,7 @@
 
 use strict;
 use warnings;
-use Test::More tests => 37;
+use Test::More tests => 39;
 
 # Create the rc file.
 if (open my $fh, '>', 'dep.rc')
@@ -69,7 +69,12 @@ like ($output, qr/This task is blocking\s+1 One\nUUID/, 'dependencies - trivial
 
 # t 1 dep:2 (again)
 $output = qx{../task rc:dep.rc 1 dep:2};
-like ($output, qr/Modified 0 tasks\./, 'dependencies - add already existing dependency');
+like ($output, qr/Task 1 already depends on task 2\./, 'dependencies - add already existing dependency');
+
+# t 1 dep:1 => error
+$output = qx{../task rc:dep.rc 1 dep:1};
+like   ($output, qr/A task cannot be dependent on itself\./, 'dependencies - cannot depend on self');
+unlike ($output, qr/Modified 1 task\./,                      'dependencies - cannot depend on self');
 
 # t 1 dep:2; t 2 dep:1 => error
 $output = qx{../task rc:dep.rc 2 dep:1};