From 6d5d9bf395a6b5431fad8e2ba915a111494f9303 Mon Sep 17 00:00:00 2001
From: kohsuke <kohsuke@71c3de6d-444a-0410-be80-ed276b4c234a>
Date: Thu, 3 Jul 2008 18:51:58 +0000
Subject: [PATCH] Rolling back 10329. See
 http://www.nabble.com/Getting-exception-when-triggering-downstream-job.-tt18252241.html
 for the report.

rev.10329 contains two changes --- one to have DependencyGraph not to implement Comparator, and the other about how the comparison was implemented.
But the commit log doesn't give enough justification about neither of them.

I can't think of any other ordering among projects in the context of dependency graph, so I'm curious about the motivation behind the first change.
It's also not clear what the new algorithm does to me. The original one is much more concise, if not too efficient. And as this bug report indicates,
the added complexity made it error prone.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@10613 71c3de6d-444a-0410-be80-ed276b4c234a
---
 .../java/hudson/model/DependencyGraph.java    | 59 ++++---------------
 .../main/java/hudson/tasks/BuildTrigger.java  |  3 +-
 2 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/core/src/main/java/hudson/model/DependencyGraph.java b/core/src/main/java/hudson/model/DependencyGraph.java
index 17b891f0e5..1cffc7bb3c 100644
--- a/core/src/main/java/hudson/model/DependencyGraph.java
+++ b/core/src/main/java/hudson/model/DependencyGraph.java
@@ -13,7 +13,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -53,7 +52,7 @@ import java.awt.image.BufferedImage;
  * @see Hudson#getDependencyGraph() 
  * @author Kohsuke Kawaguchi
  */
-public final class DependencyGraph  {
+public final class DependencyGraph implements Comparator <AbstractProject> {
 
     private Map<AbstractProject, List<AbstractProject>> forward = new HashMap<AbstractProject, List<AbstractProject>>();
     private Map<AbstractProject, List<AbstractProject>> backward = new HashMap<AbstractProject, List<AbstractProject>>();
@@ -315,51 +314,15 @@ public final class DependencyGraph  {
     public static final DependencyGraph EMPTY = new DependencyGraph(false);
 
     /**
-     * Sort the given in their Buildorder. Note that there may be more than
-     * one possible order.
-     *
-     * @param list to be ordered
+     * Compare to Projects based on the topological order defined by this Dependency Graph
      */
-    public void sort(List<AbstractProject> list) {
-        // Record the Ingrade of each Node in the graph
-        Map<AbstractProject, Integer> inGrade = new HashMap<AbstractProject, Integer>();
-        // Holds nodes/Projects with ingrade 0
-        Set<AbstractProject> roots = new HashSet<AbstractProject>();
-        // For each Project calculate the ingrade
-        for (AbstractProject p : Hudson.getInstance().getProjects()) {
-            List<AbstractProject> ancestors = new ArrayList<AbstractProject>(
-                    getUpstream(p));
-            if (ancestors.size() == 0) {
-                roots.add(p);
-                inGrade.put(p, 0);
-            } else
-                inGrade.put(p, ancestors.size());
-        }
-
-        List<AbstractProject> result = new ArrayList<AbstractProject>(list.size());
-        while (!roots.isEmpty()) {
-            AbstractProject current = roots.iterator().next();
-            // Since we took all existing Projects in account for sorting this may
-            // contain projects that have not been in the original list.
-            if (list.contains(current)) {
-                result.add(current);
-            }
-            roots.remove(current);
-            // Reduce the ingrade of successor of the current node. Some of them may become new roots
-            for (AbstractProject p : getDownstream(current)) {
-                Integer newIngrade = inGrade.get(p);
-                if (newIngrade == null)
-                    continue;
-                newIngrade = newIngrade - 1;
-                inGrade.put(p, newIngrade);
-                if (newIngrade == 0) {
-                    roots.add(p);
-                }
-            }
-        }
-        // Write the result back to the list
-        for (int i = 0; i < list.size(); i++) {
-            list.set(i, result.get(i));
-        }
-    }        
+    public int compare(AbstractProject o1, AbstractProject o2) {
+        Set<AbstractProject> o1sdownstreams = getTransitiveDownstream(o1);
+        Set<AbstractProject> o2sdownstreams = getTransitiveDownstream(o2);
+        if (o1sdownstreams.contains(o2)) {
+            if (o2sdownstreams.contains(o1)) return 0; else return 1;                       
+        } else {
+            if (o2sdownstreams.contains(o1)) return -1; else return 0; 
+        }               
+    }
 }
diff --git a/core/src/main/java/hudson/tasks/BuildTrigger.java b/core/src/main/java/hudson/tasks/BuildTrigger.java
index fef61b0a57..6b4b7ec4b3 100644
--- a/core/src/main/java/hudson/tasks/BuildTrigger.java
+++ b/core/src/main/java/hudson/tasks/BuildTrigger.java
@@ -129,7 +129,8 @@ public class BuildTrigger extends Publisher implements DependecyDeclarer, Matrix
                 new ArrayList<AbstractProject> (build.getProject().getDownstreamProjects()); 
                 
             // Sort topologically            
-            Hudson.getInstance().getDependencyGraph().sort(downstreamProjects);
+            Collections.sort(downstreamProjects, 
+                    Collections.reverseOrder (Hudson.getInstance().getDependencyGraph()));
             
             for (AbstractProject p : downstreamProjects) {
                 if(p.isDisabled()) {
-- 
GitLab