Drop MAT

[cstamas]

As Maven 3.0 is not being supported, no need for artifical indirection
between sonatype/eclipse package for resolver. Drop MAT as it
drags a LOT of dependencies.

[olamy]

the idea is to keep using org.apache.maven packages which hide the real implementation (which may change again in the future to use again org.apache.maven namespace).

[cstamas]

the idea is to keep using org.apache.maven packages which hide the real implementation (which may change again in the future to use again org.apache.maven namespace).

That WAS the idea when sonatype/eclipse package change happened. And yes, resolver may around 3.x.x change package from org.eclipse to org.apache (planned around maven 5!), but that's irrelevant due fact that Maven 4.x introduces new maven API, while 4.1 is planned to hide the resolver completely and force plugins to use new API only (so a 3.x maven plugin will work with maven 4.0.x while not with maven 4.1.x that offers new API only).

In short, this plugin will once need a change from direct-resolver use to new Maven API use, and from that point on, Maven plugin will not be bothered anymore with direct use of resolver.

[olamy]

yup so maven-resolver will change package again.
so I'm -0 to change this (again) as we will have to change again in the future.
Whereas here we have stability and not impacted by Maven internal changes.
As all of Maven plugin hosted at Apache does.

[cstamas]

yup so maven-resolver will change package again. so I'm -0 to change this (again) as we will have to change again in the future. Whereas here we have stability and not impacted by Maven internal changes. As all of Maven plugin hosted at Apache does.

Well, you don't get the point: when the package changes for resolver (planned for Maven 5!), you will be unable to reach resolver and maven internals anyway, as it will be replaced by Maven API (to be introduced in Maven 4). This change merely get's rid of huge dependency trail that MAT pulls in, like sonatype resolver, eclipse resolver, plexus-container-default and so on.... cuts off quite a nice bit of plugin dependencies, that's all. And Apache plugins are getting rid of MAT as well, now that Maven prerequisite is raised to 3.2.5 (sonatype is Maven 3.0, so no more need for MAT).

[olamy]

No sorry but I perfectly understand what you want to change.
I just don't like the idea about changing from A to B to get back on A then.
regarding dependencies there are already here and in use I do not see much problems.
well the back to A will be probably in few years...

[cstamas]

I think you completely miss the point:

• MAT is not needed unless you support Maven 3.0.x to bridge sonatype/eclipse resolver package rename. Do you support Maven 3.0.x?
• Maven 3.1+. line of Maven has no API, so directly using Resolver API is the way to go in these versions (this does this PR, drops the non-trivial MAT from dependencies and uses Resolver API directly)
• "back to A in few years" is again, wrong: Maven 4.x will introduce Maven API, while Maven 4.something.x will seal of Maven internals from plugins (so as with Maven 3, this will yield a new line of 4.x versioned plugins), but again, this API is WIP and is not (nor will be) present in Maven 3.1+ line of Maven, only in 4+. But you do support Maven 3 line, don't you?
• finally, when package change of resolver happens around Maven 5, it will not matter for plugins at all, as they will be sealed off from Maven internals (unless your plugin integrates directly with resolver).

[joakime]

I'm ok with this change personally.
I'm convinced by @cstamas arguments.

Now, why is the PR failing to build/test ?

[cstamas]

Hm

java.lang.AbstractMethodError: Receiver class org.eclipse.aether.internal.impl.Maven2RepositoryLayoutFactory$Maven2RepositoryLayoutEx does not define or inherit an implementation of the resolved method 'abstract java.util.List getChecksumAlgorithmFactories()' of interface org.eclipse.aether.spi.connector.layout.RepositoryLayout.

Seems I removed MAT (that pulled in versions of resolver as transitive) but forgot to "align" them now....

[cstamas]

Found it, is unrelated to this change it seems (but it triggered it). It is test-distribution module, mixed resolver versions:

Incomplete declaration, as maven resolver provider is before resolvers in POM, and provider wins with 1.6.3. Fix: pull in impl of resolver as well with fixed version.

[cstamas]

But interestingly, locally I cannot reproduce this

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 30.445 s - in org.eclipse.jetty.tests.distribution.session.MongodbSessionDistributionTests
[INFO] 
[INFO] Results:
[INFO] 
[WARNING] Tests run: 66, Failures: 0, Errors: 0, Skipped: 1
[INFO] 
[INFO] 
[INFO] --- maven-jar-plugin:3.2.2:jar (default-jar) @ test-distribution ---
[INFO] Building jar: /home/cstamas/Worx/eclipse/jetty.project/tests/test-distribution/target/test-distribution-10.0.10-SNAPSHOT.jar
[INFO] 
[INFO] --- jacoco-maven-plugin:0.8.8:report (jacoco-site) @ test-distribution ---
[INFO] Loading execution data file /home/cstamas/Worx/eclipse/jetty.project/tests/test-distribution/target/jacoco.exec
[INFO] Analyzed bundle 'Jetty Tests :: Distribution Tests' with 0 classes
[INFO] 
[INFO] --- maven-source-plugin:3.2.1:jar-no-fork (attach-sources) @ test-distribution ---
[INFO] Skipping source per configuration.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  07:18 min
[INFO] Finished at: 2022-06-21T15:57:58+02:00
[INFO] ------------------------------------------------------------------------
[cstamas@urnebes jetty.project (jetty-10.0.x-drop-mat)]$ 

[cstamas]

Created #8187

[olamy]

I have concern here about changing again and then again about package.
Not sure we want to reach some points where the plugin will not work anymore with maven 3.x but only with 4.x.
or 4.1.x and not 4.x etc...
I prefer avoid such changes here as we do not have per maven version branch model but per jetty core version branch model.
I can't really imagine tell our users hey you can use jetty-maven-plugin 10.0.x only if you use maven 4.x or jetty-maven-plugin 10.0.x will work with maven 3.x but not maven 4.x.

[joakime]

As it stands the jetty-maven-plugin only works reliably on Maven 3.8.0+
So I don't see this as a problem or a valid concern.

[cstamas]

Hm, this time Caused by: java.net.SocketException: Network is unreachable, seems unrelated to this PR.

[joakime]

I'm happy with this change.

If we suddenly have an uproar about not supporting ancient versions of Maven, we have source control, we can rollback this change in that scenario with relative ease.

[olamy]

I added a comment here apache/maven-install-plugin#31 (comment)
but if you want to change and change again just do it.
I just think it's not a good strategy but that's my own opinion which is not shared so fair enough

[cstamas]

@olamy I really don't get your arguments ("change to change it again"). Especially as there was already a package change in MAT, blessed and merged by you #4381 🙄

You (should) are aware that the 2nd "change it again" does not exists yes, is yet to be laid down and will come out with Maven 4 (that has even no alpha released yet), and also, as explained in documentation there will be a transitioning window as Maven 4 will work with both, unmodified maven-3 plugins (like this one) and new maven-4 plugins (those using the new API), with cut-off that will happen in some undefined future (maven 4.1, 4.5 or maybe 5.0, remains to be seen).

[olamy]

My argument is to use a org.eclipse package coming from an artifact on groupId org.apache.maven and in Apache source repository. The goal of the previous artifact was to hide this total aberration. Now we switch to another ephemeral package name and a total aberration regarding where the hosting is.
And does this change provide any feature?
I don't think so but maybe?
It just lead to a total confusion on the packaging and ownership of this which tends to look at a temporary thing.
Read it as grumpy man whining if you want but I find it an abnormal fact..