Bug#673765: dom4j FTBFS with openjdk-7

[James Page]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 23/05/12 09:48, Niels Thykier wrote:
> Revisiting this, couldn't we use Double.compare(double, double)
> for this?  This way we should be safe from changes (if any) in how
> doubles are handled in the future as well as any changes to the
> standard (possible exceptions include allowing the use "NaN" or
> "INF" values).

Actually that's much nicer - I'll update to use that method.

Thanks for pointing that out.

Cheers

James

- -- 
James Page
Ubuntu Core Developer
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCAAGBQJPvLk0AAoJEL/srsug59jDqNgQAJxqstr9yBdKNHiN44ZV0yte
jQdjNNMMR9e92367WKHsjwLugRO1R/7t2pqLLub1pw8aYam0idTwKrlT4S6NN4T/
ztfzcW1yBxZPkhK0YMBgeG0GMm+YBrnWkvI7zYYLtGd5XsRwVDT96rcXKEGOj/ms
kZmS4aiPovk2xk39SIfbGHoJ/I4Iv5Y/PI4mgO1OqRSLNpQPAE8DL52mZCKtlF/Z
Ms2LlFUDPsiuYo7o1Bfd76qDvm9zE8MmywXVImCh6izSeP0lefrARpReEN79kAZc
ox/l4aI87ThC+7QI4iTPFdoTFRuvmkbOUwSB4z7RbvN32lK9qNsvLsZvwy9rGpBV
V6Azc0na6NG6eeCX+Na+mW/26THO7ecntRaPyJw5zhQhq/WLXl/lqsS7oHz/SpKI
4gpq8O5Ig2OeicM44NmXobAcaalXAgL9rZJNa4HPeyKaurZHOpcMfsfPg7qj3Ski
sMn3+U2r2xFEYWLHkG4CHEquJGvq0uY+7KLV07hNMjbSlMCH86cI44KJzA2hna9X
V1JAwhdNB7VOk5HaV6ZvPzOVqB2a38u9hqf8yy1KJiNFoR4YfQxDVIW0vEWswaCy
EdbTGduq3n1QE4gm8TydZJvXQB7ofrAGJkQjT2/hgz/Z2Nor2lQ5Yd9G/ij8As6u
gVri/zU7dDctD9AfpRRl
=4iD3
-END PGP SIGNATURE-



__
This is the maintainer address of Debian's Java team
. 
Please use
debian-j...@lists.debian.org for discussions and questions.

Bug#673765: dom4j FTBFS with openjdk-7

[Niels Thykier]

On 2012-05-23 10:19, James Page wrote:
> Hi Niels
> 
> On 23/05/12 07:27, Niels Thykier wrote:
>> 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.
> 
> I had the same concern when writing this patch but reading into this
> in a bit more detail I decided that this was actually OK for the
> following reason.
> 
> The priority (see [0]) is always one of four values:
> 
>  -0.25
>  0.5
>  0.0
>  -0.5
> 
> I did a quick check and all four of these values can be represented
> accurately in Java (this is not true of all float/double literals).
> 
> I'll update the patch with an appropriate comment to this effect.
> 
> Cheers
> 
> James
> 
> [0] http://www.w3.org/TR/xslt11/#conflict

Revisiting this, couldn't we use Double.compare(double, double) for
this?  This way we should be safe from changes (if any) in how doubles
are handled in the future as well as any changes to the standard
(possible exceptions include allowing the use "NaN" or "INF" values).

~Niels




__
This is the maintainer address of Debian's Java team
. 
Please use
debian-j...@lists.debian.org for discussions and questions.

Bug#673765: dom4j FTBFS with openjdk-7

[James Page]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi Niels

On 23/05/12 07:27, Niels Thykier wrote:
> 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.

I had the same concern when writing this patch but reading into this
in a bit more detail I decided that this was actually OK for the
following reason.

The priority (see [0]) is always one of four values:

 -0.25
 0.5
 0.0
 -0.5

