[MCOMPILER-548] JDK 21 throws annotations processing warning that can not be turned off

[hgschmie]

Turns out this is just a documentation issue. full works fine but is
not documented.

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-548] - 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

[hgschmie]

Turns out that this might not be sufficient. JDK < 21 does not support "full" as a value, so we may want to check whether the value is "full" and omit it when the JDK is < 21.

Requires more thought.

[bmarwell]

What happens if I do not set it? This seems to contradict the wording from line 276:

If not set, both compilation and annotation processing are performed at the same time.

[hgschmie]

if you don't set it on JDK 21, you will be greeted with a warning that "soon, not setting proc:full will result in no longer doing annotation processing".

It is a very unsatisfying situation:

JDK < 21: proc:full is not allowed, results in error: invalid flag: -proc:full
JDK == 21: proc:full works, a warning is reported without it
JDK > 21: proc:full is required at some point, no more annotation processing without it

which puts the compiler plugin in the unfortunate situation of having to check the version of javac first before either not passing in -proc:full (because it is JDK < 21 and it is set by default) or passing it in for JDK >= 21.

That's why the comment reads "requires more thought". I have not found time to deal with it. Right now, I have a conditional profile in my poms... 😵‍💫

[cstamas]

Hah, just hit the same: I explicitly wanted to disable proc (as am not using it, but unintentionally there ARE APs present on classpath, like Sisu AP is), so I set in TLP proc=none. Cool so far. Then in one module I needed AP, but as parent had proc=none, I had to enable, so I did proc=full, just to figure, that it breaks everything below Java 21...

[bmarwell]

Another solution (which is probably intended by the JDK folks):
I used to do two executions in my projects:

• generate-sources: execute with proc:only
• default-compile: execute with proc:none

Worked like a charm and only changed it because Tamas hinted me to do so (because with this setup, some things like caching, incremental compilation etc. won't work that well or not at all).
However, this solution would not run into those problems at all.

[cstamas]

... and I keep forgetting about this, and for me this sounds like "most correct" solution as well: split steps and use existing phases even we provide.

[rmannibucau]

We can use conditions depending on the javac version

fact is we don't know with javac version what is supported since several vendors did backport full support.

we can add a property for proc parameter and property can be overridden in child modules with empty value.

It is already the case AFAIK (side note being adding properties for compiler args is quite misleading and lead to broken configuration more than it helps IMHO).

-> https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#proc

Think we don't have much choice to split the default compilation in 2 runs if there is at least one processor available (path or serviceloader discovery) and proc is not configured in compiler properties or args.

[slawekjaranowski]

this allow to override apache/maven-parent#157

[bmarwell]

Then we can use a property for that.

My suggestion:
Leave it like in this PR for m-c-p 3. Detection is "smart" and will many builds, because they of course will have something like this already configured.

For m-c-p 4.x use two phases all the time, even if there's no APT. An empty run (no sources) doesn't hurt.

We can define properties for those as well, of course.

[rmannibucau]

Hmm, shouldnt we revert the parent change? except it is a breaking change it is also unexpected IMHO.

side note: this does not allow more than compilerArgs but enables to get conflicting args now ;).

Agree with you @bmarwell if v4 arrives at ~the same time than v3.x+1 since 21 starts to be used and everybody creates the verbose way in the pom which is very unlikely IMHO

[slawekjaranowski]

I feel that we talk about one line in documentation too much 😄

[EwoutH]

Any update on this effort? What's needed to move it forward?