-
Notifications
You must be signed in to change notification settings - Fork 37
[UIMA-5753] Get UIMA compile on Java 9 and higher #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Update plugins and plugin dependencies to be compatible with Java 9+ - When building on Java 9 and higher, instead of adding `--add-modules java.xml.bind` rather explicitly depend on JAXB dependencies
- Fixed JavaDoc issues appearing when building with Java 9+
- Work around different behavior in generating line breaks in Java 9+
uimaj-core/pom.xml
Outdated
| </plugins> | ||
| </build> | ||
| <profiles> | ||
| <!-- ************ Java 9 enablement ************** --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could name this java 9 or 10 enablement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java 9+ ? ; )
uimaj-core/pom.xml
Outdated
| <dependency> | ||
| <groupId>javax.xml.bind</groupId> | ||
| <artifactId>jaxb-api</artifactId> | ||
| <version>2.3.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think that depending on some earlier version might get rid of that problem with extra new lines creeping in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea if the newline issue is related to JAXB. I rather expect that changes in the XSLT code introduce the newlines because turning off the pretty-print option in one test case fixed it... but well, it might be worth a try.
| <p>The Annotator Interfaces, along with supporting interfaces | ||
| and exception classes.</p> | ||
| <p>The annotator interfaces are as follows: | ||
| <p>The annotator interfaces are as follows:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some sources (e.g. http://www.oracle.com/technetwork/articles/java/index-137868.html ) say to only use the
tag, not the
. Also, I vaguely recall one of the javadoc lint checker tools complaining if you used some of these closing tags. also this https://stackoverflow.com/questions/5260368/which-tag-should-be-used-as-paragraph-separator-in-javadoc says the doclet doesn't like the closingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To embed HTML in a comment, enclose it in single quotes (for inline HTML) or three single quotes to begin a verbatim block (and later again to end it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, my comment should say that the javadoc tool (at least for java 8, maybe for 9/10) complained if you put in the closing paragraph tag '
' . So these probably should not be added?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... I guess "single quote" was not right and I should have said "backtick" (`)
The closing paragraph tag is not a problem. What JavaDoc 8+ does not like is self-closing elements (<br/>, <hr/>, etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind that the closing tag was already there before, but it was in an illegal position after the </ul> - all I did is move it before the <ul>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, +1
| <p>Implementation and Low-Level API for the CAS Interfaces.</p> | ||
| <p>Internal APIs. Use these APIs at your own risk. APIs in this package are subject to change without notice, even in minor releases. Use of this package is not supported. If you think you have found a bug in this package, please try to reproduce it with the officially supported APIs before reporting it.</p> | ||
| <hr /> | ||
| <hr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please run the javadoc with java 9 / 10 to see if any of these closing tags get complaints...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaDoc 9/10 (and actually I think even 8) do not like self-closing HTML elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| </execution> | ||
| </executions> | ||
| <dependencies> | ||
| <dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any way to factor this common set of dependencies into one spot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As UIMA dependencies, we can add them to the parent POM such that all modules inherit them.
However, if individual Maven plugins require these dependencies (such as here), I know of no way to do a single declaration that would affect all plugins. Mind that that each plugin has its own classloader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
mischor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made some comments... a couple of questions...
|
Already answered them ;) |
|
I am unsure if it is a wise idea to have the JAXB dependencies in a profile which activates on a Java 9+ JDK or if it would be better to simply have them as general runtime dependencies, even under Java 8. Do you have any opinion on that? |
|
Note: in case anybody is watching this... the UIMA project presently does not make use of GitHub (and the GitHub/ASF integration). We are basically toying around here to explore what the use of GitHub might contribute to the UIMA project development workflow. |
|
re: JAXB dependencies for Java 8 - In the interest of stability, I would leave these for Java 9/10, and not put them in for Java 8. |
|
I am a bit worried that it makes the build less reproducible. E.g. distributions or fat JARs would end up having different sets of dependencies depending on which JDK was used to build them. |
- Remove closing `</p> - Renamed profile to `java9plus` / also update the comment - Moved profile to parent pom - Changed JAXB scope to `runtime`
|
@mischor so, I made some changes to address your comments:
I also tried running tests with older JAXB and I even tried explicitly adding old Xalan and Xerces versions as dependencies, but none of these measures made the test succeed in their original form. |
|
I'm ok with these changes. Can you summarize on the uima-dev list? |
|
@mischor As I wrote, I tried explicitly depending on Xalan (2.7.2 and 2.7.1) and Xerces (2.9.1) and even Xalan Serializer (2.7.1 and 2.7.2) in the Java9+ profile - to no avail. Note that I assume that Java uses these libraries if they are on the classpath - I didn't explicitly force the use of a specific XML implementation. |
|
This was fixed for Java 11 in UIMA-5753. |
Several changes allowing a successful build of UIMA using a Java 9+ JDK.