Make mvnd 1.X work and require Maven 3.9.6 to build Quarkus

[manofthepeace]

Fixes #41323

Supersedes #41628

This is a bit of a hack. But it works.

The codebase has a lot of references to maven.multiModuleProjectDirectory not sure if everything does work. But for futureproofness this should be changed, I think.

Occurences;

• core/processor/src/main/java/io/quarkus/annotation/processor/Constants.java
• docs/src/main/asciidoc/maven-tooling.adoc
• docs/src/main/asciidoc/tests-with-coverage.adoc
• independent-projects/parent/pom.xml
• independent-projects/resteasy-reactive/pom.xml
• tcks/resteasy-reactive/pom.xml
• Some places in the maven wrapper.

cc @gsmet

[gsmet]

@manofthepeace did you test with -f and also when building inside a nested directory that contains a reference to this property?

[manofthepeace]

@manofthepeace did you test with -f and also when building inside a nested directory that contains a reference to this property?

What I tried is the following;

• Full build via mvn and mvnd
• from project root; mvnd/mvn -f tcks/resteasy-reactive clean install -Dquickly
• from tcks/resteasy-reactive mvnd/mvn clean install -Dquickly

[manofthepeace]

If you want precises tests done please let me know. But I assume we will hit this somehow; apache/maven#1589 . I think its still what can be done atm without downgrading.

but I think the fix proposed is still needed nevertheless as maven.multiModuleProjectDirectory is not coming back

[gsmet]

Do you happen to know which versions support session.rootDirectory?

I stumbled upon https://issues.apache.org/jira/browse/MNG-7038 but it's only marked for Maven 4.

[manofthepeace]

This PR actually uses session.rootDirectory which is puts in to maven.multiModuleProjectDirectory for things to just "work".

From what I understood from apache/maven-mvnd#1031 (comment) . Is that the value is only available in settings this needs to be used that way. In Maven 4 I believe this can be removed as it can be used as property, so we can change it in the poms and other places.

[gsmet]

Yeah, my question was about when session.rootDirectory was introduced as to know if we would need to require a more recent version of Maven to build the project.

[manofthepeace]

https://maven.apache.org/docs/3.9.2/release-notes.html

Task MNG-7774 implemented interpolation for configuration and command line, but also provides two new properties usable in configuration interpolation: session.topDirectory (reactor top directory) and session.rootDirectory (project root directory, usually where .mvn directory reside). It is recommended to create .mvn directory in project root directory, as presence of this directory is used to detect root directory location. If .mvn directory does not exists, root directory will not be detected, and in such case attempted use of expression session.rootDirectory in interpolation will make Maven refuse to start (will report error). It is important to mention, that those two new properties, are only working within the maven.config file and via the command line. They are NOT usable as configuration properties within your plugin configuration in your pom.xml file.

does that answer?

[gsmet]

@manofthepeace yes, perfect thanks.

I will require Maven 3.9.6+ to build Quarkus.

[gsmet]

I ran some more tests and something is odd.

When running a mvnd build with this PR, I end up with a ${session.rootDirectory} directory in my Quarkus directory with the following content:

${session.rootDirectory}/
└── target
    └── asciidoc
        └── generated

(and all the doc content is there).

So there's something that needs fixing in the tree.

[manofthepeace]

Interesting. Could it be something with asciidoctor itself, or the asciidoctor maven plugin? In anycase do you think this is major, is mvnd used in general internally/ to build the asciidoc?

I'll try to have a look eventually. If you have any pointers let me know.

[gsmet]

It's essential to have it working. I'm working on a patch.

[gsmet]

So the issue is that we end up with the following system property:

maven.multiModuleProjectDirectory=${session.rootDirectory}

And well, when we try to get the maven.multiModuleProjectDirectory property in the annotation processor, we get the ${session.rootDirectory} string.

https://github.com/quarkusio/quarkus/blob/main/core/processor/src/main/java/io/quarkus/annotation/processor/Constants.java#L88-L89

Unfortunately, session.rootDirectory is not a system property so we cannot get the path from the system properties anymore.

Note that the current behavior is very hackyish and we really need to do better and I probably need to revive my rewrite of the annotation processor. But ideally I would like to have a quick fix.

The only thing that makes sense would be to pass the path as an argument to the annotation processor so that it's fully resolved by Maven first (you can't pass additional system properties to the compiler plugin).

[manofthepeace]

Ok, I figured it could be something like this, where the value was taken but not expanded by something out of maven.. This is unfortunate, I do not see a quick win at first glance. Not sure how far maven 4 is, but I would assume it would solve all of this.. and probably create a bunch of other things though..

Note that the current behavior is very hackyish and we really need to do better

well.. I feel like removing a property like this in a minor version was not the best, so recourse to hacks to bring it back, as maven themselves dont have a way around this makes it somewhat necessary, to reinstate the old behaviour.. I was hoping the maven.config woult actually set an env, and not something just in maven, apparently it is not the case.

I'll try to take a look. But I am short on ideas.

[manofthepeace]

@gsmet do you think this could potentially work? https://stackoverflow.com/questions/45723834/pass-argument-to-annotation-processor. i.e passing the arg to the annotation processor.

EDIT: nevermind I see it is used externally.. Only way I see is again super hacky, but it would be to go from the CWD and recurse to the root via the existence of a pom file.

[gsmet]

Yeah that's what I had in mind.
I don't think we will have a lot of other options unfortunately.

[gsmet]

But long term, we actually need the extension processor to generate content in the module itself so that should probably be easier (we want to avoid writing things outside of the scope of the module, to make caching with Develocity possible).

I started working on that a while ago. I'll try to revive the PR and see if I can come up with something.

[gsmet]

I hope I will soon merge the new extension annotation processor and then I'll see if I can revive this one.

[gsmet]

Things look a lot better with the new extension annotation processor I just merged.

I force pushed a rebase and will merge once CI is green.

[gsmet]

Hmmm. I just had the following error:

Caused by: java.io.FileNotFoundException: /home/gsmet/git/quarkus/independent-projects/resteasy-reactive/common/processor/${maven.multiModuleProjectDirectory}/.forbiddenapis/banned-signatures-common.txt (No such file or directory)

So this will need more work.

Hmmm, actually, I'm dumb, it was for backporting to 3.13.2 so without these changes.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6e862d0.

✅ 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.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

✖ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

• Expecting actual: ["-6","-8","-9","-10","-11","-12","-13","-14"] to start with: ["-6", "-7", "-8", "-9"] - java.lang.AssertionError

java.lang.AssertionError: 

Expecting actual:
  ["-6","-8","-9","-10","-11","-12","-13","-14"]
to start with:
  ["-6", "-7", "-8", "-9"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:41)

---

⚙️ JVM Tests - JDK 17 Windows

📦 independent-projects/resteasy-reactive/server/vertx

✖ org.jboss.resteasy.reactive.server.vertx.test.sse.SseServerTestCase.shouldCallOnCloseOnServer - History

• 1 expectation failed. Response body doesn't match expectation. Expected: "true" Actual: false - java.lang.AssertionError

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.
Expected: "true"
  Actual: false

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)