Jetty 12.0.x refactor common maven plugin classes

[janbartel]

Refactor all eeX jetty maven plugins to extract common code.

[olamy]

test dependencies are not needed as there are no tests in this module

[janbartel]

Probably should be some tests added at some point.

[olamy]

but not yet. better keep it clean for what we do not need

[olamy]

we will probably/certainly never have tests here :)

[olamy]

no test here so not needed

[olamy]

why optional? seems to be mandatory to have it

[janbartel]

All the others are optional, so I made it the same. But happy to make it mandatory.

[olamy]

same dependency is not optional in ee9 :)
and btw it is really used by the maven plugin

[janbartel]

I've made ee9 optional too, so that they all match.

And yes, this dependency is where all the classes are that are common across ee8/9/10 versions of the plugin.

[olamy]

I've made ee9 optional too, so that they all match.

And yes, this dependency is where all the classes are that are common across ee8/9/10 versions of the plugin.

if it's really used and not depending on what the user will use it's mandatory to have it and not optional.
most of the optional dependencies looks wrong to be optional (but it's not something new I agree :))
why jetty-util is not optional whereas jetty-server is optional?

[janbartel]

I think these were maybe made optional because of the distro way of running the plugin: when we set up the fork for jetty home, we need to copy over any <dependencies> that are configured on the <plugin> so the webapp works. We put out a warning if there are any dependencies that are jetty dependencies (because you shouldn't be adding any jetty dependencies to the plugin). The thinking might have been that these dependencies of the plugin itself might register as dependencies (of the project that uses the plugin) and therefore trigger the warning. See the code here: https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-ee10/jetty-ee10-maven-plugin/src/main/java/org/eclipse/jetty/ee10/maven/plugin/AbstractWebAppMojo.java#L541

If you're sure that the dependencies of the plugin itself won't register as dependencies of the usage of the plugin, then we can probably take these optionals out (although the optional setting should only affect people who are trying to use the jetty maven plugin project as a dependency on their own project, which doesn't really make a lot of sense).

[olamy]

do we really need this for jetty-home?

[janbartel]

This seems to be where all the core modules are incorporated into the distro, so yes because there are some classes/modules that are used when the jetty maven plugin starts in distro mode so they have to be part of the distro.

[olamy]

but it;s used only by the maven plugins and they are not part of the distribution so it's not needed to be included in the jetty-home.

[janbartel]

No, the jetty-maven jar is used by the module that the distro runs to deploy the unassembled webapp. See for example: https://github.com/jetty/jetty.project/blob/jetty-12.0.x-refactor-common-maven-plugin-classes/jetty-ee10/jetty-ee10-maven-plugin/src/main/resources/ee10-maven.mod

and also https://github.com/jetty/jetty.project/blob/jetty-12.0.x-refactor-common-maven-plugin-classes/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/AbstractHomeForker.java#L175

[olamy]

ah right those modules which are not part of distro but added dynamically when using the maven plugin.
well currently the jar looks to be copied already as plugin.jar in lib/maven-ee10/*.jar via

jetty.project/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/AbstractHomeForker.java

Line 354 in 8268d56

IO.copy(jarStream, fileStream);

the mod file looks to need to be only

[lib]
lib/maven-ee10/*.jar

as the jar will be added and named plugin.jar

[olamy]

argh. not sure how to do something such

[lib]
${{jetty.base}/lib/maven-ee10/*.jar

[joakime]

The ${jetty.base} portion (btw you have a typo in yours) isn't necessary.
That location will be looked for in all of the configuration sources that are directories that the start.jar is aware of (eg: ${jetty.base} then ${jetty.home}, but you can also add more directories between those two locations via a --add-config-dir=<dir> command line)

[olamy]

that's weird I have

[lib]
${jetty.base}/lib/maven-ee10/*.jar

And

olamy@pop-os:~/dev/sources/jetty/jetty.project$ ls -lrt jetty-ee10/jetty-ee10-maven-plugin/target/it/jetty-start-war-distro-mojo-it/jetty-simple-webapp/target/jetty-base/lib/maven-ee10/
total 88
-rw-rw-r-- 1 olamy olamy 86660 Apr 26 09:14 plugin.jar

But sill CNFE

[janbartel]

I've changed the code to remove the jetty-maven.jar from the distro, and dynamically copied it over into the ${jetty.base}/lib/${ee}-maven directory as is done for the jetty-$ee}-maven-plugin.jar.

[olamy]

yeah. haha I just saw my confusion regrading the file :)

[olamy]

we will probably/certainly never have tests here :)

[olamy]

still some dependencies non needed and with probably wrong scope but it has been like this for years :) and we can live with that.
we can address the non needed test dependencies later