RFR: JDK-8306819: Consider disabling the compiler's default active annotation processing [v7]

[Romain Manni-Bucau]

On Fri, 6 Oct 2023 21:55:30 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> Change annotation processing to be opt-in rather than opt-out.
>
> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix typos.

Think there is some reasoning shortcut taken there.
The will to avoid to enable annotation processor when not needed is sane and I agree we regularly see apps enabling some of them without needing due to the dependency game/abuse.

That said it would be welcomed to take into account 3 points and not only postpone because the community kind of reject this shift - otherwise it will happen again in a few years:

1. Discovery is needed and enable a loose coupling between processor and applications (referencing the class is not great and couple the provider of the processor with all apps setup, this is undesired until a notion of alias is added to annot proc but not sure it is needed).
2. Warning undesired setup (enablement when not needed) is a good thing, if there is a too high cost (`ServiceLoader` one is not since any run will likely trigger a lot of `getResources` and it is fine, even just plain `ToolProvider` to get `javac`) then defaulting to a mode where run fails is better than disabling blindly (`-proc:failIfOverhead` - indeed a better name is welcomed). That said, due to javac code this is hardly explanable so it would better be "fail if explicitly enabled and unused, warn if unused and implicitly enabled" so pretty close to what we have for years and everyone was happy with but a bit stricter for future uses.
3. Annotation processing is a great Javac feature and I don't think it is an option to make it "hard"/verbose to setup (2 maven executions of compiler plugin for example which would make people using it even slower and harder to setup - incremental support is not trivial for people not understanding what happens under the hood).
4. Annotation processing is far to be dead, debatable probably but lombok, immutables, JPA, dagger, (google) auto, jmh, mapstruct, fusion, ... are good example of very living libraries with some serious user base (records don't solve all issues ;)).
5. Annotation processing is a neat way to solve the reflection challenges `native-image` (graalvm) brings.
6. If the generation is not part of the main compilation cycle it enforces to use 2 "load the model" cycle which is often slower (due to the model loading but also the fact it will not use javac internals to rely on a stable API so in terms of classloading it will be worse, in particular in modular systems like maven).
7. All the point about disabling it are security related but this is the wrong way to fix the security of the build for most builds (all using a dependency manager like ivy/gradle, maven/aether cause these builds will keep all the mentionned issues if they don't use the dependency validation which must be done at build level (by design it cant be done at javac level).

So overall the original statements should be (in)validated and if there is really an issue using `ServiceLoader` maybe switch to a better SPI option (better to request tools/producers to adjust than consumers) rather than disabling the discovery feature.

Side note: not great for users but acceptable if the will is really to disable the discovery, keeping the `full` flag could be an accepable compromise IMHO.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14432#issuecomment-1873404178