Rolling back 10329. See...

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

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; 
+        }               
+    }
 }

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()) {