I did a quick check and all four of these values can be represented
accurately in Java (this is not true of all float/double literals).

I'll update the patch with an appropriate comment to this effect.

Cheers

James

[0] http://www.w3.org/TR/xslt11/#conflict
- -- 
James Page
Ubuntu Core Developer
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCAAGBQJPvJ2lAAoJEL/srsug59jDGIoP/jLm1923bfy2YujGzeoD9Ap9
Wb2N6ZSI/bRqGTrz2zo533RN2BeVPsPZknj1g/0VqT42pAEgwWafzLBow8w7KafV
X3Wvio6IMdlhbASQo+iYxlXaFlykHH+UVhWYJfcrRuiCIaCDTGJfTE/Hct7ReGa/
ydk8k95kg7sGmZWXCRc4Rb+ctz48Va1a7mWiCMRGBMREkJs5oLV9FQUVSIEnwp//
zMgQ274PD736HfkkwfOSNDdnarW2rVo1+19Lf3eONNznxJm2L67BHYkDPPx5d895
979JLO+R1gYFSdrvx6WWJ26jXZux8bjKhJniVkpePhRmc+UA/uyObtcrGz7Cd8fH
dGgOUcMJIfxc6uVHYNOs34v9lMViQgyCd365GsAvZNczLg6fPBuR/ANHfNRzlo4X
B+ZL/aNffXjklJZL1PQgfEoaakhXQ+qqwBuKrma3/dw96EP35fqMr6aYaVqU83II
tDpspmnRCbhGRTXYeexQZcOt/m/g5U3XI9jO5pvFrCLAynR6FOCxfqR4RETSKSTL
5MFvuA0GWnicDkHT3L57os5/1lOjgusZApHP4RGhsKWHjpE0TgLgOeAzYFjQ6snV
aWcfweRLV0+NPUX+brdmyM/RgJxUfFZMgDr7OLL4/OgK1HSzi5V4jdISl8hy4TeU
dSCfkd9hhE0TdSuijMsX
=5aMD
-END PGP SIGNATURE-



__
This is the maintainer address of Debian's Java team
. 
Please use
debian-j...@lists.debian.org for discussions and questions.

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-de...@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 +
+++ src/java/org/dom4j/rule/Rule.java   2012-05-21 09:30:54 +
@@ -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




__
This is the maintainer address of Debian's Java team
. 
Please use
debian-j...@lists.debian.org for discussions and questions.

Bug#673765: dom4j FTBFS with openjdk-7

[James Page]

Package: dom4j
Version: 1.6.1+dfsg.2-5
Severity: normal
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu quantal ubuntu-patch openjdk-7-transition

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

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.


