Fail on misconfigured plugin-requested toolchains - #12616
Conversation
gnodet
left a comment
There was a problem hiding this comment.
Well-scoped behavioral fix that correctly changes misconfigured toolchain handling from silent swallowing (log + empty list) to fail-fast (IllegalStateException). The approach aligns with Maven 4 which already throws ToolchainManagerException on the equivalent path. Tests are thorough and well-designed.
Minor notes:
- The
@throws IllegalStateExceptionJavadoc was added togetToolchains()but not togetToolchainFromBuildContext(), which also calls the same privateselectToolchains()method and can now throw the same exception. While in practice this path is unlikely to trigger (models stored in build context were already successfully created), the interface contract should document it for completeness. - Maven 4 master already fails fast on misconfigured toolchains — the compat layer in
ToolchainManagerFactory.javawrapsMisconfiguredToolchainExceptioninRuntimeException, and the v4DefaultToolchainManager.createToolchain()throwsToolchainManagerException. This PR brings Maven 3.10.x into alignment with that behavior usingIllegalStateException, which is the right choice given the public API constraint.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
Propagate toolchain construction failures instead of returning an empty selection that lets plugins silently fall back to Maven’s runtime JDK. Keep ordinary nonmatching selections and missing factories unchanged. Fixes apache/maven-compiler-plugin#618
9a960f1 to
2199ea5
Compare
|
Fixed this:
|
gnodet
left a comment
There was a problem hiding this comment.
Re-review after new commit ✅
The new commit correctly adds the missing @throws IllegalStateException Javadoc to getToolchainFromBuildContext(), completing the documentation that was initially only applied to getToolchains(). Both methods delegate to the shared selectToolchains private method where the exception is thrown, so the annotation accurately reflects runtime behavior.
No remaining issues — the behavioral change, tests, and documentation are all consistent and well-scoped.
Reviewed via automated review loop · Feedback? Tag @gnodet
Fixes apache/maven-compiler-plugin#618
Maven Compiler Plugin passes its
jdkToolchainrequirements to Maven Core'spublic per-plugin
ToolchainManager#getToolchains(...)API. Maven 3 previouslycaught
MisconfiguredToolchainException, logged it, and returned an empty list.The compiler plugin consequently fell back to the JDK running Maven and could
report
BUILD SUCCESSdespite an explicitly requested malformed toolchain.This change propagates an
IllegalStateExceptionwith the toolchain type andthe original diagnostic, retaining
MisconfiguredToolchainExceptionas itscause. An unchecked exception avoids changing the established Maven 3 public
method signature; adding a checked exception would break source compatibility
for plugin callers. Valid toolchains that do not match continue to produce an
empty selection, and missing toolchain factories retain their existing
log-and-empty-list behavior.
The regression test exercises the public per-plugin lookup with a real
JavaToolchainFactory, matching requirements, and a nonexistentjdkHome. Aseparate control verifies that a valid nonmatching toolchain remains nonfatal.
Verification:
mvn -pl maven-core -Dtest=DefaultToolchainManagerTest testmvn -pl maven-core verify— 366 tests, 1 skippedmvn verify— all 16 reactor modules successfulmaven-3.10.xat70eabb0679, run under JDK 8 againstapache-maven-3.10.0-SNAPSHOT-bin.zipfrom this branch(
SHA-256 67b1c28163ecbf09417e262f3f1fc59ff8987fa076a5956b219cc0a97331a53a):mvn -B -V clean install -Prun-its -Dmaven.repo.local=/home/wilx/maven-sources/core/3.x/its-3/repo -DmavenDistro=/home/wilx/maven-sources/core/maven-issue-mcompiler-618-misconfigured-toolchain/apache-maven/target/apache-maven-3.10.0-SNAPSHOT-bin.zip— 867 tests, 0 failures, 0 errors, 40 skipped; all 78 reactor modules
successful.
and returned
BUILD SUCCESS.BUILD FAILUREwith the invalidJDK path and did not compile the source.
nonmatch and the build proceeded normally.
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
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.