[MNG-7266] Remove maven-compat #1487
Conversation
|
maven-site plugin is still maven2 dependant... |
|
@michael-o can we have a taste on new Ms from site suite? |
|
I've noticed in the .svg that the Does this mean we remove the |
|
I think wagon-provider-api is a must as it is used in Maven3 plugins as well... so only compat is the one that is getting booted |
You want to me run the entire test setup of Doxia/MSITE w/o depending on maven-compat or with with PR of Maven 4? |
|
@michael-o if you look at failed CI jobs, they are due msite plugin wanting stuff that is actually removed (maven-compat). Am interested could you -- maybe even within this PR -- set up M versions that hopefully does not depends on maven-compat? |
Ok, I see what you are after. Will look into this. |
|
@cstamas I gave it a spin. A few components rely falsely on maven-compat, this can be removed, but the larger work is on doxia-sitetools which still use classes from maven-compat. I'd like to complete this release trail and then have a look with you at doxia-sitetools how to remove those classes and then the entire stack can be made free of maven-compat. |
|
For start, maven-compat is really just test dep, not needed at runtime |
|
Someone needs to fix maven-plugin-plugin first: |
|
As far as I can see with So Doxia 2.0.0 should be fine with this, but ultimate test will be when this release train is done and I can test more. |
Does this still stands? Or latest one works? |
Both are unrelated. Maven Plugin Plugin remains broken without Maven Compat. |
|
Fix for m-p-p on the way apache/maven-plugin-tools#280 |
|
@cstamas @michael-o one blocker after upgrading to latest site is AbstractProjectInfoReport.java which uses the |
This is something I need to look at as a whole (reporting plugins) with @cstamas when the current Maven Reporting votes are done and Maven Site Plugin is done as well. I am currently working on lifting the reporting plugin to 3.6.3 first, whenever possible. So please bear with me. |
Sure, np. I just had a quick look again at this PR with the latest site plugin, so just wanted to report the outcome. |
Thanks, I did have this on my radar. I assumed that the cleanup in Maven Reporting Impl was enough. It obviously wasn't. |
|
Cruft piled up in Maven should not be underestimated 😄 |
Giovds
force-pushed
the
mng-7266-remove-maven-compat
branch
from
1a5f665
to
2873aed
Compare
|
Preparing Maven 3.6.3 releases are ongoing. As soon as they are finished I will rebase my work on top of these and check all UTs and ITs. |
|
@Giovds please rebase |
Giovds
force-pushed
the
mng-7266-remove-maven-compat
branch
from
2873aed
to
fc333aa
Compare
There was a problem hiding this comment.
I have done some testing. There are two issues we need to work to make this happen:
- We need to prepare a new parent release where all plugins are updated which me and others have released in the last couple of weeks to get rid of Maven Compat (or at least bump that component everywhere):
osipovmi@deblndw011x:~/var/Projekte/maven-project-info-reports-plugin (doxia-2.0.0 *=)
$ git diff -U0
diff --git a/pom.xml b/pom.xml
index cad192d..0515509 100644
--- a/pom.xml
+++ b/pom.xml
@@ -124,0 +125 @@ under the License.
+ <version.maven-plugin-tools>3.13.1</version.maven-plugin-tools>
- We sill have users of Maven Compat classes:
$ for proj in maven-project-info-reports-plugin maven-plugin-tools maven-jxr maven-pmd-plugin maven-javadoc-plugin maven-checkstyle-plugin maven-surefire maven-dependency-plugin maven-invoker-plugin; do grep -r -F -f maven-compat-classes --include='*/src/main/java/**.java' $proj ; done | grep -v org.apache.maven.reporting.MavenReportException
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/AbstractProjectInfoReport.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/DependenciesReport.java:import org.apache.maven.artifact.repository.metadata.RepositoryMetadataManager;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/DependencyManagementReport.java:import org.apache.maven.artifact.metadata.ArtifactMetadataSource;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/DependencyManagementReport.java:import org.apache.maven.artifact.repository.metadata.RepositoryMetadataManager;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/PluginManagementReport.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/PluginsReport.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/ProjectInfoReportUtils.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/RepositoryUtils.java:import org.apache.maven.artifact.repository.metadata.RepositoryMetadataManager;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/RepositoryUtils.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/renderer/DependenciesRenderer.java:import org.apache.maven.repository.RepositorySystem;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/renderer/DependencyManagementRenderer.java:import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/renderer/DependencyManagementRenderer.java:import org.apache.maven.artifact.metadata.ArtifactMetadataSource;
maven-project-info-reports-plugin/src/main/java/org/apache/maven/report/projectinfo/dependencies/renderer/DependencyManagementRenderer.java:import org.apache.maven.repository.RepositorySystem;
maven-plugin-tools/maven-plugin-plugin/src/it/annotation-with-inheritance-reactor/module-abstract-mojo/src/main/java/org/apache/maven/plugins/AbstractFirstMojo.java:import org.apache.maven.artifact.metadata.ArtifactMetadataSource;
maven-plugin-tools/maven-plugin-plugin/src/it/annotation-with-inheritance/src/main/java/org/apache/maven/plugin/coreit/AbstractFirstMojo.java:import org.apache.maven.artifact.metadata.ArtifactMetadataSource;
maven-plugin-tools/maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/metadata/AddPluginArtifactMetadataMojo.java: * @see org.apache.maven.artifact.repository.metadata.ArtifactRepositoryMetadata
maven-plugin-tools/maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/metadata/AddPluginArtifactMetadataMojo.java: * @see org.apache.maven.artifact.repository.metadata.GroupRepositoryMetadata
maven-plugin-tools/maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/metadata/LegacySupport.java:import org.apache.maven.artifact.repository.metadata.ArtifactRepositoryMetadata;
maven-plugin-tools/maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/metadata/LegacySupport.java:import org.apache.maven.artifact.repository.metadata.GroupRepositoryMetadata;
maven-dependency-plugin/src/main/java/org/apache/maven/plugins/dependency/GetMojo.java:import org.apache.maven.repository.RepositorySystem;
with both I need help to remove that dependency. I am willing to do the releases though for those three.
I will try -- meanwhile -- to test the other non-affected components.
Update: Just tried maven-pmd-plugin. Failed as well: Caused by: java.lang.ClassNotFoundException: org.apache.maven.artifact.resolver.ArtifactResolver likely something transitive...needs analysis.
|
Yet another finding: |
|
Is there some form of documentation I can take a look at in order to help migrate the plugins that depend on compat? |
Well, that's the key. Lack of documentation. I would say that @cstamas is the expert here. |
|
@cstamas Are you able to help to replace the code? |
|
Sadly, it is more "case by case", can you point me at some of the plugin you want to convert? |
These two:
|
|
Rules in general:
In essence, whatever "alternative" solution you see, like use of compat, MAT, whatever, there IS usually equivalent functionality in either Maven APIs or Resolver APIs. |
maven-plugin-tools/maven-plugin-plugin does NOT depend on maven-compat, does it? Also, it contains traces of archaic (Maven2 times) when pom was considered "metadata" and plugin had to maintain maven-metadata.xml as well (something not needed since Resolver 1.9.x, as all data deployed generally are "artifacts", no more "metadata" to be handled by client code needed). Unsure what to be done here, maybe just bite the bullet, and raise the m-p-p prerequisite to 3.9.x (this would raise build time Maven requirement for all built plugins, but not runtime requirements) and drop all this cruft. Am looking into the other one. EDIT: the other one still uses MAT.... so convert it to resolver? ... and shared dep tree that is also to be dropped/deprecated. so a LOT of deprecated stuff... |
We could also switch them directly to the Maven 4 API... |
I'd prefer to solve this for 3.x as well to make easier for us. |
We won't be able to get rid of maven-compat in Maven 3.x branch. |
Also fwiw, m-plugin-p does not depend on maven-compat or MAT anymore (and still supports Maven 3 plugins for now). |
Deleting no, but we cannot also replace the code to Resolver with 3.6.3 baseline? |
But that is on m-plugin-tools on master, no? The 3.x branch will remain as-is in this regard? |
No, 3.x is free of those too afaik |
Thanks, let me re-evaluate my findings, they could be just false positives with proper parents. |
|
I did investigate again and now I understand what actually happens: When I run current reporting plugins wich do not depend on Maven Compat I still get CNFE, e.g., RepositorySystem or others because in 967d8fc a bunch of classes were moved to Compat and are now gone. What apparantly was fine with Maven 3.x is now not possible with Maven 4. If all changes in 967d8fc are correct then those plugins aren't compatible with 4 and need a separate branch for 4. |
Do you know which class is required and not found ? |
Oh, sorry |
This means that compat cannot be removed for those plugins? How to proceed in case? |
There are three ways:
The first one, means that we can't just delete the code now, but we can stop providing it by default in the Maven distribution. |
All of these plugins are one Maven 3.6.3 already. I prefer the Resolver approach. Second option. |
Agreed, I think m-site-p is ok, it's maven-project-info-reports-plugin which depends on maven-compat and needs to be updated. |
Actually, adding maven-compat does not work, and switching to |
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MNG-XXX] SUMMARY,where you replace
MNG-XXXandSUMMARYwith the appropriate JIRA issue.[MNG-XXX] SUMMARY.Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.