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

Remove old and deprecated items, use Resolver API #336

Merged
merged 5 commits into from
Mar 17, 2023

Conversation

cstamas
Copy link
Contributor

@cstamas cstamas commented Feb 16, 2023

As part of user reported issue https://issues.apache.org/jira/browse/MRESOLVER-322 and for upcoming 1.9.5 resolver release went to look what it is about, only to figure out that this plugin accumulate several terrible lag, bad practice and many other things.

Whenever you see "inappropriate dependency" (ie. using MAT in plugin that declares itself as a 3.2.5+ plugin), use of deprecated components (ArtifactFactory, ArtifactRepository as local repository and relatives), or doing hoops and loops to get dependnecy graph ONLY to do something about it, step up and fix it. Or call the Ghostbusters!

These changes makes plugin work as expected from declared "minimum" version (3.2.5) to current stable (3.9.0) with addition that it will support split repository and any other modern feature incoming....

Lack of time prevented me to fully finish this PR.

There is one failing IT:

[ERROR] The following builds failed:
[ERROR] *  profile-with-deps-inherit-parent-depMgmt-flatten-dep-all-active/pom.xml

Someone please pick up this work, or will do it myself when get to it

As part of user reported issue https://issues.apache.org/jira/browse/MRESOLVER-322 and
for upcoming 1.9.5 resolver release went to look what it is about, only to figure out
that this plugin accumulate several terrible lag, bad practice and many other things.

Whenever you see "inappropriate dependency" (ie. using MAT in plugin that declares
itself as a 3.2.5+ plugin), use of deprecated components (ArtifactFactory and
relatives), or doing hoops and loops to get dependnecy graph ONLY to do
something about it, step up and fix it.

These changes makes plugin work as expected from declared "minimum" version
(3.2.5) to current stable (3.9.0) with addition that it will support
split repository and any other modern feature incoming....

Lack of time prevented me to finish this PR to 100%, so there are some
IT failures and some TODOs in code left, but results are:

With Maven 3.2.5:
```
[INFO] Build Summary:
[INFO]   Passed: 72, Failed: 4, Errors: 0, Skipped: 2
[INFO] -------------------------------------------------
[ERROR] The following builds failed:
[ERROR] *  flatten-dependency-all-both-test-and-transitive/pom.xml
[ERROR] *  flatten-dependency-all-provided-children/pom.xml
[ERROR] *  flatten-dependency-all-provided-direct/pom.xml
[ERROR] *  parent-with-version-range/pom.xml
[INFO] -------------------------------------------------
[WARNING] The following builds were skipped:
[WARNING] *  complete-multimodule-parent-pom-cifriendly/pom.xml
[WARNING] *  no-overwrite-3.6.3-later/pom.xml
[INFO] -------------------------------------------------
```

With Maven 3.8.7:
```
[INFO] -------------------------------------------------
[INFO] Build Summary:
[INFO]   Passed: 74, Failed: 3, Errors: 0, Skipped: 1
[INFO] -------------------------------------------------
[ERROR] The following builds failed:
[ERROR] *  flatten-dependency-all-both-test-and-transitive/pom.xml
[ERROR] *  flatten-dependency-all-provided-children/pom.xml
[ERROR] *  flatten-dependency-all-provided-direct/pom.xml
[INFO] -------------------------------------------------
[WARNING] The following builds were skipped:
[WARNING] *  no-overwrite-3.6.2-before/pom.xml
[INFO] -------------------------------------------------
```

With Maven 3.9.0:
```
[INFO] -------------------------------------------------
[INFO] Build Summary:
[INFO]   Passed: 74, Failed: 3, Errors: 0, Skipped: 1
[INFO] -------------------------------------------------
[ERROR] The following builds failed:
[ERROR] *  flatten-dependency-all-both-test-and-transitive/pom.xml
[ERROR] *  flatten-dependency-all-provided-children/pom.xml
[ERROR] *  flatten-dependency-all-provided-direct/pom.xml
[INFO] -------------------------------------------------
[WARNING] The following builds were skipped:
[WARNING] *  no-overwrite-3.6.2-before/pom.xml
[INFO] -------------------------------------------------
```
@cstamas
Copy link
Contributor Author

cstamas commented Feb 16, 2023

And one more thing: use of components that are in "impl" or "internal" named packages is not a recipe for long healthy life. Resolver promises this regarding compatibility: https://maven.apache.org/resolver/api-compatibility.html

@cstamas
Copy link
Contributor Author

