Skip to content

[MNG-8112] Honor export descriptors from the extension classpath - #13101

Open
goutamadwant wants to merge 2 commits into
apache:masterfrom
goutamadwant:fix/mng-8112-extension-classpath-exports
Open

goutamadwant wants to merge 2 commits into
apache:masterfrom
goutamadwant:fix/mng-8112-extension-classpath-exports

Conversation

@goutamadwant

@goutamadwant goutamadwant commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #10506.

Discover META-INF/maven/extension.xml from maven.ext.class.path entries and include their package and artifact exports in both CLI implementations. This makes exported packages visible to plugins and filters exported artifacts from plugin dependencies.

Reuse the existing descriptor reader. Keep extension classloader isolation and duplicate-extension precedence unchanged. Cover JARs, directories, multiple entries, optional descriptors, and loading alongside .mvn/extensions.xml.

Validation with JDK 21: both new Core ITs fail against the unmodified distribution and pass with the fix. mvn -B -ntp verify and the full mvn -B -ntp -Prun-its -Dits.forkCount=2 verify passed, including package-visibility and artifact-filtering regressions for both CLI implementations.

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.

Read package and artifact exports from maven.ext.class.path entries in both CLI implementations. Reuse descriptor discovery without moving extensions into the core realm or changing extension precedence.

Cover JAR and directory entries, optional descriptors, package isolation, artifact filtering, and mixed extension loading with unit tests and Core ITs.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Solid fix. The approach correctly reads extension.xml descriptors from -Dmaven.ext.class.path entries and feeds their exported packages/artifacts into CoreExports, so ClassRealmManager can properly filter them when building plugin realms. Both CLI implementations (compat 3.x and impl 4.x) are semantically equivalent despite their structural differences. The ordering is correct — setupContainerRealm already adds the classpath URLs to the container realm before discoverFrom is called, so descriptor scanning works against the right classloader. The list mutation in PlexusContainerCapsuleFactory is safe because setupContainerRealm makes a defensive copy.

Test coverage is thorough: unit tests verify package export visibility, artifact filtering, optional descriptors, and multi-entry classpath merging. The integration tests exercise both classpath-only and classpath-plus-extensions.xml scenarios end-to-end.

One nit below.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

import static org.junit.jupiter.api.Assertions.assertTrue;

@Deprecated
class MavenCliExtensionClasspathTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Nit: @Deprecated marks this class itself as deprecated, which is not the intent — you want to suppress deprecation warnings from using the deprecated MavenCli. The correct annotation is @SuppressWarnings("deprecation").

Suggested change
class MavenCliExtensionClasspathTest {
@SuppressWarnings("deprecation")

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.

@gnodet-bot Replaced @Deprecated with @SuppressWarnings("deprecation"). The CLI tests and module verification pass. Thanks for catching this.

Use SuppressWarnings instead of marking the regression test itself deprecated. Keep the warning suppression scoped to the test class.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review after 79dfab82: the @Deprecated@SuppressWarnings("deprecation") nit from the previous review has been addressed. No new issues introduced.

This review was generated by an AI agent, Hermès 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-8112] extension.xml is ignored when extension is loaded by -Dmaven.ext.class.path

2 participants