Skip to content
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

Java 9 support #71

Merged
merged 14 commits into from Dec 30, 2019
Merged

Java 9 support #71

merged 14 commits into from Dec 30, 2019

Conversation

theit
Copy link
Contributor

@theit theit commented Oct 12, 2018

This pull requests adds support for invocation under Java 9-11 and fixes #54.

- usage of diamond operators and try-with-resources
- changed some long(er) expressions to shorter and more readable Lambda expressions
- minor fixes in AbstractJaxwsMojo#getInvokerCP() to prevent a possible error when classpath is empty
- replaced Oracle JDK 8 with OpenJDK in travis.yml
…en Maven is invoked with OpenJDK 11

- Fixed the pom.xmls of a few IT tests because they didn't run when being executed under OpenJDK 11:
  - added missing dependency to javax.xml.ws:jaxws-api:2.3.1 (issue-12)
  - added missing dependency to javax.jws:javax.jws-api:1.1 (jaxwscommons-43)
  - Java compiler source and target 1.5 are not supported anymore (jaxwscommons-49, jaxwscommons-62 and wsgen test)
  - removed <executable> tag in jaxwscommons-81 because the executables wsgen and wsimport don't exist anymore
  - added the actual classloader in AbstractWsGenMojo#getSEIs():
    The @webservice annotation could not be found during execution; therefore the IT test jaxwscommons-43 did't run under OpenJDK 11
…coded version string depending on the Java version being used
- m-compiler-p configuration is now handled in IT parent pom
- upgraded m-compiler-p to version 3.8.0
Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

great !

@theit theit mentioned this pull request Oct 15, 2018
plexus-utils-3.1.0 that can throw an IOException after printing a
message. This results in a MojoExecutionException when the plugin is
being used in Eclipse
@olgrosTrackit
Copy link

How can I help testing?

@theit
Copy link
Contributor Author

theit commented Oct 24, 2018

How can I help testing?

Well, checkout the code, apply my patches (see above) and check in your project / code whether everything still works ;-)
According to the source code the following (dependency) artifacts are treated as endorsed artifacts, i.e. added to the bootclasspath when the plugin is being used in Java 8, i.e. "-Xbootclasspath:..." as command line option in the executed Java process:

  • jaxws-api
  • jaxb-api
  • saaj-api
  • jsr181-api
  • javax.annotation
  • javax.annotation-api
  • webservices-api
  • javax.xml.ws*
  • javax.xml.bind*

When being used in Java >=9, they should be added directly to the classpath. Does everything still work in your environment?

@hugo-ma-alves
Copy link

Hello @theit
Thanks for the fix.

I've download and compiled your branch and it is working on java 11.
Basically we couldn't jaxws-maven-plugin with java 11, it threw the "-Xbootclasspath/p is no longer a supported" error.

Our setup is something like this:

<plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>jaxws-maven-plugin</artifactId> <version>2.6-SNAPSHOT</version> <configuration> <vmArgs> <vmArg>-Djavax.xml.accessExternalSchema=all</vmArg> </vmArgs> </configuration> <executions> <execution> <id>SampleWsdl</id> <goals> <goal>wsimport</goal> </goals> <configuration> <wsdlDirectory>${basedir}/src/main/resources/wsdl</wsdlDirectory> <bindingDirectory>${basedir}/src/main/jaxws</bindingDirectory> <bindingFiles> <bindingFile>bindings_SampleWsdl.xml</bindingFile> </bindingFiles> <wsdlFiles> <wsdlFile>SampleWsdl.wsdl</wsdlFile> </wsdlFiles> <wsdlLocation>/wsdl/SampleWsdl.wsdl</wsdlLocation> </configuration> </execution> </executions> </plugin>

Do you have any idea when this PR will be merged?

@theit
Copy link
Contributor Author

theit commented Nov 21, 2018

Hello @hugo-ma-alves ,

thanks for confirming that my PR works in your environment. Regarding your question: This depends on the maintainers / owners of this project.
@andham and @hboutemy: You have created the last releases. WDYT?

