Improve setup related models

Change-Id: If0b38c44f866e1ef7b491b3c005527f3b4399f23
Signed-off-by: Ed Merks <ed.merks@gmail.com>

3 files changed, 66 insertions, 60 deletions

diff --git a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/internal/setup/SetupTaskPerformer.java b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/internal/setup/SetupTaskPerformer.java
index a101cd47c5..7b2d619d6e 100644
--- a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/internal/setup/SetupTaskPerformer.java
+++ b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/internal/setup/SetupTaskPerformer.java
@@ -39,8 +39,6 @@ import org.eclipse.ui.PlatformUI;
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
@@ -338,9 +336,45 @@ public class SetupTaskPerformer extends HashMap<Object, Object> implements Setup
     Map<SetupTask, SetupTask> substitutions = getSubstitutions(setupTasks);
     setup = copySetup(setupTasks, substitutions);
 
+    reorder(setupTasks);
+
     perform(setupTasks);
   }
 
+  private void reorder(EList<SetupTask> setupTasks)
+  {
+    for (int i = 0, size = setupTasks.size(), count = 0; i < size; ++i)
+    {
+      SetupTask setupTask = setupTasks.get(i);
+      if (count == size)
+      {
+        throw new IllegalArgumentException("Circular requirements " + setupTask);
+      }
+
+      EList<SetupTask> requirements = setupTask.getRequirements();
+      boolean changed = false;
+      for (SetupTask requirement : requirements)
+      {
+        int index = setupTasks.indexOf(requirement);
+        if (index > i)
+        {
+          setupTasks.move(i, index);
+          changed = true;
+        }
+      }
+
+      if (changed)
+      {
+        --i;
+        ++count;
+      }
+      else
+      {
+        count = 0;
+      }
+    }
+  }
+
   private void perform(EList<SetupTask> setupTasks) throws Exception
   {
     final EList<SetupTask> neededTasks = getNeededTasks(setupTasks);
@@ -383,8 +417,6 @@ public class SetupTaskPerformer extends HashMap<Object, Object> implements Setup
 
   private Map<SetupTask, SetupTask> getSubstitutions(EList<SetupTask> setupTasks)
   {
-    SetupTaskComparator.sort(setupTasks);
-
     Map<Object, SetupTask> overrides = new HashMap<Object, SetupTask>();
     Map<SetupTask, SetupTask> substitutions = new HashMap<SetupTask, SetupTask>();
 
@@ -457,7 +489,6 @@ public class SetupTaskPerformer extends HashMap<Object, Object> implements Setup
       }
     }
 
-    SetupTaskComparator.sort(setupTasks);
     return setup;
   }
 
@@ -490,51 +521,4 @@ public class SetupTaskPerformer extends HashMap<Object, Object> implements Setup
   {
     SetupTaskPerformer.progress = progress;
   }
-
-  /**
-   * @author Eike Stepper
-   */
-  public static class SetupTaskComparator implements Comparator<SetupTask>
-  {
-    public static void sort(EList<SetupTask> setupTasks)
-    {
-      Collections.sort(setupTasks, new SetupTaskComparator());
-    }
-
-    public int compare(SetupTask t1, SetupTask t2)
-    {
-      boolean t1RequiresT2 = t1.requires(t2);
-      boolean t2RequiresT1 = t2.requires(t1);
-      if (t1RequiresT2 && t2RequiresT1)
-      {
-        throw new IllegalStateException("Requirements cycle detected between " + t1 + " and " + t2);
-      }
-
-      if (t1RequiresT2 && !t2RequiresT1)
-      {
-        return 1;
-      }
-
-      if (!t1RequiresT2 && t2RequiresT1)
-      {
-        return -1;
-      }
-
-      int scope1 = t1.getScope().getValue();
-      int scope2 = t2.getScope().getValue();
-      if (scope1 < scope2)
-      {
-        return -1;
-      }
-
-      if (scope1 > scope2)
-      {
-        return 1;
-      }
-
-      String uri1 = EcoreUtil.getURI(t1).toString();
-      String uri2 = EcoreUtil.getURI(t2).toString();
-      return uri1.compareTo(uri2); // Arbitrary but symmetric within one ResourceSet
-    }
-  }
 }
diff --git a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupImpl.java b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupImpl.java
index b4bb261357..f84cd94845 100644
--- a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupImpl.java
+++ b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupImpl.java
@@ -111,7 +111,9 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
       if (branch != oldBranch)
       {
         if (eNotificationRequired())
+        {
           eNotify(new ENotificationImpl(this, Notification.RESOLVE, SetupPackage.SETUP__BRANCH, oldBranch, branch));
+        }
       }
     }
     return branch;
@@ -137,7 +139,9 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
     Branch oldBranch = branch;
     branch = newBranch;
     if (eNotificationRequired())
