Skip to content

[MNG-6797] Remember if Maven model problems were encountered in Maven 4 - #12620

Open
wilx wants to merge 2 commits into
apache:masterfrom
wilx:maven-remember-problems-4x
Open

[MNG-6797] Remember if Maven model problems were encountered in Maven 4#12620
wilx wants to merge 2 commits into
apache:masterfrom
wilx:maven-remember-problems-4x

Conversation

@wilx

@wilx wilx commented Jul 29, 2026

Copy link
Copy Markdown

This ports the Maven 3 work from #298 to Maven 4 while preserving compatibility
with rules and plugins that use the legacy MavenSession API.

Maven 4 now retains the model problems reported while discovering the reactor.
The native Session exposes the retained ProblemCollector<ModelProblem> and a
hasModelProblems() convenience method, while the legacy
MavenSession#hasModelProblems() and setModelProblems(boolean) methods bridge
to the same session state. The default native collector is backed by
SessionData, so custom Session implementations and derived sessions share a
writable collector without depending on Maven Core's DefaultSession.

Problems encountered while constructing an external parent or imported BOM are
part of the owning project's model result. Derived model-builder requests
therefore share that result's collector, preserving the collector's limits,
counts, and causes rather than copying problems afterward.

Project-collection problems are retained from the selected discovery result
before -pl and related project trimming, matching Maven 3 behavior. Problems
from a speculative multi-module collection attempt are buffered until that
attempt is selected; if Maven rejects it and falls back to the explicitly
requested POM, those unrelated problems are discarded. Later, unrelated
model-building operations remain outside the session collector's scope.

Binary compatibility is preserved by adding default methods to the native
Session and ProjectsSelector interfaces and retaining the existing project
collection entry points. The session-data helper behind the Session defaults
is package-private.

Fixes #8485.

Verification:

  • mvn -pl impl/maven-impl -am -Dtest=DefaultModelBuilderTest -Dsurefire.failIfNoSpecifiedTests=false test
    • 17 tests, 0 failures, 0 errors
  • mvn -pl impl/maven-core -am -Dtest=DefaultGraphBuilderTest,DefaultSessionTest -Dsurefire.failIfNoSpecifiedTests=false test
    • 41 tests, 0 failures, 0 errors
  • mvn -e -B -V -Prun-its clean install
    • all 90 reactor modules passed
    • 1,049 Core ITs completed, 0 failures, 0 errors, 43 skipped
    • MavenITRememberModelProblemsTest passed
    • Checkstyle, RAT, Spotless, and Japicmp passed

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
    Note that commits might be squashed by a maintainer on merge.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
    This may not always be possible but is a best-practice.
  • Run mvn verify to make sure basic checks pass.
    A more thorough check will be performed on your pull request automatically.
  • You have run the Core IT successfully.

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.

Retain reactor-discovery model problems in the Maven 4 session and expose the state through both the native and legacy session APIs. Preserve collection-strategy compatibility and ignore problems from discarded discovery attempts.

Fixes apache#8485.
@wilx
wilx force-pushed the maven-remember-problems-4x branch from 992accf to 1edef52 Compare July 29, 2026 18:15
@wilx
wilx marked this pull request as ready for review July 29, 2026 18:26

@gnodet gnodet left a comment

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.

Well-structured port of the model-problem retention feature from Maven 3 to Maven 4 with thorough test coverage, proper backward compatibility, and correct thread-safe design. Two minor observations:

  1. @since version tag: The @since 3.10.0 tag on hasModelProblems() and setModelProblems() in MavenSession.java is inconsistent with the existing @since 4.0.0 convention used elsewhere in the file. The 3.10.0 was carried over from PR #298 (which targets maven-3.10.x), but since this is the Maven 4 codebase, using @since 4.0.0 (or whatever the target Maven 4 release version is) would be more consistent.

  2. Duplicate simple class name: Two new classes named SessionModelProblems are introduced — one package-private in org.apache.maven.api (API module) and one public in org.apache.maven.internal.impl (core module). This would be the first instance in the codebase of the same simple name appearing in both api and internal.impl packages. A distinct name for the impl class (e.g., LegacySessionModelProblems or SessionModelProblemsBridge) would reduce potential confusion for future maintainers.

Neither finding is blocking — the overall design is sound: thread safety via ProblemCollector's concurrent data structures, correct enum mapping, binary compatibility via default methods, proper problem propagation through multi-module collection, and thorough test coverage across unit and integration tests.

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

Claude Code on behalf of gnodet

@wilx

wilx commented Jul 29, 2026

Copy link
Copy Markdown
Author

The @since 3.10.0 seems appropriate if the 3.x change gets accepted.

@gnodet gnodet left a comment

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.

Thanks for the quick follow-up! The rename commit cleanly resolved the duplicate class name concern — the impl class is now SessionModelProblemsBridge, and all references in MavenSession.java, DefaultSession.java, and the class file itself are updated with no stale references.

Regarding @since 3.10.0: your reasoning is sound — the tags are only on MavenSession (the 3.x legacy class), not on the Maven 4 API Session.java, so tagging with the 3.x version they first appear in is defensible.

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

Claude Code on behalf of gnodet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MNG-6797] Remember in MavenSession that there were problems detected when building Maven model

2 participants