@AlexKovalevich
Copy link

AlexKovalevich commented Jan 7, 2019

Thanks for the PR. This fix does work and solves the issue with -Xbootclasspath/p.
This fix is definitely needs to be merged and the new version of plugin should be released cause it is a major issue for Java9+.

@carlam16
Copy link

I am seeing the same error, any update on this issue ?

@niko-dunixi
Copy link

I am also running into this issue. Any ETA on this pull request? Our organization has mandated that we must have Java 11 for all our applications and this definitely blocks us. I've resorted to using the fork someone was kind enough to publish, but I imagine that keeping everything centralized and merging this will prevent more divergent versions of the plugin as Java continues to evolve

@jackdpeterson
Copy link

Is this project dead? Or is there something that's blocking this from getting merged in that the community can help push forward? I'd love to see this working when working with third-party integrations that are SOAP based :-P

@andham
Copy link
Member

andham commented Sep 7, 2019

The project is not dead, but I guess there isn't much attention to it from us developers. Personally I'm currently not using jaxws. However, I totally understand the need for this to be Java 11 compatible.

@theit
Copy link
Contributor Author

theit commented Sep 7, 2019

Do you need someone to help in supporting the development of this plugin? If yes, I could help out; we're using this plugin in our company...

@andham
Copy link
Member

andham commented Sep 10, 2019

@theit We're always looking for people to help us at Mojohaus maintaining our mojos.
Let's start by getting this release out and we can take it from there. I can perform the actual release, but I will not have time to dig into the actual coding required. And there you can help.
A new release would then focus on adding Java 11 support. I see no reason to support Java 9/10 though.

@andham andham mentioned this pull request Sep 10, 2019
pom.xml Outdated Show resolved Hide resolved
@@ -51,7 +58,6 @@
<goal>wsgen</goal>
</goals>
<configuration>
<executable>${tool.wsgen}</executable>
Copy link
Member

Choose a reason for hiding this comment

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

