[MCOMPILER-391] Use dep mgmt when resolving annotation processors and their deps

[psiroky]

• dependency managemet can be used for two slighty difference use
  cases (when it comes to annotation processors):
  -- getting the version of the top-level processor path elements
  -- passing the list of managed dependencies to maven-resolver
  when resolving the whole processorpath
  These two can be combined, so there is total of 4 valid combinations
  (some of them make more sense than the others, but generally there is
  no reason to not support all of them)

• using dependency management when resolving transitive dependencies of
  annotation processors is something that may or may not be desired.
  For that reason, there is a new flag that explicitly enables this
  behavior, while default is to not use dependency management
  (the current behavior)

• do not be scared by the the number of new lines. 95% of that are ITs, and of that like 50% are license headers 😄

These are the 4 use cases I am considering as part of this PR:

1. Version of the annotation processor path is explicitly set + useDepMgmt flag = false (default) -- the current behavior, dependency management is not used at all

2. Version of the annotation processor path is explicitly set + useDepMgmt flag = true (this may make the least sense of the combinations, but I see no reason to not allow it)

3. Version of the annotation processor dep is taken from dep mgmt + useDepMgmt flag = false (dep mgmt is when resolving the whole processorpath).

4. Version of the annotation processor dep is taken from dep mgmt + useDepMgmt flag= true (dep mgmt used when resolving the whole processorpath)

Note: the whole logic around annotation processor paths is getting bit out of hand (read more complex) and I think it would make sense to extract this into a separate class. I did want to complicate this PR further though, so that would be something for next PR(s).

Following this checklist to help us incorporate your
contribution quickly and easily:

• Make sure there is a JIRA issue filed
  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.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [MCOMPILER-XXX] - Fixes bug in ApproximateQuantiles,
  where you replace MCOMPILER-XXX with the appropriate JIRA issue. Best practice
  is to use the JIRA issue title in the pull request title and in the first line of the
  commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify to make sure basic checks pass. A more thorough check will
  be performed on your pull request automatically.
• You have run the integration tests successfully (mvn -Prun-its clean verify).

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.

[psiroky]

I decided to proceed with the suggested solution. The PR now contains changes for both use cases:

1. using dependency management to get just the top-level annotation processor path version
2. using dependency management when resolving the transitive dependencies via maven-resolver

There is also a new flag that enables / disables the usage of dependency management for the second use case.

I added some inline comments for things that I am not 100% sure about or things that potentially need more eyes.

[psiroky]

I ran a quick performance test using the Quarkus repo (1k modules, where 500 of them are using annotation processors) and I could not really spot any noticeable difference in the overall build time. I am sure there is more "work" being done (e.g. search those dependency management lists), but it seems it gets lost in the rest of the work being done as part of the build. I ran the following command:

mvn387 clean install -DskipTests -Dversion.enforcer.plugin=3.2.1 -Dversion.compiler.plugin=3.11.1-SNAPSHOT -T8

(using the enforcer 3.2.1 since that is much faster than the default 3.0.0-M3)
with three different configurations:

• default (no dependency management usage) - option 1) above
• using dep mgmt to only get the top-level versions - option 3) above
• using dep mgmt to get the top-level versions and also to resolve transitive dependencies - option 4)

In all cases I got build times ranging from 01:49min to 01:52min and there was no noticeable difference overall.

[slawekjaranowski]

Very good work.
Thanks for comments with explanation.

[graemerocher]

could we kindly ask for an increase in priority on this issue. The fact that Maven is unable to correctly manage annotation processor dependencies like Gradle can is a major disadvantage. It is issues like this that imagine lead to any tool chain that relies on annotation processors moving away from Maven (see Android)

[psiroky]

I can update the docs, based on the comments above. Besides that, the PR should be ready from my point of view. Please let me know if there is anything else that would need changing.

[famod]

