Author: wang
Date: Wed Jan 29 22:46:00 2014
New Revision: 1562621

URL: http://svn.apache.org/r1562621
Log:
HADOOP-10250. VersionUtil returns wrong value when comparing two versions. 
Merged from r1561861 in branch-2.

Added:
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java
      - copied unchanged from r1561861, 
hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ComparableVersion.java
Modified:
    hadoop/common/branches/branch-2.3/hadoop-common-project/   (props changed)
    hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-auth/   
(props changed)
    hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/   
(props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt
   (contents, props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml
    hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/  
 (props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/docs/
   (props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/
   (props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/core/
   (props changed)
    
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java

Propchange: hadoop/common/branches/branch-2.3/hadoop-common-project/
------------------------------------------------------------------------------
  Merged /hadoop/common/branches/branch-2/hadoop-common-project:r1561861

Propchange: hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-auth/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-auth:r1561861

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common:r1561861

Modified: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1562621&r1=1562620&r2=1562621&view=diff
==============================================================================
--- 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt
 (original)
+++ 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt
 Wed Jan 29 22:46:00 2014
@@ -369,6 +369,9 @@ Release 2.3.0 - UNRELEASED
     HADOOP-10288. Explicit reference to Log4JLogger breaks non-log4j users
     (todd)
 
+    HADOOP-10250. VersionUtil returns wrong value when comparing two versions.
+    (Yongjun Zhang via atm)
+
 Release 2.2.0 - 2013-10-13
 
   INCOMPATIBLE CHANGES

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/CHANGES.txt
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt:r1561861

Modified: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml?rev=1562621&r1=1562620&r2=1562621&view=diff
==============================================================================
--- 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml
 (original)
+++ 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml
 Wed Jan 29 22:46:00 2014
@@ -364,4 +364,11 @@
       <Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
     </Match>
 
+     <!-- code from maven source, null value is checked at callee side. -->
+     <Match>
+       <Class name="org.apache.hadoop.util.ComparableVersion$ListItem" />
+       <Method name="compareTo" />
+       <Bug code="NP" />
+     </Match>
+
 </FindBugsFilter>

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src:r1561861

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/docs/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/docs:r1561861

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java:r1561861

Modified: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java?rev=1562621&r1=1562620&r2=1562621&view=diff
==============================================================================
--- 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java
 (original)
+++ 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/VersionUtil.java
 Wed Jan 29 22:46:00 2014
@@ -17,55 +17,17 @@
  */
 package org.apache.hadoop.util;
 
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import org.apache.hadoop.classification.InterfaceAudience;
 
-import com.google.common.collect.ComparisonChain;
-
+/**
+ * A wrapper class to maven's ComparableVersion class, to comply
+ * with maven's version name string convention 
+ */
 @InterfaceAudience.Private
 public abstract class VersionUtil {
-  
-  private static final Pattern COMPONENT_GROUPS = 
Pattern.compile("(\\d+)|(\\D+)");
-
-  /**
-   * Suffix added by maven for nightly builds and other snapshot releases.
-   * These releases are considered to precede the non-SNAPSHOT version
-   * with the same version number.
-   */
-  private static final String SNAPSHOT_SUFFIX = "-SNAPSHOT";
-
   /**
-   * This function splits the two versions on &quot;.&quot; and performs a
-   * naturally-ordered comparison of the resulting components. For example, the
-   * version string "0.3" is considered to precede "0.20", despite the fact 
that
-   * lexical comparison would consider "0.20" to precede "0.3". This method of
-   * comparison is similar to the method used by package versioning systems 
like
-   * deb and RPM.
-   * 
-   * Version components are compared numerically whenever possible, however a
-   * version component can contain non-numeric characters. When a non-numeric
-   * group of characters is found in a version component, this group is 
compared
-   * with the similarly-indexed group in the other version component. If the
-   * other group is numeric, then the numeric group is considered to precede 
the
-   * non-numeric group. If both groups are non-numeric, then a lexical
-   * comparison is performed.
-   * 
-   * If two versions have a different number of components, then only the lower
-   * number of components are compared. If those components are identical
-   * between the two versions, then the version with fewer components is
-   * considered to precede the version with more components.
-   * 
-   * In addition to the above rules, there is one special case: maven SNAPSHOT
-   * releases are considered to precede a non-SNAPSHOT release with an
-   * otherwise identical version number. For example, 2.0-SNAPSHOT precedes
-   * 2.0.
-   * 
-   * This function returns a negative integer if version1 precedes version2, a
-   * positive integer if version2 precedes version1, and 0 if and only if the
-   * two versions' components are identical in value and cardinality.
-   * 
+   * Compares two version name strings using maven's ComparableVersion class.
+   *
    * @param version1
    *          the first version to compare
    * @param version2
@@ -75,58 +37,8 @@ public abstract class VersionUtil {
    *         versions are equal.
    */
   public static int compareVersions(String version1, String version2) {
-    boolean isSnapshot1 = version1.endsWith(SNAPSHOT_SUFFIX);
-    boolean isSnapshot2 = version2.endsWith(SNAPSHOT_SUFFIX);
-    version1 = stripSnapshotSuffix(version1);
-    version2 = stripSnapshotSuffix(version2);
-    
-    String[] version1Parts = version1.split("\\.");
-    String[] version2Parts = version2.split("\\.");
-    
-    for (int i = 0; i < version1Parts.length && i < version2Parts.length; i++) 
{
-      String component1 = version1Parts[i];
-      String component2 = version2Parts[i];
-      if (!component1.equals(component2)) {
-        Matcher matcher1 = COMPONENT_GROUPS.matcher(component1);
-        Matcher matcher2 = COMPONENT_GROUPS.matcher(component2);
-        
-        while (matcher1.find() && matcher2.find()) {
-          String group1 = matcher1.group();
-          String group2 = matcher2.group();
-          if (!group1.equals(group2)) {
-            if (isNumeric(group1) && isNumeric(group2)) {
-              return Integer.parseInt(group1) - Integer.parseInt(group2);
-            } else if (!isNumeric(group1) && !isNumeric(group2)) {
-              return group1.compareTo(group2);
-            } else {
-              return isNumeric(group1) ? -1 : 1;
-            }
-          }
-        }
-        return component1.length() - component2.length();
-      }
-    }
-    
-    return ComparisonChain.start()
-      .compare(version1Parts.length, version2Parts.length)
-      .compare(isSnapshot2, isSnapshot1)
-      .result();
-  }
-  
-  private static String stripSnapshotSuffix(String version) {
-    if (version.endsWith(SNAPSHOT_SUFFIX)) {
-      return version.substring(0, version.length() - SNAPSHOT_SUFFIX.length());
-    } else {
-      return version;
-    }
-  }
-
-  private static boolean isNumeric(String s) {
-    try {
-      Integer.parseInt(s);
-      return true;
-    } catch (NumberFormatException nfe) {
-      return false;
-    }
+    ComparableVersion v1 = new ComparableVersion(version1);
+    ComparableVersion v2 = new ComparableVersion(version2);
+    return v1.compareTo(v2);
   }
 }

Propchange: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/core/
------------------------------------------------------------------------------
  Merged 
/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/core:r1561861

Modified: 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java?rev=1562621&r1=1562620&r2=1562621&view=diff
==============================================================================
--- 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java
 (original)
+++ 
hadoop/common/branches/branch-2.3/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestVersionUtil.java
 Wed Jan 29 22:46:00 2014
@@ -28,10 +28,30 @@ public class TestVersionUtil {
     // Equal versions are equal.
     assertEquals(0, VersionUtil.compareVersions("2.0.0", "2.0.0"));
     assertEquals(0, VersionUtil.compareVersions("2.0.0a", "2.0.0a"));
-    assertEquals(0, VersionUtil.compareVersions("1", "1"));
     assertEquals(0, VersionUtil.compareVersions(
         "2.0.0-SNAPSHOT", "2.0.0-SNAPSHOT"));
-    
+
+    assertEquals(0, VersionUtil.compareVersions("1", "1"));
+    assertEquals(0, VersionUtil.compareVersions("1", "1.0"));
+    assertEquals(0, VersionUtil.compareVersions("1", "1.0.0"));
+
+    assertEquals(0, VersionUtil.compareVersions("1.0", "1"));
+    assertEquals(0, VersionUtil.compareVersions("1.0", "1.0"));
+    assertEquals(0, VersionUtil.compareVersions("1.0", "1.0.0"));
+
+    assertEquals(0, VersionUtil.compareVersions("1.0.0", "1"));
+    assertEquals(0, VersionUtil.compareVersions("1.0.0", "1.0"));
+    assertEquals(0, VersionUtil.compareVersions("1.0.0", "1.0.0"));
+
+    assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha-1", "1.0.0-a1"));
+    assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha-2", "1.0.0-a2"));
+    assertEquals(0, VersionUtil.compareVersions("1.0.0-alpha1", 
"1.0.0-alpha-1"));
+
+    assertEquals(0, VersionUtil.compareVersions("1a0", "1.0.0-alpha-0"));
+    assertEquals(0, VersionUtil.compareVersions("1a0", "1-a0"));
+    assertEquals(0, VersionUtil.compareVersions("1.a0", "1-a0"));
+    assertEquals(0, VersionUtil.compareVersions("1.a0", "1.0.0-alpha-0"));
+
     // Assert that lower versions are lower, and higher versions are higher.
     assertExpectedValues("1", "2.0.0");
     assertExpectedValues("1.0.0", "2");
@@ -51,15 +71,27 @@ public class TestVersionUtil {
     assertExpectedValues("1.0.2a", "1.0.2ab");
     assertExpectedValues("1.0.0a1", "1.0.0a2");
     assertExpectedValues("1.0.0a2", "1.0.0a10");
+    // The 'a' in "1.a" is not followed by digit, thus not treated as "alpha",
+    // and treated larger than "1.0", per maven's ComparableVersion class
+    // implementation.
     assertExpectedValues("1.0", "1.a");
-    assertExpectedValues("1.0", "1.a0");
+    //The 'a' in "1.a0" is followed by digit, thus treated as "alpha-<digit>"
+    assertExpectedValues("1.a0", "1.0");
+    assertExpectedValues("1a0", "1.0");    
+    assertExpectedValues("1.0.1-alpha-1", "1.0.1-alpha-2");    
+    assertExpectedValues("1.0.1-beta-1", "1.0.1-beta-2");
     
     // Snapshot builds precede their eventual releases.
     assertExpectedValues("1.0-SNAPSHOT", "1.0");
-    assertExpectedValues("1.0", "1.0.0-SNAPSHOT");
+    assertExpectedValues("1.0.0-SNAPSHOT", "1.0");
     assertExpectedValues("1.0.0-SNAPSHOT", "1.0.0");
     assertExpectedValues("1.0.0", "1.0.1-SNAPSHOT");
     assertExpectedValues("1.0.1-SNAPSHOT", "1.0.1");
+    assertExpectedValues("1.0.1-SNAPSHOT", "1.0.2");
+    
+    assertExpectedValues("1.0.1-alpha-1", "1.0.1-SNAPSHOT");
+    assertExpectedValues("1.0.1-beta-1", "1.0.1-SNAPSHOT");
+    assertExpectedValues("1.0.1-beta-2", "1.0.1-SNAPSHOT");
   }
   
   private static void assertExpectedValues(String lower, String higher) {


Reply via email to