- -- System Information:
Debian Release: wheezy/sid
  APT prefers precise-updates
  APT policy: (500, 'precise-updates'), (500, 'precise-security'), (500, 
'precise'), (100, 'precise-backports')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-24-generic (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBCAAGBQJPuhE2AAoJEL/srsug59jDABQQAKyuELfulLsZ22ObDbnWkeRd
mkKqWZLp/dbbximTyx4lqaEa4ZzMgp6WvMzAqRSWyIJXv4C7lb7qlPuo+stBGuKk
HT8VEleWgf68ode2Ss4ATkrbuRU8CavvrO4BDAY4E50cL0pTUiPm8AZSh/POe7os
odqscXIpoqV2AIhJq01kQm6z+X/Fq2VawqGLfPmVGmCLLO7G5vB9SKCs3ADop8kB
Od7P2qegd9plGFLhgDtLkf9ldhUOc1esckmgfkfYuaVOEGvqwV3tkv4tvJqXnYCq
1wYEvL6qOcKa/+6cL6njsynHrbn2Mg7gUHD0La4mbwrazIeJJrw8XLKOEOLV9kmy
SwugP7z7CRcEgQJidjEuhL95xlNrX80/ebgtj7rm3rWbiaTtIwU/DzZntmr94y/6
fWI261CcN3IuFi5TEJsSjcrSC3ofOcNcIWgD92ppJpXpimjuVcmi29YxWWnT3tjj
HfMMncEEbWz5ZvwdVwGS2WSmB2vNWyBLUT4oVGav752xQMQfcDNSWkGTTbzozr+O
kJ4v5UNcJgDqtDiq3GoPOGXxwXQlNBHTr0KBQmRFsnWGWFX7O+5DsMMIZ0BE+Jof
BgD4r6jqUYghvumkYxkCSVwI+bELd1UgY+rCjbL36904KWQutG9KJiqG60Cszr8A
1t3aGn6Se+bG8pC3JYp5
=IIzc
-END PGP SIGNATURE-
=== modified file '.pc/applied-patches'
--- .pc/applied-patches	2011-08-17 18:43:40 +
+++ .pc/applied-patches	2012-05-21 09:30:00 +
@@ -1 +1,2 @@
 oldchanges.patch
+java7-compat.patch

=== added directory '.pc/java7-compat.patch'
=== added directory '.pc/java7-compat.patch/src'
=== added directory '.pc/java7-compat.patch/src/java'
=== added directory '.pc/java7-compat.patch/src/java/org'
=== added directory '.pc/java7-compat.patch/src/java/org/dom4j'
=== added directory '.pc/java7-compat.patch/src/java/org/dom4j/rule'
=== added file '.pc/java7-compat.patch/src/java/org/dom4j/rule/Rule.java'
--- .pc/java7-compat.patch/src/java/org/dom4j/rule/Rule.java	1970-01-01 00:00:00 +
+++ .pc/java7-compat.patch/src/java/org/dom4j/rule/Rule.java	2012-05-21 09:29:53 +
@@ -0,0 +1,331 @@
+/*
+ * Copyright 2001-2005 (C) MetaStuff, Ltd. All Rights Reserved.
+ *
+ * This software is open source.
+ * See the bottom of this file for the licence.
+ */
+
+package org.dom4j.rule;
+
+import org.dom4j.Node;
+
+/**
+ * 
+ * Rule matches against DOM4J Node so that some action can be
+ * performed such as in the XSLT processing model.
+ * 
+ * 
+ * @author James Strachan 
+ * @version $Revision: 1.7 $
+ */
+public class Rule implements Comparable {
+/** Holds value of property mode. */
+private String mode;
+
+/** Holds value of property importPrecedence. */
+private int importPrecedence;
+
+/** Holds value of property priority. */
+private double priority;
+
+/** Holds value of property appearenceCount. */
+private int appearenceCount;
+
+/** Holds value of property pattern. */
+private Pattern pattern;
+
+/** Holds value of property action. */
+private Action action;
+
+public Rule() {
+this.priority = Pattern.DEFAULT_PRIORITY;
+}
+
+public Rule(Pattern pattern) {
+this.pattern = pattern;
+this.priority = pattern.getPriority();
+}
+
+public Rule(Pattern pattern, Action action) {
+this(pattern);
+this.action = action;
+}
+
+/**
+ * Constructs a new Rule with the same instance data as the given rule but a
+ * different pattern.
+ * 
+ * @param that
+ *DOCUMENT ME!
+ * @param pattern
+ *DOCUMENT ME!
+ */
+public Rule(Rule that, Pattern pattern) {
+this.mode = that.mode;
+this.importPrecedence = that.importPrecedence;
+this.priority = that.priority;
+this.appearenceCount = that.appearenceCount;
+this.action = that.action;
+this.pattern = pattern;
+}
+
+public boolean equals(Object that) {
+if (that instanceof Rule) {
+return compareTo((Rule) that) == 0;
+}
+
+return false;
+}
+
+public int hashCode() {
+return importPrecedence + appearenceCount;
+}
+
+public int compareTo(Object that) {
+if (that instanceof Rule) {
+