Just a heads up: This implementation does not seem to respect relocations, e.g. I was still using https://mvnrepository.com/artifact/org.hibernate/hibernate-jpamodelgen in my plugin config but the Quarkus BOM contains (only) the new and proper group id org.hibernate.orm and not the old one org.hibernate and so the annotation processor path version lookup logic didn't find it. Took me a while to figure out.

PS: I think I never got a relocation hint on the console.

[psiroky]

@famod could you please provide a bit more details, maybe with an example configuration of the plugin? Are you relying on the new dependency management, or specifying the version manually as before?

[famod]

@psiroky sure:

Are you relying on the new dependency management

Yes, that's what I tried and this doesn't work (old groupId):

                <configuration>
                    <annotationProcessorPaths>
                        <path>
                            <groupId>org.hibernate</groupId>
                            <artifactId>hibernate-jpamodelgen</artifactId>
                        </path>

but this does work (new, relocated groupId):

                <configuration>
                    <annotationProcessorPaths>
                        <path>
                            <groupId>org.hibernate.orm</groupId>
                            <artifactId>hibernate-jpamodelgen</artifactId>
                        </path>

The Quarkus BOM I'm importing has this entry (new groupId only!):
https://github.com/quarkusio/quarkus/blob/3.6.4/bom/application/pom.xml#L5012-L5014

[psiroky]

@famod thanks. In this case, I am not quite sure this is supported from Maven itself.

I assume there is no version anywhere in the project depMgmt for org.hibernate:hibernate-jpamodelgen and so there is also no information which could map it to the new artifact org.hibernate.orm:hibernate-jpamodelgen. That mapping information is part of the old / original POM, but since the plugin does not have the version for this old artifact, it cannot lookup the POM with the relocation config.

Does that make sense? Or am I overlooking something in the setup?

Or this perhaps something that is working for you outside of the compiler plugin configuration with standard project dependencies?

[famod]

@psiroky

Yeah, I think you have a very valid point: There is no explicit version for that old groupId and so there is no precise way to find the exact pom with the relocation info (=> new groupId).
As a human you could just check the latest version for the old groupId and get the relocation info from that but I doubt there is a nice way of finding the latest version via Maven builtin features (?).
Not worth the hassle.

For extra user-friendliness this plugin could check (if no version is found) for similar deps, e.g. same artifactId but different groupId and print put all those candidates, but relocations are not limited to groupIds (AFAIK), so...

It probably boils down to the question why there are/were no relocation warnings like you get for "normal" dependencies. For general plugin dependencies that's clearly something that Maven should cover.
annotationProcessorPaths is probably a different story.

[psiroky]

Yeah, there could possibly be some sort of heuristics to try to detect these relocations. Not sure if this would be something that should be done directly in the plugin or rather left to the dependency resolver.

I think the missing relocation warning has the same cause - the plugin (or more precisely the dependency resolution mechanism the plugin uses) simply does not know that the artifact was relocated, because without version, it cannot fetch the relocation config.

I believe the relocation warning gets printed in case you depend on the old artifact, but the version must be known, explicitly or via depMgmt: e.g:

<dependency>
  <groupId>org.hibernate</groupId>
  <artifactId>hibernate-jpamodelgen</artifactId>
  <verison>6.0.0</version>

In that case, the POM is fetched, relocation config found and the warning can be printed.

[famod]

I think the missing relocation warning has the same cause - the plugin (or more precisely the dependency resolution mechanism the plugin uses) simply does not know that the artifact was relocated, because without version, it cannot fetch the relocation config.

I believe the relocation warning gets printed in case you depend on the old artifact, but the version must be known, explicitly or via depMgmt: e.g:

<dependency>
  <groupId>org.hibernate</groupId>
  <artifactId>hibernate-jpamodelgen</artifactId>
  <verison>6.0.0</version>

In that case, the POM is fetched, relocation config found and the warning can be printed.

I guess I should have been more precise:

I had exactly that (only a version property instead of the version literal, but that shouldn't make any difference) and still no warning. Maven 3.9.6, btw.

[psiroky]

Ah, I see. In that case you are right, the relocation message should be there. I'll see if I can find some time to take a look at this.