Re: OSGi friendly LatestRevisionStrategy patch

[Alex Radeski]

On 9/1/08, Tony Sweeney <[EMAIL PROTECTED]> wrote:

> > 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

> Note that several of your examples are not valid OSGi revision numbers.
> The syntax for OSGi is "major[.minor[.micro[.qualifier]]]" where major,
> minor and micro can only be numeric, and qualifier is the only part of a
> version number which can contain alphanumeric characters, from the character
> range [a-zA-Z0-9], '_' and '-'. If the minor or micro number is absent it 
> should
> sort as 0 (i.e. 2.1 == 2.1.0), and the sort order of qualifier is a natural
> alphabetical sort. You can only have a qualifier if you have all three numeric
> components, so only the versions from 1.0 onwards in your list are valid.

Yep, what I was adding was OSGi version compatibility to the existing
Latest Strategy impl,
not strict conformance. I appended the OSGi versions to the existing
test case (these are
the non-osgi versions you mentioned above), which in hind sight
probably wasn't the best idea.

> > 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

> 2.0.0 should sort with 2.0 before all of the 2.0.0 versions with a qualifier,
> so if your patch fixes that then it's an improvement.

Yep, the patch does order them all correctly (non-osgi and osgi
versions together). However, the testing
is limited to that done in the Ivy test suite. There  is a small
possibility that in the wild there are more exotic
versions strings than may be ordered incorrectly. As per Xaviers
suggestion, it may be better to provide an alternative
Latest Strategy than patching the existing one, but I'll leave that
decision to you guys.

Alex.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: OSGi friendly LatestRevisionStrategy patch

[Alex Radeski]

On Mon, Sep 1, 2008 at 7:43 AM, Xavier Hanin <[EMAIL PROTECTED]> wrote:

Hi Xavier,

> While I understand the your point of view, the change you request may imply
> breaking versionning strategies for people using for years. Hence I'm not in
> favor of changing this, at least we need to provide an easy to use backward
> compatible version of this strategy.

That's a fair point, I wouldn't want to break backward compatibility.I
should explain that I appended the OSGi-like versions to the unit test
in my previous post, i probably should have created a separate unit
test. So the old test still passes with the new implementation, hence
still backward compatible.

>
> Once you have defined the defaultLatestStrategy, you shouldn't have to
> specify it everywhere. If you have to, it's a bug, please report it with a
> JIRA.

Ok, I'll double check this was an issue.

> As I said, I don't think we can use it like this. But maybe we could find a
> way for the latest revision strategy to behave as you want or as before. It
> could be an attribute, which value could be set either directly, or through
> an Ivy variable. Then switching to OSGi mode would only require setting one
> variable, instead of what you do now.
>
> What do you and others think about that?

I think what you say is pretty fair. Rather than patching the
LatestReviStrategy class, we could provide a separate OSGi-friendly
class.

Alex.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

OSGi friendly LatestRevisionStrategy patch

[Alex Radeski]

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:









...


...








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;