cstamas commented Feb 16, 2023

BTW, with Maven 3.2.5 there is one extra IT failing (parent-with-version-range where parent POM uses range for parent version): Are we sure 3.2.5 allowed ranges there? I have a feeling that IT is a fluke, and just need proper skip? Unsure here.

@cstamas
Copy link
Contributor Author

cstamas commented Mar 10, 2023

There is only 1 IT failing, and am unsure is that IT correct or not btw.

@slawekjaranowski slawekjaranowski linked an issue Mar 16, 2023 that may be closed by this pull request
@cstamas
Copy link
Contributor Author

cstamas commented Mar 17, 2023

@slawekjaranowski 🍻

@slawekjaranowski
Copy link
Member

One test fails on Maven < 3.5.2 I skipped it for old version
I hope we don't need more investigation for it

@slawekjaranowski slawekjaranowski changed the title Make up some years of debt Remove old and deprecated items, use Resolver API Mar 17, 2023
@slawekjaranowski slawekjaranowski merged commit 5b757ae into mojohaus:master Mar 17, 2023
<version>0.13.1</version>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-interpolation</artifactId>
<version>1.26</version>
</dependency>
Copy link

Choose a reason for hiding this comment

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

@cstamas I use maven-artifact-transfer in one of the Maven plugins maintained by the Jenkins project and I was unaware that it was deprecated. If it is, what is its replacement and how should I migrate?

Copy link
Contributor Author

@cstamas cstamas Apr 18, 2023

Choose a reason for hiding this comment

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

@basil MAT was introduced to make transition from org.sonatype.aether Java package to org.eclipse.aether possible, and did it by use of Java reflection. This change happened when Aether (today Resolver) moved from Sonatype to Eclipse, between Maven 3.0 and Maven 3.1 (then it moved to ASF, but ASF got permission to use org.eclipse.aether package). The resolver packages will not be changed (for example to org.apache.maven.resolver) UNTIL resolver is also made "hidden" by Maven (like all of it's internals), and the new Maven4 API becomes the ONLY way to interact with Maven (and Resolver).

This is to happen in far future, as similarly like Maven 3.x supported 2.x and 3.x plugins, Maven 4.x will support 4.x (the new API) and 3.x (current, 3.1+ "API") plugins, hence the cut-oif is expected maybe in Maven 5, still undecided (but also is safely far in future).

Hence, if you do NOT support Maven 3.0.x anymore (hopefully not, even ASF raised the bar to 3.2.5, that is still almost 10 years of history), all you have to do is to use Resolver classes "directly", just like this PR does.

Ironically, this "direct use" of resolver classes was considered "bad practice" due Maven 3.0/Maven 3.1 transition fiasco (as plugins became "maven 3.0 only" or "maven 3.1 only", depending on which packages they used), and this got "imprinted" into maven devs, while this is actually NOT true from the moment you declare "supported" Maven versions as [3.1,) (3.1 and onwards). It was almost full 13 years since release of 3.0, Maven Project dropped support for Maven 3.0.x since long time as well. And similar resolver package change is not gonna happen in long term, again, probably in Maven 5 or so.

Copy link
Member

Choose a reason for hiding this comment

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

Can we release mat? I found it useful for code simplification

Copy link

Choose a reason for hiding this comment

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

OK, our minimum Maven version is 3.81 so I filed jenkinsci/maven-hpi-plugin#469. I appreciate your explanation. I have inherited several old Maven plugins and have been trying to modernize them, but it has been very challenging for me since I am an outsider to the Maven developer community and these transitions are not documented very well. That makes your explanation particularly helpful. Thanks!

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 accept to not support Maven 3.0.x line (and anything older), it is totally safe to go for "direct use" of Resolver classes. Maven 4 introduces "new API" that will coexists with "old Maven 3.1+ API (am including resolver API here as well)" during Maven 4.x lifespan to offer transition period. It will be "cut off" (still undecided) probably in Maven 5, when the "new API" becomes the only way to interact with Maven, while all of it's internals and guts (unlike today, where you can access or inject just anything from Maven) becomes hidden from plugins.

Still, to be on safe side, please read this https://maven.apache.org/resolver/api-compatibility.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we release mat? I found it useful for code simplification

Problem with MAT is that it uses Maven2 ArtifactRepository type for a local repository of "modern maven3 library" (org.sonatype or org.eclipse, does not matter). See issues:

https://issues.apache.org/jira/browse/MNG-7706
https://issues.apache.org/jira/browse/MNG-7663

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