Skip to content

Fix #2520: default maven.mainClass in Java instead of classworlds config - #12875

Merged
gnodet merged 1 commit into
masterfrom
fix-220
Aug 28, 2026
Merged

Fix #2520: default maven.mainClass in Java instead of classworlds config#12875
gnodet merged 1 commit into
masterfrom
fix-220

Conversation

@gnodet

@gnodet gnodet commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2520 — Maven Wrapper (and IDEs like IntelliJ) broken with Maven 4.0.0-beta-5+ because maven.mainClass is not set when launching through external tools.

Root cause: m2.conf used main is ${maven.mainClass} which required classworlds property resolution. When external tools bypassed the mvn scripts, -Dmaven.mainClass was never set, causing ConfigurationException: No such property: maven.mainClass. The previous fix (PR #10998, rc-5) added set maven.mainClass default in m2.conf, but this kept the defaulting in classworlds config rather than Java — as noted by @gnodet in the issue discussion.

Fix: Move the main class defaulting from classworlds property resolution to Java code:

  • m2.conf: Hardcode main is org.apache.maven.cling.MavenCling from plexus.core — no variable resolution needed, no possibility of classworlds ConfigurationException
  • MavenCling.java: Check maven.mainClass system property in the ClassWorld Launcher entry point and delegate to the specified class via reflection when set (e.g., MavenEncCling, MavenShellCling, MavenUpCling)

This makes MavenCling the Java-level default entry point that all external tools can rely on without any special configuration.

Behavior

Scenario Before After
mvn script (normal) -Dmaven.mainClass=MavenCling → classworlds resolves -Dmaven.mainClass=MavenCling → classworlds calls MavenCling → no dispatch
mvn --enc -Dmaven.mainClass=MavenEncCling → classworlds resolves -Dmaven.mainClass=MavenEncClingMavenCling delegates to MavenEncCling
Maven Wrapper (no -Dmaven.mainClass) m2.conf default resolves classworlds calls MavenCling directly — no resolution needed
IDE (no -Dmaven.mainClass) m2.conf default resolves classworlds calls MavenCling directly — no resolution needed

Test plan

  • New MavenClingTest with 3 tests: delegation to valid class, unknown class error, missing method error
  • All new tests pass
  • mvn verify -pl impl/maven-cli -DskipTests — build + format checks pass
  • Pre-existing test failures are unrelated (MimirInfuser version mismatch)

🤖 Generated with Claude Code

Move the main class defaulting from classworlds property resolution to
Java code. MavenCling is now hardcoded as the entry point in m2.conf
and delegates to the class specified by -Dmaven.mainClass via
reflection when set. This fixes the Maven Wrapper and IDE integration
by removing the dependency on classworlds resolving ${maven.mainClass}.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet added this to the 4.0.0-rc-7 milestone Aug 27, 2026
@gnodet
gnodet marked this pull request as ready for review August 27, 2026 20:20
@gnodet
gnodet requested a review from cstamas August 27, 2026 20:20

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-motivated fix that moves the maven.mainClass dispatch from classworlds property resolution to Java-level reflection in MavenCling. The approach eliminates the root cause of Maven Wrapper and IDE failures and follows the design suggested in the original issue discussion.

All four known delegation targets (MavenCling, MavenEncCling, MavenShellCling, MavenUpCling) have the expected public static int main(String[], ClassWorld) signature, and the error handling in delegateMain is thorough.

The PR description and behavior matrix are excellent. The approach of hardcoding MavenCling in m2.conf and delegating in Java is strictly more robust than the previous classworlds-level default (PR #10998), since it eliminates classworlds property resolution from the critical path entirely.

Minor observations:

  • The instanceof checks in delegateMain use old-style casts rather than pattern matching instanceof, which is used elsewhere in the codebase (Java 17 target). Minor style inconsistency.
  • Tests cover delegation scenarios well but miss the two no-delegation paths: (1) when maven.mainClass is not set (null), and (2) when it is set to MavenCling itself. While these paths are simple, explicit tests would document the contract.
  • Tests mutate the global maven.mainClass system property — the try/finally cleanup is correct, but this could cause flaky failures if tests run in parallel within the same JVM.

📋 PR Metadata

Aspect Current Suggested
Labels (none) bug

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

set maven.mainClass default org.apache.maven.cling.MavenCling

main is ${maven.mainClass} from plexus.core
main is org.apache.maven.cling.MavenCling from plexus.core

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the syntax here but is this right? Is this supposed to be a comment?

@gnodet gnodet added the bug Something isn't working label Aug 28, 2026
@gnodet gnodet modified the milestones: 4.0.0-rc-7, 4.1.0 Aug 28, 2026
@gnodet
gnodet merged commit a8be2cb into master Aug 28, 2026
24 checks passed
@gnodet
gnodet deleted the fix-220 branch August 28, 2026 13:03
gnodet added a commit that referenced this pull request Aug 28, 2026
Now that MavenCling is the hardcoded entry point (#12875) and delegates
via maven.mainClass, the IT can use "mvn --up" instead of the separate
"mvnup" script. This eliminates the chmod workaround for CI environments
where zip extraction loses Unix execute permissions.

Also use orElseGet for optional element creation (review feedback).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet added a commit that referenced this pull request Aug 28, 2026
…fig (#12875) (#12889)

Move the main class defaulting from classworlds property resolution to
Java code. MavenCling is now hardcoded as the entry point in m2.conf
and delegates to the class specified by -Dmaven.mainClass via
reflection when set. This fixes the Maven Wrapper and IDE integration
by removing the dependency on classworlds resolving ${maven.mainClass}.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gnodet added a commit that referenced this pull request Aug 29, 2026
Now that MavenCling is the hardcoded entry point (#12875) and delegates
via maven.mainClass, the IT can use "mvn --up" instead of the separate
"mvnup" script. This eliminates the chmod workaround for CI environments
where zip extraction loses Unix execute permissions.

Also use orElseGet for optional element creation (review feedback).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maven wrapper broken with maven 4.0.0-beta-5 and above (solved issue included)

4 participants