gnodet commented on PR #13101: URL: https://github.com/apache/maven/pull/13101#issuecomment-5758363352
Thanks for this contribution — this is exactly the right problem to fix. We've been discussing a broader classloader improvement plan for Maven 4.1 (tracked in #13223) and this PR is part of it. A few points from that discussion that affect how this should land: **`compat/maven-embedder` / `MavenCli` — please drop that change** `MavenCli` is deprecated and will be removed. The active CLI stack is `impl/maven-cli` / `MavenCling` (`o.a.maven.cling`). The 6-line change to `MavenCli.java` and the 159-line `MavenCliExtensionClasspathTest` should be removed — they're dead weight. The fix in `PlexusContainerCapsuleFactory` (already in this PR) is the right target. **Test should move to `impl/maven-cli`** `PlexusContainerCapsuleFactoryTest` is already there — great. But `MavenCliExtensionClasspathTest` in `compat/maven-embedder` should be dropped entirely (or the relevant test cases folded into `PlexusContainerCapsuleFactoryTest`). **Path deduplication** When multiple entries on `maven.ext.class.path` resolve to the same canonical path (symlinks, relative vs. absolute), we should deduplicate by `toRealPath()` before processing. Worth a quick check that the current implementation handles this. Once those are addressed this looks mergeable as Layer 3 of the overall plan. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
