Hi,

While working on the Bushel project (http://code.google.com/p/bushel/)
to add OSGi bundle support to Ivy, I stumbled across a limitation in
the LatestRevisionStrategy. The current implementation sorts OSGi
major.minor.micro[.qualifier] in what I think is the wrong order.

For example, from the LatestRevisionStrategyTest.testComparator()
test, the natural order of both OSGi and non-OSGi versions is:
    0.2a, 0.2_b, 0.2rc1, 0.2-final, 1.0-dev1, 1.0-dev2, 1.0-alpha1,
1.0-alpha2, 1.0-beta1, 1.0-beta2, 1.0-gamma, 1.0-rc1, 1.0-rc2, 1.0,
1.0.1, 2.0, 2.0.0, 2.0.0.b006, 2.0.0.b012, 2.0.0.xyz

However, the current implementation produces the following, where the
last four elements are in the wrong order:
    0.2a, 0.2_b, 0.2rc1, 0.2-final, 1.0-dev1, 1.0-dev2, 1.0-alpha1,
1.0-alpha2, 1.0-beta1, 1.0-beta2, 1.0-gamma, 1.0-rc1, 1.0-rc2, 1.0,
1.0.1, 2.0, 2.0.0.b006, 2.0.0.b012, 2.0.0.xyz, 2.0.0

This is because it currently gives equal weighting to the [._-+]
characters, instead of giving the '.' character precedence. I have
attached a patch the fixes this limitation, as well as updating the
unit test. I have also fixed a minor bug in the test where it assumed
the versions were shuffled, but they weren't.

The current way to work around this is to override all the
latest-strategies in the ivysettings.xml, which I'd like to avoid if
possible. For example:

<ivysettings>
    <classpath file="${basedir}/lib/bushel-0.6.1.jar" />

    <typedef name="osgi-parser"
classname="com.googlecode.bushel.ivy.OsgiManifestParser" />
    <typedef name="osgi-file"
classname="com.googlecode.bushel.ivy.OsgiFileResolver" />
    <typedef name="osgi-latest"
classname="com.googlecode.bushel.ivy.OsgiLatestStrategy" />

    <settings defaultLatestStrategy="osgi-latest-revision" />
    ...
    <resolvers>
        <osgi-file name="local-rcp-repo" latest="osgi-latest-revision">
        ...
        </osgi-file>
    </resolvers>

    <latest-strategies>
        <osgi-latest name="osgi-latest-revision" />
    </latest-strategies>
</ivysettings>

I hope you can use this patch.

Cheers,
Alex
Index: test/java/org/apache/ivy/plugins/latest/LatestRevisionStrategyTest.java
===================================================================
--- test/java/org/apache/ivy/plugins/latest/LatestRevisionStrategyTest.java	(revision 684210)
+++ test/java/org/apache/ivy/plugins/latest/LatestRevisionStrategyTest.java	(working copy)
@@ -29,7 +29,8 @@
     public void testComparator() {
         ArtifactInfo[] revs = toMockAI(new String[] {"0.2a", "0.2_b", "0.2rc1", "0.2-final",
                 "1.0-dev1", "1.0-dev2", "1.0-alpha1", "1.0-alpha2", "1.0-beta1", "1.0-beta2",
-                "1.0-gamma", "1.0-rc1", "1.0-rc2", "1.0", "1.0.1", "2.0"});
+                "1.0-gamma", "1.0-rc1", "1.0-rc2", "1.0", "1.0.1", 
+                "2.0", "2.0.0", "2.0.0.b006","2.0.0.b012","2.0.0.xyz"});
 
         List shuffled = new ArrayList(Arrays.asList(revs));
         Collections.shuffle(shuffled);
@@ -57,6 +58,7 @@
                 "1.0-gamma", "1.0-rc1", "1.0-rc2", "1.0", "1.0.1", "2.0"});
 
         List shuffled = new ArrayList(Arrays.asList(revs));
+        Collections.shuffle(shuffled);
         ArtifactInfo[] shuffledRevs = (ArtifactInfo[]) shuffled
                 .toArray(new ArtifactInfo[revs.length]);
 
Index: src/java/org/apache/ivy/plugins/latest/LatestRevisionStrategy.java
===================================================================
--- src/java/org/apache/ivy/plugins/latest/LatestRevisionStrategy.java	(revision 684210)
+++ src/java/org/apache/ivy/plugins/latest/LatestRevisionStrategy.java	(working copy)
@@ -21,12 +21,18 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.apache.ivy.core.IvyContext;
 import org.apache.ivy.core.module.id.ModuleRevisionId;
 import org.apache.ivy.plugins.version.VersionMatcher;
 
 public class LatestRevisionStrategy extends ComparatorLatestStrategy {
+    
+    private static final Pattern ALPHA_NUM_REGEX = Pattern.compile("([a-zA-Z])(\\d)");
+    private static final Pattern NUM_ALPHA_REGEX = Pattern.compile("(\\d)([a-zA-Z])");
+    private static final Pattern LABEL_REGEX = Pattern.compile("[_\\-\\+]");
+    
     /**
      * Compares two ModuleRevisionId by their revision. Revisions are compared using an algorithm
      * inspired by PHP version_compare one.
@@ -35,50 +41,68 @@
         public int compare(Object o1, Object o2) {
             String rev1 = ((ModuleRevisionId) o1).getRevision();
             String rev2 = ((ModuleRevisionId) o2).getRevision();
-        
-            rev1 = rev1.replaceAll("([a-zA-Z])(\\d)", "$1.$2");
-            rev1 = rev1.replaceAll("(\\d)([a-zA-Z])", "$1.$2");
-            rev2 = rev2.replaceAll("([a-zA-Z])(\\d)", "$1.$2");
-            rev2 = rev2.replaceAll("(\\d)([a-zA-Z])", "$1.$2");
-        
-            String[] parts1 = rev1.split("[\\._\\-\\+]");
-            String[] parts2 = rev2.split("[\\._\\-\\+]");
-        
-            int i = 0;
-            for (; i < parts1.length && i < parts2.length; i++) {
-                if (parts1[i].equals(parts2[i])) {
+
+            String[] outerParts1 = rev1.split("[\\.]");
+            String[] outerParts2 = rev2.split("[\\.]");
+            
+            for (int i=0; i < outerParts1.length && i < outerParts2.length; i++) {
+                String outerPart1 = outerParts1[i];
+                String outerPart2 = outerParts2[i];
+
+                if (outerPart1.equals(outerPart2)) {
                     continue;
                 }
-                boolean is1Number = isNumber(parts1[i]);
-                boolean is2Number = isNumber(parts2[i]);
-                if (is1Number && !is2Number) {
-                    return 1;
+
+                outerPart1 = ALPHA_NUM_REGEX.matcher(outerPart1).replaceAll("$1_$2");
+                outerPart1 = NUM_ALPHA_REGEX.matcher(outerPart1).replaceAll("$1_$2");
+                outerPart2 = ALPHA_NUM_REGEX.matcher(outerPart2).replaceAll("$1_$2");
+                outerPart2 = NUM_ALPHA_REGEX.matcher(outerPart2).replaceAll("$1_$2");
+                
+                String[] innerParts1 = LABEL_REGEX.split(outerPart1);
+                String[] innerParts2 = LABEL_REGEX.split(outerPart2);
+            
+                for (int j=0; j < innerParts1.length && j < innerParts2.length; j++) {
+                    if (innerParts1[j].equals(innerParts2[j])) {
+                        continue;
+                    }
+                    boolean is1Number = isNumber(innerParts1[j]);
+                    boolean is2Number = isNumber(innerParts2[j]);
+                    if (is1Number && !is2Number) {
+                        return 1;
+                    }
+                    if (is2Number && !is1Number) {
+                        return -1;
+                    }
+                    if (is1Number && is2Number) {
+                        return Long.valueOf(innerParts1[j]).compareTo(Long.valueOf(innerParts2[j]));
+                    }
+                    // both are strings, we compare them taking into account special meaning
+                    Map specialMeanings = getSpecialMeanings();
+                    Integer sm1 = (Integer) specialMeanings.get(innerParts1[j].toLowerCase(Locale.US));
+                    Integer sm2 = (Integer) specialMeanings.get(innerParts2[j].toLowerCase(Locale.US));
+                    if (sm1 != null) {
+                        sm2 = sm2 == null ? new Integer(0) : sm2;
+                        return sm1.compareTo(sm2);
+                    }
+                    if (sm2 != null) {
+                        return new Integer(0).compareTo(sm2);
+                    }
+                    return innerParts1[j].compareTo(innerParts2[j]);
                 }
-                if (is2Number && !is1Number) {
-                    return -1;
+                if (i < innerParts1.length) {
+                    return isNumber(innerParts1[i]) ? 1 : -1;
                 }
-                if (is1Number && is2Number) {
-                    return Long.valueOf(parts1[i]).compareTo(Long.valueOf(parts2[i]));
+                if (i < innerParts2.length) {
+                    return isNumber(innerParts2[i]) ? -1 : 1;
                 }
-                // both are strings, we compare them taking into account special meaning
-                Map specialMeanings = getSpecialMeanings();
-                Integer sm1 = (Integer) specialMeanings.get(parts1[i].toLowerCase(Locale.US));
-                Integer sm2 = (Integer) specialMeanings.get(parts2[i].toLowerCase(Locale.US));
-                if (sm1 != null) {
-                    sm2 = sm2 == null ? new Integer(0) : sm2;
-                    return sm1.compareTo(sm2);
-                }
-                if (sm2 != null) {
-                    return new Integer(0).compareTo(sm2);
-                }
-                return parts1[i].compareTo(parts2[i]);
             }
-            if (i < parts1.length) {
-                return isNumber(parts1[i]) ? 1 : -1;
-            }
-            if (i < parts2.length) {
-                return isNumber(parts2[i]) ? -1 : 1;
-            }
+
+            if(outerParts1.length > outerParts2.length) {
+                return 1;
+            } else if(outerParts1.length < outerParts2.length) {
+                return -1;
+            } 
+            
             return 0;
         }
 
@@ -86,7 +110,7 @@
             return str.matches("\\d+");
         }
     }
-
+    
     /**
      * Compares two ArtifactInfo by their revision. Revisions are compared using an algorithm
      * inspired by PHP version_compare one, unless a dynamic revision is given, in which case the
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to