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

Maven 3.9.2 displays warnings with version 2.2.3 #634

Closed
1 of 3 tasks
OrangeDog opened this issue May 11, 2023 · 12 comments
Closed
1 of 3 tasks

Maven 3.9.2 displays warnings with version 2.2.3 #634

OrangeDog opened this issue May 11, 2023 · 12 comments
Labels
Milestone

Comments

@OrangeDog
Copy link

OrangeDog commented May 11, 2023

Thank you for taking your time to talk with us!

What is this issue about?

  • Bug report
  • Feature request
  • Question

Description

[WARNING]  * org.asciidoctor:asciidoctor-maven-plugin:2.2.3
[WARNING]   Plugin issue(s):
[WARNING]    * Plugin should declare these Maven artifacts in `provided` scope: [org.apache.maven:maven-model-builder:3.0.5, org.apache.maven:maven-core:3.0.5, org.apache.maven:maven-plugin-api:3.0.5, org.apache.maven:maven-model:3.0.5, org.apache.maven:maven-settings:3.0.5, org.apache.maven:maven-artifact:3.0.5, org.apache.maven:maven-repository-metadata:3.0.5, org.apache.maven:maven-aether-provider:3.0.5, org.apache.maven:maven-settings-builder:3.0.5]       
[WARNING]    * Plugin depends on plexus-container-default, which is EOL

Environment information

  • asciidoctor-maven-plugin version: 2.2.3
  • asciidoctorj version: ___
  • Maven, Java and OS version:
Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
Maven home: C:\Users\me\.m2\wrapper\dists\apache-maven-3.9.2-bin\3238cb54\apache-maven-3.9.2
Java version: 11.0.19, vendor: Eclipse Adoptium, runtime: C:\Program Files\Eclipse Adoptium\jdk-11.0.19.7-hotspot
Default locale: en_GB, platform encoding: Cp1252
OS name: "windows 11", version: "10.0", arch: "amd64", family: "windows"
@abelsromero
Copy link
Member

Thanks for the heads up.
No work has started in testing maven 4, after v3 release we can start experimenting, but keeping a branch following milestones takes time.

@OrangeDog
Copy link
Author

It seems to be that as long as you don't get warnings on the new checks being added to Maven 3, everything should just work with Maven 4.

@abelsromero
Copy link
Member

It seems to be that as long as you don't get warnings on the new checks being added to Maven 3, everything should just work with Maven 4.

I though so, but the mentioned dependencies are not directly used, so at first glance, I am unsure how to proceed here. Need some time look for official docs.

@snicoll
Copy link

snicoll commented May 25, 2023

mvn:dependency-tree should tell you where plexus-container-default comes from.

@abelsromero
Copy link
Member

abelsromero commented May 25, 2023

I could get the warnings with mvn -Dmaven.plugin.validation=VERBOSE and those don't show in the current main branch (future v3.0.0). We already changed some dependencies to provided some time ago, and apparently we got them right 🎉

Also, good news, the first manual testing of main branch with Maven v4.0.0-alpha-5 works so far, I found some issues with the tests and created a milestone to mark problems with v4.x. Based on that I am considering this issue related to Maven v3.9 and the asciidoctor-maven-plugin v3.0.0.

I'd add v4 to the pipeline and see if we can make main branch fully compatible with v3.x and v4.x 🤞 I want to avoid an experimental branch, those require too much work rebasing.

@abelsromero abelsromero added this to the 3.0.0 milestone May 25, 2023
@snicoll
Copy link

snicoll commented May 26, 2023

@abelsromero the warnings are really annoying so please consider backporting that in a maintenance release. Waiting for 3.0 doesn't sound right IMO.

I believe this issue should be split in two. While making sure the plugin works with Maven 4 is for sure something that can happen in 3.0, getting rid of the warnings are very different as they outcomes something that Maven considers to be a misuse of the API.

@abelsromero
Copy link
Member

@abelsromero the warnings are really annoying so please consider backporting that in a maintenance release.

Sure thing, it's a matter of time, I can't commit to a date but I hope no more than 2 weeks 🤞

@OrangeDog If you don't mind I'll re-purpose this issue for the v2.2.4 fix. If you find any specific issue with Maven 4, feel free to open another issue and I'll tag with the new "Maven v4" milestone.

@abelsromero abelsromero changed the title Maven 4 compatibility / Maven 3.9 warnings Maven 3.9 warnings May 26, 2023
@abelsromero abelsromero changed the title Maven 3.9 warnings Maven 3.9.2 warnings with version 2.2.3 May 26, 2023
@abelsromero abelsromero changed the title Maven 3.9.2 warnings with version 2.2.3 Maven 3.9.2 displays warnings with version 2.2.3 May 26, 2023
@abelsromero abelsromero modified the milestones: 3.0.0, 2.2.4 May 26, 2023
@OrangeDog
Copy link
Author

As far as I know. everything should work in Maven 4 as long as there are no warnings in the latest Maven 3.

@abelsromero
Copy link
Member

I could fix all of the warnings, but those are workarounds. The docs say it's for Maven 4 compatibility, but that's not the case here. For example, I had to exclude a dependency marked as EOL that during a build will be added anyway by a maven-site-plugin transitive.

