[arturobernalg]

JAnsi stopped being a maven-core dependency when it was migrated to JLine (Migrate from JAnsi to JLine, introduce MessageBuilderFactory, #11874), but maven-core/src/main/resources/META-INF/maven/extension.xml still declares org.fusesource.jansi:jansi as an exportedArtifact and org.fusesource.jansi.* as an exportedPackage.

DefaultArtifactFilterManager builds its classpath exclusion filter from these declarations (via CoreExports), so any plugin that declares jansi as its own dependency silently has it stripped from its realm, even though maven-core does not actually provide it. This broke at least the jkube Kubernetes Maven plugin on 3.10.0.

Added ExtensionDescriptorBuilderTest#testCoreExtensionDescriptorHasNoStaleExports, which loads maven-core's own production extension.xml from the classpath and asserts none of its exports mention jansi -- fails on the current descriptor, passes once the stale entries are removed. Full maven-core suite, checkstyle and spotless pass locally.

---

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
check one of the following:

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
• I hereby declare that I have filed an Apache Individual Contributor License Agreement.

[slawekjaranowski]

It was intentionally to support backward compatibility with plugins used
https://github.com/apache/maven-shared-utils MessageBuilder

[gnodet-bot]

The fix itself is correct: org.fusesource.jansi:jansi is not a maven-core dependency on 3.10.x (removed in #11874), so keeping it in exportedArtifacts and exportedPackages actively harms plugins that declare jansi themselves — DefaultArtifactFilterManager filters it from their realm even though Maven core does not provide it. Removing the stale entry is the right fix.

Re: @slawekjaranowski's backward-compatibility concern — the export was providing negative compatibility: plugins that genuinely need jansi were having it silently stripped. Removing the export restores their ability to bring their own jansi. Any plugin relying on Maven core to inject jansi into its realm was already broken on 3.10.x (since core doesn't ship it). So removing the export is strictly a fix, not a break.

One nit on the test Javadoc:

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

[arturobernalg]

@slawekjaranowski thanks for the context. I checked how that intent holds up now:

• maven-core/maven-embedder on maven-3.10.x don't depend on maven-shared-utils at all anymore (grepped both poms, no match).
• • maven-shared-utils's own jansi dependency is declared optional, so it was never pulled in transitively just by depending on maven-shared-utils -- a plugin that wants AnsiMessageBuilder/MessageUtils has always had to declare org.fusesource.jansi:jansi itself.
    So the export's only effect today is on that plugin's own explicit jansi declaration, and DefaultArtifactFilterManager strips it assuming core provides a replacement -- which it no longer does since Migrate from JAnsi to JLine, introduce MessageBuilderFactory #11874. The export can't be helping anyone run maven-shared-utils's jansi-based formatting anymore; it can only break it, exactly as reported in jkube's case. Happy to be shown a path I'm missing, but I don't see one.

[gnodet-bot]

Following up on my earlier review — the prior nit (wrong issue reference in the test Javadoc: MNG-8724 should be #13726) is addressed in commit 1de59523. All prior findings resolved.

The fix is correct and the test is well-designed: it loads maven-core's own production extension.xml via the classloader and asserts noneMatch(a -> a.contains("jansi")) on both exported artifacts and packages. Statically verified: the pre-fix descriptor contains org.fusesource.jansi:jansi → noneMatch returns false → assertTrue fails; post-fix → passes. The test correctly gates against future stale export regressions.

Note: master's impl/maven-core/src/main/resources/META-INF/maven/extension.xml also still exports org.fusesource.jansi:jansi and org.fusesource.jansi.Ansi — worth a separate follow-up if jansi is similarly not a maven-core dependency on 4.x.

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

[slawekjaranowski]

I have added a integration test for color output

• Test for color always output maven-integration-testing#424

this test should be extended with execution:

• help:effective-pom with verbose
• some unit test

add assertions if color output is generated

[slawekjaranowski]

We have in Maven core:

• https://github.com/apache/maven/blob/maven-3.10.x/maven-jline/src/main/java/org/fusesource/jansi/Ansi.java
• https://github.com/apache/maven/blob/master/compat/maven-embedder/src/main/java/org/fusesource/jansi/Ansi.java

which replace original class from JAnsi by exporting it in extension.xml and forward call to existing already initialized JLine terminal

I don't know if we can initialize two terminal one by JAnsi and one by JLine in the same process.

[arturobernalg]

@slawekjaranowski you were right, I was missing something. I checked maven-jline's Ansi.java and confirmed it exists specifically to forward JAnsi API calls to the already-initialized JLine terminal for maven-shared-utils compatibility -- removing the exportedPackage entry would have cut that path.

I then checked how plexus-classworlds actually resolves an import: SelfFirstStrategy/ParentFirstStrategy.loadClass() call loadClassFromImport() first, and when that throws ClassNotFoundException in the imported realm, they fall back to loadClassFromSelf() -- the plugin's own classpath. So the exportedPackage: org.fusesource.jansi.* import only "wins" for Ansi (the one class maven-jline ships); any other class in that package a plugin needs, like AnsiConsole, which maven-jline doesn't provide, falls through to the plugin's own copy. That import was never the cause of #13726.

The actual bug is the exportedArtifact entry: DefaultArtifactFilterManager uses it as a dependency-resolution-time exclusion (DefaultClassRealmManager's providedArtifacts), so the real org.fusesource.jansi:jansi artifact never gets added to a plugin's resolved classpath at all -- the plugin has no copy to fall back to for AnsiConsole, regardless of the package import. That's the only entry I'm now removing.

Pushed 427555f: restored exportedPackage, kept the exportedArtifact removal, narrowed the test to match, re-ran the full suite + checkstyle + spotless. Thanks for catching this before it merged.

[slawekjaranowski]

Maybe we can also add AnsiConsole in the same way

[gnodet-bot]

Previous findings are all addressed. The new commit (427555f) correctly restores the org.fusesource.jansi.* exportedPackage — the exportedPackage mechanism is a ClassRealm import with graceful fallback (CNFE → null → falls through to the plugin's own classpath), not the classpath-stripping exportedArtifact. Removing only the exportedArtifact is the right fix.

One minor inaccuracy in the new XML comment (inline).

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

[gnodet-bot]

Previous findings are all addressed in commit c2a917187.

• ✅ XML comment inaccuracy (SelfFirstStrategy): fixed — now correctly references ClassRealm#loadClassFromImport as the fallback mechanism.
• ✅ Export narrowed from org.fusesource.jansi.* to org.fusesource.jansi.Ansi (exact class) — this is the right scoping, since maven-jline only ships Ansi.java; other org.fusesource.jansi.* classes fall back to the plugin's own classpath via ClassRealm#loadClassFromImport.
• ✅ Test updated to assert both aspects of the fix: no jansi artifact export, and the exact-class package export stays.

Pre-fix test validation: testCoreExtensionDescriptorHasNoStaleArtifactExports was confirmed failing before the fix (static trace: getExportedArtifacts() contained org.fusesource.jansi:jansi → noneMatch → false; getExportedPackages() contained org.fusesource.jansi.* wildcard → contains("org.fusesource.jansi.Ansi") → false), passing after.

LGTM.

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

[arturobernalg]

Pushed c2a9171, applying both suggestions:

• @slawekjaranowski: switched to the exact class name org.fusesource.jansi.Ansi instead of the org.fusesource.jansi.* wildcard. Checked Entry#matches() in plexus-classworlds: a non-wildcard entry (no trailing .*) matches only that exact class name, the same way other entries in this file already do (e.g. org.apache.maven.artifact). Same behavior today since Ansi is the only class maven-jline ships, but it no longer silently claims the whole package if real jansi classes with the same simple names ever show up there.
• @gnodet-bot: dropped the SelfFirstStrategy mention in the XML comment -- the import-with-fallback happens in ParentFirstStrategy too, so citing one strategy implied it was strategy-dependent.

Added a second assertion so the test also locks in that the exact-class export stays. Full suite, checkstyle and spotless pass locally.

[arturobernalg]

@slawekjaranowski checked both branches for that -- maven-jline on maven-3.10.x and compat/maven-embedder on master only ship Ansi.java, no AnsiConsole anywhere. Exporting org.fusesource.jansi.AnsiConsole wouldn't change behavior either way (the import falls back to the plugin's own classpath when the imported realm doesn't have the class), but it would advertise something core doesn't actually provide. I'd leave it out unless a real AnsiConsole forwarder gets added next to Ansi.java.

[slawekjaranowski]

@arturobernalg about add AnsiConsole I thought to add implementation and export, but can be in next time if will be needed.

After it we allow plugins to use own JAnsi implentation. If plugin try to initialize JAnsi console by own way it can override statically shared sys out, err stream ... and finally break console in whole Maven process ...

Maybe I miss in this case but it will be a plugin problem.

@gnodet what do you think here?

[slawekjaranowski]

looks ok, tested only with standard plugins like help, surefire - color output is ok

Not tested in case when plugin will call AnsiConsole.systemInstall(), AnsiConsole.systemUninstall()