Skip to content

Conversation

@sbliven
Copy link
Member

@sbliven sbliven commented Nov 11, 2021

There was an issue with running CeSymm on java 15+ (rcsb/symmetry#104). I suspect that this issue effected all projects that include biojava with the maven-shade-plugin. I was able to trace the problem back to the old com.sun.xml.bind implementations. Switching these to the glassfish JAXB implementation fixed the problem.

  • Downgrade jgrapht to maintain java 8 support
  • Switch JAXB to glassfish implementation for better java 15+ support

- Downgrade jgrapht to maintain java 8 support
- Switch JAXB to glassfish implementation for better java 15+ support
Copy link
Contributor

@josemduarte josemduarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! good catch about jgrapht dependency. I think we'll need a new bugfix release soon to keep the java-8 compatibility.

See comments below

<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like biojava-aa-prop did not have an xml dependency before. I think this is not needed here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got test failures without it ('No JAXP implementation found'). I think it was serendipitously included before since we were using the com.sun namespace, but that the glassfish implementation needs to be specifically included. I have included it on all modules that import javax.xml.bind.

<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like biojava-core did not have an xml dependency before. I think this is not needed here

<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like biojava-modfinder did not have an xml dependency before. I think this is not needed here

This also works with Java 8
Copy link
Contributor

@josemduarte josemduarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@sbliven sbliven merged commit ebb6eb0 into biojava:master Nov 16, 2021
@sbliven sbliven deleted the fix-jaxb branch November 16, 2021 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants