[FIXED JENKINS-9960]

We weren't actually enforcing the initialization order at all, as sorting was based on the plugin name.
jenkinsci · Jun 12, 2011 · e03ba7d · e03ba7d
1 parent 01696a8
commit e03ba7d
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 12 deletions.
diff --git a/changelog.html b/changelog.html
@@ -71,6 +71,9 @@
   <li class=bug>
     Fixed timeline on build trend page.
     (<a href="http://issues.jenkins-ci.org/browse/JENKINS-6439">issue 6439</a>)
+  <li class=bug>
+    Fixed the initialization order of plugins
+    (<a href="http://issues.jenkins-ci.org/browse/JENKINS-9960">issue 9960</a>)
   <li class=rfe>
     <tt>LDAPBindSecurityRealm.groovy</tt> can be now overridden in <tt>$JENKINS_HOME</tt> if it exists.
   <li class=rfe>

diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java
@@ -69,6 +69,7 @@
 import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Set;
 import java.util.Map;
 import java.util.HashMap;
@@ -188,8 +189,6 @@ public void run(Reactor session1) throws Exception {
 
                                             p.isBundled = bundledPlugins.contains(arc.getName());
                                             plugins.add(p);
-                                            if(p.isActive())
-                                                activePlugins.add(p);
                                         } catch (IOException e) {
                                             failedPlugins.add(new FailedPlugin(arc.getName(),e));
                                             throw e;
@@ -213,34 +212,43 @@ private boolean isDuplicate(PluginWrapper p) {
                                 });
                             }
 
-                            g.requires(PLUGINS_PREPARED).add("Checking cyclic dependencies",new Executable() {
+                            g.followedBy().attains(PLUGINS_LISTED).add("Checking cyclic dependencies", new Executable() {
                                 /**
                                  * Makes sure there's no cycle in dependencies.
                                  */
                                 public void run(Reactor reactor) throws Exception {
                                     try {
-                                        new CyclicGraphDetector<PluginWrapper>() {
+                                        CyclicGraphDetector<PluginWrapper> cgd = new CyclicGraphDetector<PluginWrapper>() {
                                             @Override
                                             protected List<PluginWrapper> getEdges(PluginWrapper p) {
                                                 List<PluginWrapper> next = new ArrayList<PluginWrapper>();
-                                                addTo(p.getDependencies(),next);
-                                                addTo(p.getOptionalDependencies(),next);
+                                                addTo(p.getDependencies(), next);
+                                                addTo(p.getOptionalDependencies(), next);
                                                 return next;
                                             }
 
                                             private void addTo(List<Dependency> dependencies, List<PluginWrapper> r) {
                                                 for (Dependency d : dependencies) {
                                                     PluginWrapper p = getPlugin(d.shortName);
-                                                    if (p!=null)
+                                                    if (p != null)
                                                         r.add(p);
                                                 }
                                             }
-                                        }.run(getPlugins());
+                                        };
+                                        cgd.run(getPlugins());
+
+                                        // obtain topologically sorted list and overwrite the list
+                                        ListIterator<PluginWrapper> litr = plugins.listIterator();
+                                        for (PluginWrapper p : cgd.getSorted()) {
+                                            litr.next();
+                                            litr.set(p);
+                                            if(p.isActive())
+                                                activePlugins.add(p);
+                                        }
                                     } catch (CycleDetectedException e) {
                                         stop(); // disable all plugins since classloading from them can lead to StackOverflow
                                         throw e;    // let Hudson fail
                                     }
-                                    Collections.sort(plugins);
                                 }
                             });
 

diff --git a/core/src/main/java/hudson/util/CyclicGraphDetector.java b/core/src/main/java/hudson/util/CyclicGraphDetector.java
@@ -2,7 +2,10 @@
 
 import hudson.Util;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.Stack;
@@ -17,11 +20,21 @@ public abstract class CyclicGraphDetector<N> {
     private final Set<N> visiting = new HashSet<N>();
     private final Stack<N> path = new Stack<N>();
 
+    private final List<N> topologicalOrder = new ArrayList<N>();
+
     public void run(Iterable<? extends N> allNodes) throws CycleDetectedException {
         for (N n : allNodes)
             visit(n);
     }
 
+    /**
+     * Returns all the nodes in the topologically sorted order.
+     * That is, if there's an edge a->b, b always come earlier than a.
+     */
+    public List<N> getSorted() {
+        return topologicalOrder;
+    }
+
     /**
      * List up edges from the given node (by listing nodes that those edges point to.)
      *
@@ -43,6 +56,7 @@ private void visit(N p) throws CycleDetectedException {
         }
         visiting.remove(p);
         path.pop();
+        topologicalOrder.add(p);
     }
 
     private void detectedCycle(N q) throws CycleDetectedException {

diff --git a/plugins/pom.xml b/plugins/pom.xml
@@ -4,7 +4,7 @@
   <parent>
     <groupId>org.jenkins-ci</groupId>
     <artifactId>jenkins</artifactId>
-    <version>1.18</version>
+    <version>1.21</version>
   </parent>
 
   <groupId>org.jenkins-ci.plugins</groupId>
@@ -155,7 +155,7 @@
         <!-- version specified in parent pom -->
       </plugin>
       <plugin>
-        <groupId>org.codehaus.groovy.maven</groupId>
+        <groupId>org.codehaus.gmaven</groupId>
         <artifactId>gmaven-plugin</artifactId>
         <!-- version specified in grandparent pom -->
         <executions>

diff --git a/pom.xml b/pom.xml
@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jenkins-ci</groupId>
     <artifactId>jenkins</artifactId>
-    <version>1.18</version>
+    <version>1.21</version>
   </parent>
 
   <groupId>org.jenkins-ci.main</groupId>