Issue #497: adopt build for java 11 #498

whyicantusemyemailasusername · 2019-02-16T15:45:36Z

Various fixes to make maven build work on java 11 (see #497) :

lombok version update. it is used in single file (test); can be removed completely from the project
add explicit jaxb dependencies where required; see https://stackoverflow.com/questions/43574426/how-to-resolve-java-lang-noclassdeffounderror-javax-xml-bind-jaxbexception-in-j
fix javadoc issues (closing
tags, wrong @see syntax, html entities)
update mockito versions
update maven plugin versions

- explicit jaxb dependency - bump maven plugin versions - update mockito version

- javadoc error fixes: html entities (& --> & etc), <p> tag

- jaxb dependencies for ehcache3

- javadoc issues: wrong @see tag format - lombok version update

coveralls · 2019-02-16T15:52:08Z

Pull Request Test Coverage Report for Build 1057

0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.003%) to 21.679%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pippo-core/src/main/java/ro/pippo/core/util/StringUtils.java	0	2	0.0%

Totals
Change from base Build 1055:	-0.003%
Covered Lines:	1392
Relevant Lines:	6421

💛 - Coveralls

decebals

I prefer that this PR to contain only things about Java11. For other things about javadoc please submit a separate PR. Thanks!

decebals · 2019-02-16T20:21:48Z

...-controller-parent/pippo-controller/src/main/java/ro/pippo/controller/ControllerHandler.java