These changes to this test case worries me a bit. The issue at java.net is no longer available, but the summary for the issue says "Allow 'fork'" (see https://www.mojohaus.org/jaxws-maven-plugin/release-notes-221.html). So possibly the usage of 'executable' is what this test case is all about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you only have Java 11+ on your machine, this test fails because the commands wsgen and wsimport don't exist anymore. Using adoptopenjdk-12.0.2 the following error appears in target/it\ tests/jaxwscommons-81/build.log:
(...) [INFO] jaxws:wsimport args: [-keep, -s, '/Users/thorsten/git/theit-jaxws-maven-plugin/target/it tests/jaxwscommons-81/target/generated-sources/wsimport', -d, '/Users/thorsten/git/theit-jaxws-maven-plugin/target/it tests/jaxwscommons-81/target/classes', -Xnocompile, "file:/Users/thorsten/git/theit-jaxws-maven-plugin/target/it%20tests/jaxwscommons-81/src/wsdl/HelloWs.wsdl"] [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 0.783 s [INFO] Finished at: 2019-09-11T21:39:20+02:00 [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.codehaus.mojo:jaxws-maven-plugin:2.6-SNAPSHOT:wsimport (id4) on project jaxwscommons81: Cannot execute: /Library/Java/JavaVirtualMachines/adoptopenjdk-12.jdk/Contents/Home/../bin/wsimport -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:jaxws-maven-plugin:2.6-SNAPSHOT:wsimport (id4) on project jaxwscommons81: Cannot execute: /Library/Java/JavaVirtualMachines/adoptopenjdk-12.jdk/Contents/Home/../bin/wsimport at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117) (...)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should then keep the test as it was and configure it to only run with Java 8. Preferrably the release is performed with Java 8 and then the test is executed as intended.

* Java8:
  - try-with-resources added for URLClassLoader usage - replaced Oracle JDK 8 with OpenJDK in travis.yml
org.codehaus.plexus:plexus-utils: 3.1.0 => 3.2.1
org.apache.maven.plugin-tools:maven-plugin-annotations: 3.5.2 => 3.6.0

Build plugin updates:
org.apache.maven.plugins:maven-plugin-plugin: 3.5.2 => 3.6.0
- changed oraclejdk9 to openjdk9
@voipfuture
Copy link

voipfuture commented Sep 12, 2019

Just for the record: I'm using the patch for building multiple (admittedly simple in terms of SOAP usage) projects on OpenJDK 11 and it worked as a drop-in replacement. Thanks for your work.

@andham
Copy link
Member

andham commented Sep 13, 2019

What's left now, I believe, is to restore the jaxwscommons-81 IT with some tweaks so it just executes for Java 8 platform. @theit is that something you could fix? Have a look here: https://maven.apache.org/plugins/maven-invoker-plugin/examples/selector-conditions.html
(Also include a clear comment of why.)

@theit
Copy link
Contributor Author

theit commented Sep 17, 2019

@andham: I thought about restoring the jaxwscommons-81 IT as it was, and adding a clone of it with the patches in the pom.xml for Java >= 9. That way it can be executed independent of the JDK you're using. What do you think?

@andham
Copy link
Member

andham commented Sep 18, 2019

Unfortunately the jvnet issue tracker is not available any longer so we can't view the details of that issue (jaxwscommons-81), but the description in the pom states:

wsgen, wsimport - support tools from JDK

so I believe the purpose of the IT is to verify the executable config param. Thus, removing that removes the purpose of the test.
I believe these "executables" are now scripts which are part of the JAX-WS RI (https://javaee.github.io/metro-jax-ws/). So you could have a look at how to auto-install these depending on the OS, and use that for Java 9+. Or maybe use that for alla Java versions. The distro is available as com.sun.xml.ws:jaxws-ri:2.3.2:zip in central repo.

@GabrielBB
Copy link

This fork is working good with Java 13 (released just some days ago) : https://github.com/phax/jaxws-maven-plugin

@andham
Copy link
Member

andham commented Sep 27, 2019

@theit Do you agree with my last comment and do you think it's something you can fix?

@OmarHawk
Copy link

Are there any news on when this could be finished up?

@andham
Copy link
Member

andham commented Nov 19, 2019

We need to update the jaxwscommons-81 IT.

@clagenna
Copy link

clagenna commented Dec 3, 2019

I've tryed with jdk 13 and works like a charm! Good job!

@andham andham added this to the 2.6 milestone Dec 28, 2019
@andham andham merged commit 99449f6 into mojohaus:master Dec 30, 2019
@andham
Copy link
Member

andham commented Dec 30, 2019

I've merged this PR. Thanks for contributing.
However, the jaxwscommons-81 IT needs to be fixed before a release.

@theit
Copy link
Contributor Author

theit commented Dec 30, 2019

Hi,

sorry for the very long delay; I was a bit too busy with $DAYJOB and personal things...

The problem with the IT is that it defines an executable to use for the wsgen(-test) and the wsimport(-test) goals. These exist in Java 8, but not anymore in Java 11. The PR simply removes these entries so that the Mojo calls the Java classes directly instead of the executables in the Java installation which - as far as I understand - only start the JVM and call the same Java classes as the Mojo and feed them with the given parameters...

Anyway. To restore the IT behavior for Java 8 I was thinking about creating two profiles: one for Java <= 8, and one for Java >= 9. They contain the same build/plugins/plugin block but differ only in the definition for the executable that would be available only in the Java <= 8 profile and that is missing in the Java >= 9 profile.

The downside of this is that it would blow up the pom.xml by ~100 lines because the plugin definition contains six execution blocks, and I actually don't see a simpler way to do this.

WDYT?

@andham
Copy link
Member

andham commented Dec 30, 2019

@theit see comment in #71 (comment)

@andham
Copy link
Member

andham commented Jan 9, 2020

For everybody following this, a release vote for v2.6 has started:
https://groups.google.com/forum/#!topic/mojohaus-dev/OeRAr6AxamE

@phax
Copy link

phax commented Jan 14, 2020

And the majority decided unanimously :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin dont work on java 9