Adding module-info.java to non-tests Jetty modules

[sbordet]

Fixes #2978, adding module-info.java to non-tests Jetty modules.

[joakime]

Looks like this branch could use a rebase against jetty-10.0.x (to eliminate the noise of merges)

[joakime]

This seems awkward.
@olamy got any ideas on how to do this more clearly? and not via the local repository?

[olamy]

sadly no...... I would prefer using the directory within reactor (--patch-module can accept directories as well as file but I don't any to say use project.build.outputDirectory of those modules...) well you need to install first to get this working with ide... (not ideal...) Definitely something to add in maven-compiler-plugin

[olamy]

I would even prefer using <goal>copy-dependencies</goal> to copy the jar in the target directory. So at least IDE can deal with that. Otherwise before running test with your IDE you need to first install the jars

[sbordet]

@joakime @olamy I have split the test utility classes of jetty-http into a new module jetty-http-test.
This should resolve your concerns (and it's way simpler for JPMS too).

[olamy]

Yeah. and first part of #3080

[olamy]

I don't think so. But maybe leave it temporary until we finish fixing #3080 . Well this will change the jars we publish to Maven central. But is there anyone using http://repo.maven.apache.org/maven2/org/eclipse/jetty/jetty-server/9.4.14.v20181114/jetty-server-9.4.14.v20181114-tests.jar ?

[sbordet]

@joakime @olamy I have removed the usages of tests jars from jetty-http and websocket-core.

The only other place I reference ${settings.localRepository} for JPMS is https://github.com/eclipse/jetty.project/blob/jetty-10.0.x-2978-add_module_info/jetty-quickstart/pom.xml.

I need to patch servlet-api.jar with jetty-schemas.jar to make the jetty-quickstart tests working fine.

Any better idea on how can this be done?

[joakime]

Perhaps it's time to bring back the custom servlet-api Jar.

We could start with https://github.com/eclipse/jetty.toolchain/blob/master/jetty-osgi-servlet-api/pom.xml
Then rename it to be something like jetty-servlet-api.
Followed by appropriate changes for OSGi (the original purpose), and then JPMS (like a reference to jetty-schemas)?
WDYT?

[sbordet]

@gregw is filing an issue to the Servlet Spec to have the standard jar include the schemas.

@janbartel also says that using the toolchain "jetty-osgi-servlet-api" jar may be the way to go: drop "osgi" from the jar name and just call the toolchain jar "jetty-servlet-api".
@joakime can you do what @janbartel suggests?

[joakime]

@sbordet yup, can do.
Created jetty/jetty-toolchain#20 to work on these tasks.
Should have a PR ready at eclipse/jetty.toolchain ready today.

[joakime]

Do we need to update our schemas from the https://github.com/javaee/schemas ?
(Found this github reference at jakartaee/servlet#1 (comment))

[sbordet]

@joakime regarding the jetty-schemas jar included in the servlet-api jar, here is the story.

Jetty loads the XML schemas resources using the context classloader; the JVM finds that no JMPS module defines javax.servlet.resources, so it looks in all the JPMS modules, cracking open their jars and looking for the resource.
If jetty-schemas.jar had a Automatic-Module-Name, it would find these resources.

However, the same resources are loaded also by org.apache.tomcat.util.descriptor.DigesterFactory which tries to load the schema resources using ServletContext.class.getResource(), which loads resources only from the module that exports the class, so in this case the Servlet API jar.

Tomcat does not get warnings (about not being able to find schema resources) because they have their own Servlet API jar that includes the schema resources.

Jetty has the warning because it uses DigesterFactory with the official Servlet API jar that does not have the schema resources in it.

[gregw]

I think jetty-http-tools is a better name
Something called jetty-http-test should test jetty-http
Something called jetty-http-tools contains tools built from jetty-http