I will apply them anyway to avoid concerns for users but, these fixes don't offer any assurance of compatibility with Maven 4. We'll still need to work on a branch and experiment with the alpha releases.

@OrangeDog
Copy link
Author

OrangeDog commented May 27, 2023

@abelsromero that means all you have to do is update maven-site-plugin. No workarounds are needed.

Then the warnings will be gone, and it will work in Maven 4.

@abelsromero
Copy link
Member

abelsromero commented May 27, 2023

I did not want to go into much detail, but to clarify my points let me go into a long post format.
For anyone curious to know more, I am happy to answer any question or concern.


@abelsromero that means all you have to do is update maven-site-plugin

Sadly it's not that simple.
The warnings you get depend on how you are using the asciidoctor-maven-plugin, wheter as mojo or maven-site module.
For the asciidoctor-maven-plugin v2.2.x we bundle both the normal mojos and the site maven-site-plugin in the same jar (that has been addressed for future v3.0.0, you can see the main branch has different maven modules). For the site integration, the asciidoctor plugin jar library acts as a simple extra dependency for the maven-site-plugin, so it's normal you don't see issues because we are not being run as a plugin and only warning about the actual plugins are reported. However, the jar uses certain dependencies required for the site generation (the "doxia" framework to be precise). Those, in turn, pull other transitive dependencies which are the ones flagged as EOL when run as a plugin...:bear: Bear with me...so, when running the mojo (not with maven-site-plugin, but when using the actual asciidoctor-maven-plugin as a plugin) the validation detects the EOL being pulled from "doxia" which in reality is not required at all for normal mojos. Scoping as provided doesn't fix that btw.

You can see now:

  • We get a false positive because the plugin required transitive dependencies that are not used. But when actually used in the maven-site-plugin, we don't actually control whether they are used or not in reality, they are part of the doxia runtime.
  • The only solution we can do is to exclude the EOL library , and run an integration test hoping that maven-site-plugin configures the correct runtime. ℹ️ And I just checked and they are doing the same on the maven-site-plugin side.

On the topic of dependencies with invalid scope, all follow the same pattern as maven-model. This is pulled from maven-core dependency which contains the key interfaces to implement a Maven plugin and that should be enough. You don't want to define every single dependency in your pom (even more, ideally you want a BOM to even forget about versions, but this is not there yet), so you trust in some transitive pulls. However, in this case, the transitive maven-model is pulled as compile scope, so our fix is simply overriding it with the provided scope.


After seeing both issues and solutions, users can understand now how I don't see this as any guarantee of Maven v4 compatibility. The only real thing options are (a or b or both):
a) Running current code against Maven4 CLI -> I already did that locally successfully, I am considering adding v4 pre-releases to CI, but I am not fully convinced given the early stages of development (based on the fact releases are called alpha).
b) Build a dedicated branch against the alpha pre-releases already available (https://central.sonatype.com/artifact/org.apache.maven/maven-core/4.0.0-alpha-5)

On top of that, to provide some degree of compatibility with older versions, we build against Maven 3.8.5 (1 year old version) and test in CI against newer Maven CLIs. Thanks to the magic of Java binary compatibility and amazing Maven's Team effort to not respect semantic versioning (reminds me of the "we do not break userspace" Linux moto ❤️ ) things work.

In conclusion: seeing warnings does not mean things will work for Maven v4, and as a matter of fact, not seeing warnings doesn't either. That means the affirmation Then the warnings will be gone, and it will work in Maven 4. is not true.
If anything, putting a release addressing them means the plugin is supported and I am 100% on board with that.


To understand my interest now in v4...
We have already been experimenting with maven-site-plugin alphas and the truth is the Java interfaces are in flux and there are unrelated issues popping up. The effort from the Maven team is huge and my sincere appreciation for them, I wish I could help them but at the end of the day, I need to prioritize to make the most of my time. And having to spend time to test and fix every alpha is not worth it to me. I'd rather wait until we have some stable release candidates or even the final version, that's my meaning of the initial post #634 (comment).


Someone may ask ¿why not work upstream to fix things?

Again, it's a matter of time. This a voluntary project run in spare time. And I already dip my toes in Maven source code. But there's a steep learning curve, only getting to Plexus injection is not easy, after that comes all the other maven and doxia modules.
I trust the team, they sure know what they are doing, but I also know it's extremely hard and will take some time. I don't think people realize how much work is being put into cleaning internals and fixing legacy code already and that by a team of not too many people. That meme of the whole internet being supported by a few ppl working on their spare time is very accurate for the Maven team.
From my point of view, I don't think v4 to be released anytime soon (happy to be proven wrong).


Finally, for peace of mind on the topic of maven-site, we are already running integration tests against the latest maven-site-plugin 3.12.1. And I am adding a test for older versions to also ensure we are still compatible with not only the latest. I want to split that branch into different PRs for git history reasons, that's the only reason why it hasn't been formalized in a PR yet.

abelsromero added a commit that referenced this issue May 28, 2023
* Apply Maven workarounds to dependencies to remove warnings
* Set maven-resources-plugin version to 3.3.1

Fixes  #634
@abelsromero
Copy link
Member

Release v2.2.4 is out! Should appear in maven central at any moment.

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

No branches or pull requests

3 participants