Better support for APT in dev mode

[FroMage]

Fixes #1502

With this, there are three supported ways of running APT:

• Use the APT processor as a provided dep
• Use the APT processor in the compiler plugin, via annotationProcessorPaths
• Use the APT processor in the compiler plugin, via annotationProcessors

This works for Maven, but not Gradle. This needs tests, but not sure where to add them.

Most of this work required passing the right options to javac as part of DEV mode, as well as the proper output folder for generated sources (otherwise mvn would put them somewhere, and our DEV mode somewhere else).

[gsmet]

@FroMage do you plan to continue progress on this for 3.7? 3.7.0.CR1 will be released next Wednesday and this looks like a very nice improvement.

[FroMage]

I'm not sure I'd be able to finish this in time, the Gradle part is still entirely missing, and so are tests.

[FroMage]

I've rebased and implemented the change. What remains is being able to test APT processors, really not sure where and how I could do that. And Gradle, and this is even harder.

Any guidance, @aloubyansky ?

[aloubyansky]

Let's begin with some Maven tests. I suppose you want to test a project in dev mode for that.
In that case, you could create a sample project under https://github.com/quarkusio/quarkus/tree/main/integration-tests/maven/src/test/resources-filtered/projects
Look at how Quarkus and Maven plugin versions are configured to make sure you are testing the current Quarkus project version.
The test itself can be added to https://github.com/quarkusio/quarkus/blob/main/integration-tests/maven/src/test/java/io/quarkus/maven/it/DevMojoIT.java

I guess that could be a starting point.

[FroMage]

Thanks!

[FroMage]

Wow, there's sooo many projects in this test. Also, it's been running for an hour locally 😱 I'll run only my tests in the future 😂

[aloubyansky]

You probably know @FroMage but just in case you can run individual tests with mvn test -Dtest=DevMojoIT#<mytestmethod>

[FroMage]

Yeah, thanks.

While writing the docs, I noticed that QueryDsl is now advising yet another way to declare APT processors, via the com.mysema.maven:apt-maven-plugin plugin, which I was not aware of. I guess we'll also have to support this form of declaration… 🤦

https://github.com/querydsl/querydsl/tree/master/querydsl-jpa#querydsl-jpa

[FroMage]

Actually, based on https://github.com/querydsl/apt-maven-plugin this is legacy, so let's forget it.

[FroMage]

So, while adding a test, I wanted to see if the annotationProcessorsPath could now be used with version management, as discussed in #37477 and realised that yes, it now works, because we're using the latest Maven Compiler Plugin, but either it's not supported in Aether, or I'm not doing it right when looking at the model in DevMojo:

Caused by: java.lang.IllegalArgumentException: version can neither be null, empty nor blank
    at org.apache.commons.lang3.Validate.notBlank (Validate.java:454)
    at org.apache.maven.artifact.ArtifactUtils.notBlank (ArtifactUtils.java:107)
    at org.apache.maven.artifact.ArtifactUtils.key (ArtifactUtils.java:97)
    at org.apache.maven.ReactorReader.findArtifact (ReactorReader.java:101)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve (DefaultArtifactResolver.java:306)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts (DefaultArtifactResolver.java:229)
    at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact (DefaultArtifactResolver.java:207)
    at org.apache.maven.repository.internal.DefaultArtifactDescriptorReader.loadPom (DefaultArtifactDescriptorReader.java:240)
    at org.apache.maven.repository.internal.DefaultArtifactDescriptorReader.readArtifactDescriptor (DefaultArtifactDescriptorReader.java:171)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.resolveCachedArtifactDescriptor (DefaultDependencyCollector.java:538)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.getArtifactDescriptorResult (DefaultDependencyCollector.java:523)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.processDependency (DefaultDependencyCollector.java:410)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.processDependency (DefaultDependencyCollector.java:362)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.process (DefaultDependencyCollector.java:349)
    at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.collectDependencies (DefaultDependencyCollector.java:254)
    at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies (DefaultRepositorySystem.java:309)
    at io.quarkus.maven.DevMojo.readAnnotationProcessorPaths (DevMojo.java:752)
    at io.quarkus.maven.DevMojo.setAnnotationProcessorFlags (DevMojo.java:1525)

[FroMage]

I think I found a way

[FroMage]

Rebased and added tests and dependency management for APT processors as supported by the latest Maven Java Compiler plugin (see #37477).

So now, all that's missing is the Gradle support. Is it required before we merge this, or can we do it in two steps?

[FroMage]

Pushed a fix for the NPE

[aloubyansky]

I think we can add Gradle support in a separate PR

[FroMage]

I think we can add Gradle support in a separate PR

Fine by me. But still, who's our resident Gradle person who can help me with that? Or better yet, do it 😂

[FroMage]

DevMojoIT.testThatNonExistentSrcDirCanBeAdded fails because the "generated sources dir" target/generated-sources/annotations does not exist, and javac is not happy about that, when we invoke it, it checks and does not auto-create it.

I'm not sure why that directory is not created, and who's supposed to create it. I guess probably it's Maven which is supposed to create it. This test is about a non-existent source directory, not output. target/classes is created, though.

I wonder if I should auto-create it, or detect that it does not exist and not set it. Perhaps Maven doesn't create it IFF there's no annotation processor? Let's look at the sources.

[FroMage]

Yeah, the Maven compiler creates it if it does not exist. I do wonder why it does not happen in my case, but I'll just create it and call it a week. https://github.com/psiroky/maven-compiler-plugin/blob/master/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L757

[FroMage]

This latest rebase and bug fix should fix the remaining Gradle test (not support for Gradle, just preventing it from crashing).

[FroMage]

CI failure doesn't seem related, can't reproduce locally, re-running failed job.

[aloubyansky]

I think we can add Gradle support in a separate PR

Fine by me. But still, who's our resident Gradle person who can help me with that? Or better yet, do it 😂

I'll give you some pointers similar to the Maven ones.
Here you can see sample projects https://github.com/quarkusio/quarkus/tree/main/integration-tests/gradle/src/main/resources (the quarkus versions gets filled in when projects are prepare for building)
And here you can see dev mode tests https://github.com/quarkusio/quarkus/tree/main/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode

Unlike DevMojoIT, in Gradle TS we have a class per test.
You can run them from the gradle module with a usual Maven command mvn test -Dtest=<ClassName>.

Hope that helps?

[FroMage]

Hope that helps?

Well, I also have to find out how to declare APT processors in Gradle, and how to extract their info, and where Gradle places generated sources. But sure, it does help, thanks :)

[FroMage]

I suppose I should update documentation as part of this PR too.

[aloubyansky]

Hope that helps?

Well, I also have to find out how to declare APT processors in Gradle, and how to extract their info, and where Gradle places generated sources. But sure, it does help, thanks :)

Here is one more relevant pointer https://github.com/quarkusio/quarkus/tree/main/integration-tests/gradle/src/main/resources/annotation-processor-simple-module

[quarkus-bot]

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[FroMage]

@FroMage do you plan to continue progress on this for 3.7? 3.7.0.CR1 will be released next Wednesday and this looks like a very nice improvement.

Well @gsmet looks like this is ready just in time. I'll merge it, and do docs and gradle support in another PR later.