@@ -462,7 +462,7 @@ protected void handleDeclaredThrownException(Exception e, RouteContext routeCont
    /*
     * Cleans a complex content-type or accept header value by removing the
     * quality scores.
-     * <p/>
+     * <p>


otherwise javadoc tool fails. See https://stackoverflow.com/questions/5260368/which-tag-should-be-used-as-paragraph-separator-in-javadoc and https://stackoverflow.com/questions/5260368/which-tag-should-be-used-as-paragraph-separator-in-javadoc

decebals · 2019-02-16T20:22:48Z

pippo-controller-parent/pippo-controller/src/main/java/ro/pippo/controller/util/ClassUtils.java

     * does <b>not return bridge methods</b>, in other words, only the methods of
     * the class are returned. <b>If you just want the methods declared in
-     * {@code clazz} use this method!</b></p>
+     * {@code clazz} use this method!</b>


Without </p>, are you sure?

decebals · 2019-02-16T20:25:30Z

pippo-core/src/main/java/ro/pippo/core/Pippo.java

     * <pre>
-     * <@code>


https://stackoverflow.com/a/19302439

this particular entry has incorrect <@code> syntax (instead of ${@code})

whyicantusemyemailasusername · 2019-02-16T20:48:09Z

I prefer that this PR to contain only things about Java11. For other things about javadoc please submit a separate PR. Thanks!

I can do that, however w/o javadoc fixes build will fail (looks like javadoc tool from java 11 reports invalid tags like <p/> as errors and aborts the build).

whyicantusemyemailasusername · 2019-02-16T20:51:20Z

actually javadoc checks is feature of java 8, enabled by latest version of maven-javadoc-plugin: https://stackoverflow.com/questions/15886209/maven-is-not-working-in-java-8-when-javadoc-tags-are-incomplete

decebals · 2019-02-16T21:17:47Z

Thanks for clarification! I think that you are right.

whyicantusemyemailasusername · 2019-02-16T21:24:36Z

I've got another branch with non-javadoc changes cherry-picked (and disabled docLink feature) - here. Do you want me to make a separate PR for it (and decline this one)?

decebals · 2019-02-16T21:33:18Z

In another project initiated by me (PF4J), now few days ago someone came with a PR about how to make the project build on Java 11. This is a sign that Java 11 is on an ascending trend from adoption point of view. In this context this PR is welcome.

I think that the new dependency introduced by this PR:

<!-- Jaxb -->
<dependency>
    <groupId>javax.xml.bind</groupId>
    <artifactId>jaxb-api</artifactId>
    <version>${jaxb.version}</version>
</dependency>

must be placed in the context of Java 11. So, this dependency will be added only when Java version is 11, using a Maven's profile. As example see this.

whyicantusemyemailasusername · 2019-02-16T21:50:28Z

I don't think that moving jaxb dependency into separate profile is a good idea. It will lead to two different build artefacts - one with jaxb jars (if built on java 11) and another one without.
Former can be run on java 8+, latter can be run on java 8 only - java 9 will issue warnings about missing modules and java 11 will fail as jaxb modules doesn't exist there.
In other words, you'll loose forward compatibility.

whyicantusemyemailasusername · 2019-02-16T21:52:04Z

#499 raised

whyicantusemyemailasusername · 2019-02-16T22:53:42Z

I don't think that moving jaxb dependency into separate profile is a good idea. It will lead to two different build artefacts - one with jaxb jars (if built on java 11) and another one without.
Former can be run on java 8+, latter can be run on java 8 only - java 9 will issue warnings about missing modules and java 11 will fail as jaxb modules doesn't exist there.
In other words, you'll loose forward compatibility.

Example of failure: build server has jdk8, production server has jre 11.

whyicantusemyemailasusername · 2019-02-18T17:14:10Z

closing this in favour of #499

decebals · 2019-02-18T20:26:18Z

I don't think that moving jaxb dependency into separate profile is a good idea. It will lead to two different build artefacts - one with jaxb jars (if built on java 11) and another one without.
Former can be run on java 8+, latter can be run on java 8 only - java 9 will issue warnings about missing modules and java 11 will fail as jaxb modules doesn't exist there.
In other words, you'll loose forward compatibility.

Example of failure: build server has jdk8, production server has jre 11.

I understand very well what you say but I am not convincied yet that to add a dependency (for java 8 and java 11) is the correct solution.
What you say about actual situation when java 8 is used? After the modification proposed by you
(with new dependency), we have a new jar (from dependency) whith classes that are already present in Java 8 (Java version used in production).
I think that the current version (1.12.0) of Pippo works with Java 11 out of the box, if you add the dependecy to JAXB in your project. JAXB is used in pippo-core only in Error class and I don't know if it's not a good idea to remove JAXB from pippo-core module.

whyicantusemyemailasusername · 2019-02-18T21:01:17Z

You're right. Basically there are several options:

no jaxb dependency, pippo users are required to add it to their poms manually for java 9+.
jaxb dependency, users are required to exclude the dependency for java 8.
drop jaxb dependency.
two different builds.
dependency in maven profile.

I fully support 'drop' option. However session-ehcache3 tests fail without jaxb.
Excluding is implemented, for example, in https://github.com/jOOQ/jOOQ/blob/master/pom.xml . For newer VMs there is nothing to do, for older ones - add <exclude> section.
No jaxb dependency is the other way around, except users with up-to-date vm would need to perform additional steps.
Dependency in maven profile is most magical one. I'd prefer to fail fast.

decebals · 2019-02-18T22:34:38Z

I think we should go with option no jaxb dependency, pippo users are required to add it to their poms manually for java 9+ because:

majority of PF4J users use Java 8 (or don't use Java 9+)
don't brake actual applications that use PF4J

So, I believe that it's a good idea to add the dependencies in the pom.xml files (current PR) but in a comment form with a specification that they are useful for Java 9+.

Thanks for your patience! This PR is not so simple and we must somehow to take a decision.

whyicantusemyemailasusername · 2019-02-18T22:51:30Z

Sure :) This also means "maven profile for jaxb to able to build on jdk9+". I'll update the other PR

alexberdnik added 4 commits February 16, 2019 13:48

java 11 compatibility:

f2ec2dc

- explicit jaxb dependency - bump maven plugin versions - update mockito version

java 11 compatibility:

4d450bc

- javadoc error fixes: html entities (& --> & etc), <p> tag

java 11 compatibility:

f09089f

- jaxb dependencies for ehcache3

java 11 compatibility:

0078556

- javadoc issues: wrong @see tag format - lombok version update

decebals reviewed Feb 16, 2019

View reviewed changes

whyicantusemyemailasusername mentioned this pull request Feb 16, 2019

Feature/java 11 build #499

Merged

whyicantusemyemailasusername closed this Feb 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #497: adopt build for java 11 #498

Issue #497: adopt build for java 11 #498

whyicantusemyemailasusername commented Feb 16, 2019

coveralls commented Feb 16, 2019

decebals left a comment

decebals Feb 16, 2019

whyicantusemyemailasusername Feb 16, 2019

decebals Feb 16, 2019

decebals Feb 16, 2019

whyicantusemyemailasusername Feb 16, 2019

whyicantusemyemailasusername commented Feb 16, 2019 •

edited

whyicantusemyemailasusername commented Feb 16, 2019

decebals commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 16, 2019

decebals commented Feb 16, 2019 •

edited

whyicantusemyemailasusername commented Feb 16, 2019 •

edited

whyicantusemyemailasusername commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 18, 2019

decebals commented Feb 18, 2019

whyicantusemyemailasusername commented Feb 18, 2019 •

edited

decebals commented Feb 18, 2019

whyicantusemyemailasusername commented Feb 18, 2019

Issue #497: adopt build for java 11 #498

Issue #497: adopt build for java 11 #498

Conversation

whyicantusemyemailasusername commented Feb 16, 2019

coveralls commented Feb 16, 2019

Pull Request Test Coverage Report for Build 1057

💛 - Coveralls

decebals left a comment

Choose a reason for hiding this comment

decebals Feb 16, 2019

Choose a reason for hiding this comment

whyicantusemyemailasusername Feb 16, 2019

Choose a reason for hiding this comment

decebals Feb 16, 2019

Choose a reason for hiding this comment

decebals Feb 16, 2019

Choose a reason for hiding this comment

whyicantusemyemailasusername Feb 16, 2019

Choose a reason for hiding this comment

whyicantusemyemailasusername commented Feb 16, 2019 • edited

whyicantusemyemailasusername commented Feb 16, 2019

decebals commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 16, 2019

decebals commented Feb 16, 2019 • edited

whyicantusemyemailasusername commented Feb 16, 2019 • edited

whyicantusemyemailasusername commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 16, 2019

whyicantusemyemailasusername commented Feb 18, 2019

decebals commented Feb 18, 2019

whyicantusemyemailasusername commented Feb 18, 2019 • edited

decebals commented Feb 18, 2019

whyicantusemyemailasusername commented Feb 18, 2019

whyicantusemyemailasusername commented Feb 16, 2019 •

edited

decebals commented Feb 16, 2019 •

edited

whyicantusemyemailasusername commented Feb 16, 2019 •

edited

whyicantusemyemailasusername commented Feb 18, 2019 •

edited