Fixed regression: dep id listing with gc off; test added

Signed-off-by: ashprice <gitcommit1@sl.ashprice.co.uk>

Author: ashprice

src/Task.cpp

@@ -1049,11 +1049,18 @@ bool Task::hasDependency(const std::string& uuid) const {
 ////////////////////////////////////////////////////////////////////////////////
 std::vector<int> Task::getDependencyIDs() const {
   std::vector<int> ids;
-  for (auto& attr : all()) {
-    if (!isDepAttr(attr)) continue;
-    auto dep = attr2Dep(attr);
-    ids.push_back(Context::getContext().tdb2.id(dep));
-  }
+
+  const auto& graph = Context::getContext().tdb2.dependency_graph();
+  auto found = graph.dependencies.find(get_ref("uuid"));
+  if (found == graph.dependencies.end()) return ids;
+
+  const auto& tasks = Context::getContext().tdb2.pending_tasks();
+  ids.reserve(found->second.size());
+  for (auto idx : found->second)
+    if (tasks[idx].getStatus() != Task::completed && tasks[idx].getStatus() != Task::deleted)
+      ids.push_back(tasks[idx].id);
+
+  std::sort(ids.begin(), ids.end());
 
   return ids;
 }

src/columns/ColDepends.cpp

@@ -76,20 +76,7 @@ void ColumnDepends::measure(const Task& task, unsigned int& minimum, unsigned in
     return;
   }
 
-  // We also don't need to call getDependencyTasks(),
-  // which would copy the full objects from the cache just to read the ID
-  // field. Instead, we can use getDependencyUUIDs() and tdb2.id().
-
-  auto dep_uuids = task.getDependencyUUIDs();
-
-  std::vector<int> blocking_ids;
-  blocking_ids.reserve(dep_uuids.size());
-  for (const auto& uuid : dep_uuids) {
-    int id = Context::getContext().tdb2.id(uuid);
-    if (id > 0) blocking_ids.push_back(id);
-  }
-
-  std::sort(blocking_ids.begin(), blocking_ids.end());
+  auto blocking_ids = task.getDependencyIDs();
 
   if (blocking_ids.size() > 0) {
     if (_style == "count") {
@@ -121,17 +108,7 @@ void ColumnDepends::render(std::vector<std::string>& lines, const Task& task, in
     return;
   }
 
-  // We use the same approach to look up UUIDs as for measure().
-  auto dep_uuids = task.getDependencyUUIDs();
-
-  std::vector<int> blocking_ids;
-  blocking_ids.reserve(dep_uuids.size());
-  for (const auto& uuid : dep_uuids) {
-    int id = Context::getContext().tdb2.id(uuid);
-    if (id > 0) blocking_ids.push_back(id);
-  }
-
-  std::sort(blocking_ids.begin(), blocking_ids.end());
+  auto blocking_ids = task.getDependencyIDs();
 
   if (blocking_ids.size() > 0) {
     if (_style == "count") {

test/dependencies.test.py

@@ -87,6 +87,19 @@ def test_circular_5(self):
         code, out, err = self.t.runError("1 modify dep:5")
         self.assertIn("Circular dependency detected and disallowed.", err)
 
+    def test_completed_dependency_is_not_rendered(self):
+        """Completed dependencies should not be listed even if working set is stale"""
+        self.t("2 modify dep:1")
+        self.t("1 done rc.gc=off")
+
+        self.assertIn("depends", self.t.export_one("rc.gc=off 2"))
+        code, out, err = self.t(
+            "2 list rc.gc=off rc.report.list.columns=id,depends "
+            "rc.report.list.labels=ID,Depends"
+        )
+
+        self.assertIn(["2"], [line.split() for line in out.splitlines()])
+
     def test_dag(self):
         """Check acyclic graph support"""
         self.t("add three")