+    {
       eNotify(new ENotificationImpl(this, Notification.SET, SetupPackage.SETUP__BRANCH, oldBranch, branch));
+    }
   }
 
   /**
@@ -154,8 +158,10 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
       if (eclipseVersion != oldEclipseVersion)
       {
         if (eNotificationRequired())
+        {
           eNotify(new ENotificationImpl(this, Notification.RESOLVE, SetupPackage.SETUP__ECLIPSE_VERSION,
               oldEclipseVersion, eclipseVersion));
+        }
       }
     }
     return eclipseVersion;
@@ -181,8 +187,10 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
     EclipseVersion oldEclipseVersion = eclipseVersion;
     eclipseVersion = newEclipseVersion;
     if (eNotificationRequired())
+    {
       eNotify(new ENotificationImpl(this, Notification.SET, SetupPackage.SETUP__ECLIPSE_VERSION, oldEclipseVersion,
           eclipseVersion));
+    }
   }
 
   /**
@@ -199,8 +207,10 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
       if (preferences != oldPreferences)
       {
         if (eNotificationRequired())
+        {
           eNotify(new ENotificationImpl(this, Notification.RESOLVE, SetupPackage.SETUP__PREFERENCES, oldPreferences,
               preferences));
+        }
       }
     }
     return preferences;
@@ -226,8 +236,10 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
     Preferences oldPreferences = preferences;
     preferences = newPreferences;
     if (eNotificationRequired())
+    {
       eNotify(new ENotificationImpl(this, Notification.SET, SetupPackage.SETUP__PREFERENCES, oldPreferences,
           preferences));
+    }
   }
 
   /**
@@ -239,17 +251,17 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
   {
     EList<SetupTask> setupTasks = new BasicEList<SetupTask>();
 
-    Preferences preferences = getPreferences();
-    getSetupTasks(filterRestrictions, trigger, setupTasks, preferences);
-
-    Branch branch = getBranch();
-    getSetupTasks(filterRestrictions, trigger, setupTasks, branch);
+    EclipseVersion eclipseVersion = getEclipseVersion();
+    getSetupTasks(filterRestrictions, trigger, setupTasks, eclipseVersion);
 
     Project project = branch.getProject();
     getSetupTasks(filterRestrictions, trigger, setupTasks, project);
 
-    EclipseVersion eclipseVersion = getEclipseVersion();
-    getSetupTasks(filterRestrictions, trigger, setupTasks, eclipseVersion);
+    Branch branch = getBranch();
+    getSetupTasks(filterRestrictions, trigger, setupTasks, branch);
+
+    Preferences preferences = getPreferences();
+    getSetupTasks(filterRestrictions, trigger, setupTasks, preferences);
 
     return setupTasks;
   }
@@ -290,15 +302,21 @@ public class SetupImpl extends MinimalEObjectImpl.Container implements Setup
     {
     case SetupPackage.SETUP__BRANCH:
       if (resolve)
+      {
         return getBranch();
+      }
       return basicGetBranch();
     case SetupPackage.SETUP__ECLIPSE_VERSION:
       if (resolve)
+      {
         return getEclipseVersion();
+      }
       return basicGetEclipseVersion();
     case SetupPackage.SETUP__PREFERENCES:
       if (resolve)
+      {
         return getPreferences();
+      }
       return basicGetPreferences();
     }
     return super.eGet(featureID, resolve, coreType);
diff --git a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupTaskImpl.java b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupTaskImpl.java
index c33d040385..10c774abc6 100644
--- a/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupTaskImpl.java
+++ b/plugins/org.eclipse.emf.cdo.releng.setup/src/org/eclipse/emf/cdo/releng/setup/impl/SetupTaskImpl.java
@@ -163,8 +163,10 @@ public abstract class SetupTaskImpl extends MinimalEObjectImpl.Container impleme
     Set<Trigger> oldExcludedTriggers = excludedTriggers;
     excludedTriggers = newExcludedTriggers;
     if (eNotificationRequired())
+    {
       eNotify(new ENotificationImpl(this, Notification.SET, SetupPackage.SETUP_TASK__EXCLUDED_TRIGGERS,
           oldExcludedTriggers, excludedTriggers));
+    }
   }
 
   public void setExcludedTriggers(Set<Trigger> newExcludedTriggers)
@@ -244,7 +246,7 @@ public abstract class SetupTaskImpl extends MinimalEObjectImpl.Container impleme
 
   private boolean requires(SetupTask setupTask, Set<SetupTask> visited)
   {
-    if (visited.add(setupTask))
+    if (visited.add(this))
     {
       if (setupTask == this)
       {
@@ -387,7 +389,9 @@ public abstract class SetupTaskImpl extends MinimalEObjectImpl.Container impleme
   public String toString()
   {
     if (eIsProxy())
+    {
       return super.toString();
+    }
 
     StringBuffer result = new StringBuffer(super.toString());
     result.append(" (excludedTriggers: ");