Bug#673765: dom4j FTBFS with openjdk-7

[Niels Thykier]

On 2012-05-21 11:56, James Page wrote:
> Package: dom4j
> Version: 1.6.1+dfsg.2-5
> Severity: normal
> Tags: patch
> User: ubuntu-devel at lists.ubuntu.com
> Usertags: origin-ubuntu quantal ubuntu-patch openjdk-7-transition
> 
> Dear Maintainer,
> 
> In Ubuntu, the attached patch was applied to achieve the following:
> 
>   * Fix FTBFS with openjdk-7 (LP: #888121):
>     - d/patches/java7-compat.patch: Fix compareTo function in Rule class
>       to ensure that comparisions are actually symmetric.
> 
> I suspect that the way the Collections.sort() function works has changed in
> Java 7 - basically dom4j Rule comparision was a bit broken in that
> 
>     r1 > r2 = 1
> 
> but
> 
>     r2 < r1 = 0 (should be -1)
> 
> This causes Java 7 to leave the arraylist intact rather than sorting it.
> 
> This patch works with both Java6 and Java7.
> 
> Thanks for considering the patch.
> 
> 
> [...]

Hi,

Thanks for reporting this.

However, I must admit I am a bit concerned in the use of of "!=" with
reals (floats/doubles)[0].  To my understanding (in at least C/C++) that
is very likely to always be true due to even minor rounding errors[1].
Presumably this is why upstream used "Math.round" in the first place.
  It is possible that it only apply to expressions (and not stored
values in variables) or Java handles this better, but in this case, I
believe a comment in the code documenting this assertion would be prudent.

~Niels

[0]
--- src/java/org/dom4j/rule/Rule.java	2006-10-09 21:24:19 +0000
+++ src/java/org/dom4j/rule/Rule.java	2012-05-21 09:30:54 +0000
@@ -99,16 +99,16 @@
      * @return DOCUMENT ME!
      */
     public int compareTo(Rule that) {
-        int answer = this.importPrecedence - that.importPrecedence;
-
-        if (answer == 0) {
-            answer = (int) Math.round(this.priority - that.priority);
-
-            if (answer == 0) {
-                answer = this.appearenceCount - that.appearenceCount;
-            }
-        }
-
+        int answer = 0;
+        if (this.importPrecedence != that.importPrecedence) {
+            answer = this.importPrecedence < that.importPrecede[...]
+        }
+        else if (this.priority != that.priority) {
+            answer = this.priority < that.priority ? -1 : 1;
+        }
+        else if (this.appearenceCount != that.appearenceCount) {
+            answer = this.appearenceCount < that.appearenceCoun[...]
+        }
         return answer;
     }


[1] http://warp.povusers.org/programming/floating_point_caveats.html