xercesImpl indecision

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.1.1
  • Fix Version/s: None
  • Component/s: Global
  • Labels:
    None

Description

Various versions of xercesImpl are referenced by poms in projects. This breaks things when running local (single) tests in maven and more importantly, Netbeans which uses maven.

Geotools is using 2.7.1. Unless there is a specific problem or need, this seems to be a reasonable choice.

I changed the top-level to 2.7.1 and everything seems to work. I also tested excel by forcing it to use 2.7.1. The main issue encountered w/ 2.4 being present is the infamous 'java.lang.AbstractMethodError: org.apache.xerces.dom.DocumentImpl.getXmlStandalone()Z' error.

grep -H -A2 -r xercesImpl --include pom.xml .
./release/pom.xml: <artifactId>xercesImpl</artifactId>
./release/pom.xml- <version>2.6.2</version>
–
./pom.xml: <artifactId>xercesImpl</artifactId>
./pom.xml- <version>2.4.0</version>
–
./pom.xml: <artifactId>xercesImpl</artifactId>
./pom.xml- <version>2.6.2</version>
–
./extension/excel/pom.xml: <artifactId>xercesImpl</artifactId>
./extension/excel/pom.xml- <version>2.9.1</version>
–
./main/pom.xml: <artifactId>xercesImpl</artifactId>
./main/pom.xml- </dependency>
–
./community/dbconfig/pom.xml: <artifactId>xercesImpl</artifactId>
./community/dbconfig/pom.xml- </exclusion>

  1. Text File
    fix-xerces.patch
    21/Jul/11 12:13 AM
    0.9 kB
    Ben Caradoc-Davies
  2. Text File
    GEOS-4685.patch
    22/Jul/11 2:58 PM
    2 kB
    Ian Schneider

Issue Links

relates to

Improvement - An improvement or enhancement to an existing feature or task. GEOS-4652 Support building GeoServer with Maven 3

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ben Caradoc-Davies added a comment -

Perfect timing. I just had to face this for GEOS-4652.

Show
Ben Caradoc-Davies added a comment - Perfect timing. I just had to face this for GEOS-4652.
Hide
Permalink
Ben Caradoc-Davies added a comment -

The attached patch is what I was planning to do to get Maven 3 working (still got one unrelated WMS failure).

The patch bumps xercesImpl to 2.7.1 and groups all the xerces dependencies.

Show
Ben Caradoc-Davies added a comment - The attached patch is what I was planning to do to get Maven 3 working (still got one unrelated WMS failure). The patch bumps xercesImpl to 2.7.1 and groups all the xerces dependencies.
Hide
Permalink
Andrea Aime added a comment -

Ben, Ian, if you come to an agreement for this patch just go ahead and commit (at least, it looks good to me, maybe getting Justin's opinion would be good). Ben, is Ian patch compatible with yours?

Show
Andrea Aime added a comment - Ben, Ian, if you come to an agreement for this patch just go ahead and commit (at least, it looks good to me, maybe getting Justin's opinion would be good). Ben, is Ian patch compatible with yours?
Hide
Permalink
Justin Deoliveira added a comment -

Patch looks good to me. I meant to actually apply this as a while back and it fell off. +1

Show
Justin Deoliveira added a comment - Patch looks good to me. I meant to actually apply this as a while back and it fell off. +1
Hide
Permalink
Ian Schneider added a comment -

The patch adressess the top level deps, but the other (confusing) references remain. Should the dependency be removed or updated in release/pom.xml , too?

Show
Ian Schneider added a comment - The patch adressess the top level deps, but the other (confusing) references remain. Should the dependency be removed or updated in release/pom.xml , too?
Hide
Permalink
Justin Deoliveira added a comment -

Hmmm... yeah probably a good idea. However dependencies defined in the release/pom.xml do not show up in the geoserver webapp, they are used for running jetty, so they end up in the lib folder of the container. So yeah, generally having different versions of xml libs in the container and in the webapp is trouble. But it would be good to test that jetty (the version we are using) still works ok with the upgraded version of xerces.

Show
Justin Deoliveira added a comment - Hmmm... yeah probably a good idea. However dependencies defined in the release/pom.xml do not show up in the geoserver webapp, they are used for running jetty, so they end up in the lib folder of the container. So yeah, generally having different versions of xml libs in the container and in the webapp is trouble. But it would be good to test that jetty (the version we are using) still works ok with the upgraded version of xerces.
Hide
Permalink
Ian Schneider added a comment -

Patch removes other versions, incorporates Ben's change.

Show
Ian Schneider added a comment - Patch removes other versions, incorporates Ben's change.
Hide
Permalink
Ian Schneider added a comment -

I tested Jetty w/ the GEOS-4652 patch and found no problems (it started and the stuff I tried worked). A quick google yielded no obvious issues regarding jetty and xercesImpl 2.7.1. Geoserver itself of course builds and tests out.

Show
Ian Schneider added a comment - I tested Jetty w/ the GEOS-4652 patch and found no problems (it started and the stuff I tried worked). A quick google yielded no obvious issues regarding jetty and xercesImpl 2.7.1. Geoserver itself of course builds and tests out.
Hide
Permalink
Ben Caradoc-Davies added a comment -

Looks good, Ian.

Just one question: is there any reason to specify the xercesImpl version in the release module now that you have confirmed that 2.7.1 works fine? Or would it be better to remove the version so that it is under dependencyManagement?

I think we are agreed that, either way, you should go ahead and commit.

Show
Ben Caradoc-Davies added a comment - Looks good, Ian. Just one question: is there any reason to specify the xercesImpl version in the release module now that you have confirmed that 2.7.1 works fine? Or would it be better to remove the version so that it is under dependencyManagement? I think we are agreed that, either way, you should go ahead and commit.
Hide
Permalink
Ian Schneider added a comment -

Thanks Ben. I'm not sure about the release module and whether the entry there is required.

As for committing, I do not have commit access.

Show
Ian Schneider added a comment - Thanks Ben. I'm not sure about the release module and whether the entry there is required. As for committing, I do not have commit access.
Hide
Permalink
Andrea Aime added a comment -

Ian, you can ask for commit access on the mailing list, should be quick, just say it's just for committing patches once they are reviewed by some core dev and you're gold (copyright assignment should be covered by your employer I think, but just to be sure double check)

Show
Andrea Aime added a comment - Ian, you can ask for commit access on the mailing list, should be quick, just say it's just for committing patches once they are reviewed by some core dev and you're gold (copyright assignment should be covered by your employer I think, but just to be sure double check)
Hide
Permalink
Ian Schneider added a comment -

fixed in r16213

Show
Ian Schneider added a comment - fixed in r16213
Hide
Permalink
Andrea Aime added a comment -

Mass transition all resolved issue that did not see any further comment in the last month to closed status

Show
Andrea Aime added a comment - Mass transition all resolved issue that did not see any further comment in the last month to closed status

People

  • Assignee:
    Andrea Aime
    Reporter:
    Ian Schneider
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: