You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/10/13 22:27:07 UTC
[PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
gnodet opened a new pull request, #13490:
URL: https://github.com/apache/maven/pull/13490
## Summary
Internalizes `plexus-classworlds` as `impl/maven-classworlds` and introduces a stable public API module `api/maven-api-classworlds`.
This is a prerequisite for:
- #XXXXX — load JLine on module-path for targeted `--enable-native-access` (PR B)
- #XXXXX — per-plugin JPMS `ModuleLayer` support (PR C, future)
## Changes
### New modules
- **`api/maven-api-classworlds`** — stable API interfaces (`ClassWorld`, `ClassRealm`, `Strategy`, `ClassWorldListener`, exceptions) under `org.apache.maven.api.classworlds`
- **`impl/maven-classworlds`** — fork of `plexus-classworlds` with the `org.codehaus.plexus.classworlds` implementation, backward-compat adapters under `org.codehaus.classworlds`, and new capabilities:
- `module` directive in `m2.conf` — loads JARs on the JPMS module-path in a child `ModuleLayer` (enables targeted `--enable-native-access`)
- `META-INF/maven/module-access` descriptor — classpath plugins can declare `add-exports/add-opens/add-reads` against named modules declaratively
### Across the codebase
- Replace external `plexus-classworlds` dependency with `maven-classworlds` across all modules
- Remove `classWorldsVersion` property from root `pom.xml`
- Import rewrites: `org.codehaus.plexus.classworlds.*` → same package (implementation unchanged), new API via `org.apache.maven.api.classworlds.*`
## Scope
This PR intentionally excludes:
- `mvn`, `mvn.cmd`, `m2.conf` changes (PR B — JLine on module-path)
- `createModularPluginRealm`, `<modular>true</modular>`, `ModuleLayerCreationException` (PR C — per-plugin modular realms)
## Testing
`api/maven-api-classworlds`, `impl/maven-classworlds`, and `impl/maven-core` compile and the classworlds unit tests pass.
Note: `impl/maven-cli` has pre-existing compilation issues from master changes not yet integrated into this branch; they will resolve on rebase.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4268401815
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -285,17 +259,13 @@ public int doMain(String[] args, String workingDirectory, PrintStream stdout, Pr
final Set<String> realms;
if (classWorld != null) {
realms = new HashSet<>();
- for (ClassRealm realm : classWorld.getRealms()) {
+ for (ClassRealm realm : ((org.codehaus.plexus.classworlds.ClassWorld) classWorld).getRealms()) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — `installEmbeddedTerminal()` and the dumb-terminal color detection are preserved. The PR now only changes classworlds imports and adds casts where needed.
##########
impl/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java:
##########
@@ -364,4 +365,27 @@ private static Object getId(ClassLoader classLoader) {
}
return classLoader;
}
+
+ private void applyExportOrOpen(org.codehaus.plexus.classworlds.realm.ClassRealm realm, String spec, boolean open) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Removed from this PR. The method is now in #13494 (per-plugin JPMS ModuleLayer support) where it is called by `createModularPluginRealm()`.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6214611444
two open questions:
1. do we want to preserve the jar name in the distro for backward compat? (I'm keen we do it)
2. do we want to drop the `src/test/**/*.jar` and generate them in test programmatically to avoid to have binaries in the sources?
rest LGTM
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4268404425
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassRealm.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.Closeable;
+import java.net.URL;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A class loading realm that provides isolated class loading with controlled imports and exports.
+ * <p>
+ * A ClassRealm represents an isolated class loading environment with its own classpath
+ * and controlled access to classes from other realms through imports.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface ClassRealm extends Closeable {
+
+ /**
+ * Returns the unique identifier for this realm.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ String getId();
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/Strategy.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.Enumeration;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A strategy for defining how classes and resources are located in class realms.
+ * <p>
+ * Different strategies can implement different class loading behaviors, such as
+ * parent-first, child-first, or custom delegation patterns.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface Strategy {
+
+ /**
+ * Loads a class using this strategy.
+ *
+ * @param name the fully qualified class name
+ * @return the loaded class
+ * @throws ClassNotFoundException if the class cannot be found
+ */
+ @Nonnull
+ Class<?> loadClass(@Nonnull String name) throws ClassNotFoundException;
+
+ /**
+ * Finds a resource using this strategy.
+ *
+ * @param name the resource name
+ * @return the resource URL, or null if not found
+ */
+ @Nullable
+ URL getResource(@Nonnull String name);
+
+ /**
+ * Finds all resources with the given name using this strategy.
+ *
+ * @param name the resource name
+ * @return an enumeration of resource URLs
+ * @throws IOException if an I/O error occurs
+ */
+ @Nonnull
+ Enumeration<URL> getResources(@Nonnull String name) throws IOException;
+
+ /**
+ * Returns the class realm that this strategy operates on.
+ *
+ * @return the associated class realm
+ */
+ @Nonnull
+ ClassRealm getRealm();
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassWorldException.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * Base exception for class world related errors.
+ * <p>
+ * This is the root exception type for all class world operations.
+ * Specific error conditions are represented by subclasses.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class ClassWorldException extends Exception {
+
+ /**
+ * The class world associated with this exception.
+ */
+ private final ClassWorld world;
+
+ /**
+ * Constructs a new ClassWorldException.
+ *
+ * @param world the class world associated with this exception
+ */
+ public ClassWorldException(@Nonnull ClassWorld world) {
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified detail message.
+ *
+ * @param world the class world associated with this exception
+ * @param message the detail message
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable String message) {
+ super(message);
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified detail message and cause.
+ *
+ * @param world the class world associated with this exception
+ * @param message the detail message
+ * @param cause the cause
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable String message, @Nullable Throwable cause) {
+ super(message, cause);
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified cause.
+ *
+ * @param world the class world associated with this exception
+ * @param cause the cause
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable Throwable cause) {
+ super(cause);
+ this.world = world;
+ }
+
+ /**
+ * Returns the class world associated with this exception.
+ *
+ * @return the class world
+ */
+ @Nonnull
+ public ClassWorld getWorld() {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/NoSuchRealmException.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Exception thrown when attempting to retrieve a class realm with an identifier
+ * that does not exist in the class world.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class NoSuchRealmException extends ClassWorldException {
+
+ /**
+ * The missing realm identifier.
+ */
+ private final String id;
+
+ /**
+ * Constructs a new NoSuchRealmException.
+ *
+ * @param world the class world
+ * @param id the missing realm identifier
+ */
+ public NoSuchRealmException(@Nonnull ClassWorld world, @Nonnull String id) {
+ super(world, "No such realm: " + id);
+ this.id = id;
+ }
+
+ /**
+ * Returns the missing realm identifier.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ public String getId() {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/DuplicateRealmException.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Exception thrown when attempting to create a class realm with an identifier
+ * that already exists in the class world.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class DuplicateRealmException extends ClassWorldException {
+
+ /**
+ * The duplicate realm identifier.
+ */
+ private final String id;
+
+ /**
+ * Constructs a new DuplicateRealmException.
+ *
+ * @param world the class world
+ * @param id the duplicate realm identifier
+ */
+ public DuplicateRealmException(@Nonnull ClassWorld world, @Nonnull String id) {
+ super(world, "Duplicate realm: " + id);
+ this.id = id;
+ }
+
+ /**
+ * Returns the duplicate realm identifier.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ public String getId() {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6214844740
Thanks @rmannibucau for the final pair of questions.
**1. Preserve the jar name in the distro for backward compat?**
Yes, strongly agreed. The assembled distribution currently ships `boot/maven-classworlds-<version>.jar` (and `mvn`/`mvn.cmd` glob on that name). Any custom wrapper script, CI provisioning script, or tooling that greps or globs `plexus-classworlds-*.jar` in `boot/` will silently break.
The assembly plugin supports `<outputFileNameMapping>` on a `<dependencySet>` to rename the artifact in the assembled directory independently of its Maven coordinates. We can keep the file named `plexus-classworlds-<version>.jar` in `boot/` without touching the groupId/artifactId:
```xml
<dependencySet>
<useProjectArtifact>false</useProjectArtifact>
<outputDirectory>boot</outputDirectory>
<includes>
<include>org.apache.maven:maven-classworlds</include>
</includes>
<outputFileNameMapping>plexus-classworlds-${artifact.version}.jar</outputFileNameMapping>
</dependencySet>
```
`mvn`/`mvn.cmd` then keep their existing `plexus-classworlds-*.jar` glob and need no change either. Will apply this before merging.
**2. Drop the `src/test/test-data/` binary jars and generate them programmatically?**
The jars (`a.jar`, `b.jar`, `component0-1.0.jar`, `deadlock.jar`, …) are compiled class fixtures that have been binary files in plexus-classworlds since its initial commit — they contain simple stub classes (`a.A`, `a.Aa`, `deadlock.A`/`B`/`C`/`D`, etc.) that the classloading tests load across realm boundaries.
Generating them at test time would require either: (a) a separate Maven sub-module that compiles them and produces JARs before the main module's tests run, or (b) an in-test bytecode generation approach (e.g. ASM). Both are significantly more complex than static fixtures and would themselves need sources tracked in git — just Java source files instead of class files.
My preference is to leave them as binary test fixtures in this PR (it faithfully copies the plexus-classworlds approach) and track the replacement as a follow-up if the project wants it. Keeping the scope of this internalization PR minimal makes review easier.
Does that work for you, or would you prefer we tackle the jar generation in this PR?
_Claude Code on behalf of Guillaume Nodet_
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6194281070
@rmannibucau Thanks for the feedback. The API is marked `@Experimental` precisely to allow refinement based on real usage — so this is the right time to raise concerns.
A few points on the current design choices:
- **`ClassWorld` and `ClassRealm`** mirror the plexus-classworlds concepts that the Maven container already uses internally. Exposing them under the `@Experimental` API makes the existing Maven extension/plugin contract explicit rather than hiding it behind a raw `ClassLoader`. Plugins and extensions already deal with realms (e.g. `DefaultClassRealmManager`).
- **`Strategy`** — fair point. It's a lower-level concern and could arguably be omitted from the initial API surface. If there's no concrete use case for it in the `maven-api` layer yet, we can defer it.
- **Simpler `ClassLoader`-centric API** — the concern is that a plain `ClassLoader` view loses the isolation semantics (realm-to-realm imports, exports) that Maven's classloading model relies on. Exposing those semantics explicitly, even if under `@Experimental`, lets us document and stabilize the contract rather than letting plugins rely on implementation internals.
That said, if you have a concrete proposal for a slimmer API surface (e.g. just expose the import/export operations and drop `Strategy` from the public API), I'm open to discussion. The `@Experimental` annotation means we can iterate. Could you sketch what you have in mind?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4284082696
##########
apache-maven/src/assembly/component.xml:
##########
@@ -23,14 +23,16 @@ under the License.
<useProjectArtifact>false</useProjectArtifact>
<outputDirectory>boot</outputDirectory>
<includes>
- <include>org.codehaus.plexus:plexus-classworlds</include>
+ <include>org.apache.maven:maven-classworlds</include>
+ <include>org.apache.maven:maven-api-classworlds</include>
</includes>
</dependencySet>
<dependencySet>
<useProjectArtifact>false</useProjectArtifact>
<outputDirectory>lib</outputDirectory>
<excludes>
- <exclude>org.codehaus.plexus:plexus-classworlds</exclude>
+ <exclude>org.apache.maven:maven-classworlds</exclude>
+ <exclude>org.apache.maven:maven-api-classworlds</exclude>
Review Comment:
⚠️ **Boot launcher glob mismatch — Maven won't start**
`component.xml` places `maven-classworlds-<version>.jar` and `maven-api-classworlds-<version>.jar` in `boot/`, but `mvn` still uses `echo "$MAVEN_HOME"/boot/plexus-classworlds-*.jar` (unchanged from master) and `mvn.cmd` uses `for %%i in ("%MAVEN_HOME%"\boot\plexus-classworlds-*) do set LAUNCHER_JAR=...`. Neither glob matches any file in the assembled distribution — Maven cannot start at all.
Both launchers must be updated to match the new artifact names:
`mvn`:
```
LAUNCHER_JAR=`echo "$MAVEN_HOME"/boot/maven-classworlds-*.jar`:`echo "$MAVEN_HOME"/boot/maven-api-classworlds-*.jar`
```
`mvn.cmd`:
```
for %%i in ("%MAVEN_HOME%"oot\maven-classworlds-*) do set LAUNCHER_JAR="%%i"
for %%i in ("%MAVEN_HOME%"oot\maven-api-classworlds-*) do set LAUNCHER_JAR=%LAUNCHER_JAR%;"%%i"
```
(And the test scripts `test-mvn-fast-version.sh` and `test-mvn-path-conversion.sh` need matching stubs.)
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/MavenCling.java:
##########
@@ -64,18 +64,19 @@ public static void main(String[] args) throws IOException {
* This makes {@code MavenCling} the default entry point that external tools (such as the Maven Wrapper
* or IDEs) can rely on without needing to set the {@code maven.mainClass} property.
*/
- public static int main(String[] args, ClassWorld world) throws IOException {
+ public static int main(String[] args, org.codehaus.plexus.classworlds.ClassWorld world) throws IOException {
String mainClass = System.getProperty(MAVEN_MAIN_CLASS_PROPERTY);
if (mainClass != null && !MavenCling.class.getName().equals(mainClass)) {
return delegateMain(mainClass, args, world);
}
return new MavenCling(world).run(args, null, null, null, false);
}
- private static int delegateMain(String mainClass, String[] args, ClassWorld world) throws IOException {
+ private static int delegateMain(String mainClass, String[] args, org.codehaus.plexus.classworlds.ClassWorld world)
+ throws IOException {
try {
Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass(mainClass);
- Method method = clazz.getMethod("main", String[].class, ClassWorld.class);
+ Method method = clazz.getMethod("main", String[].class, org.codehaus.plexus.classworlds.ClassWorld.class);
Review Comment:
⚠️ **`delegateMain()` also reverted — `mvnval` broken here too**
Same issue as `Launcher.getEnhancedMainMethod()`: the single-type lookup `clazz.getMethod("main", String[].class, org.codehaus.plexus.classworlds.ClassWorld.class)` throws `NoSuchMethodException` for `MavenValCling` (its `main` takes `org.apache.maven.api.classworlds.ClassWorld`). The dual-candidate loop that was approved in the last review must be restored here as well:
```suggestion
String[] cwClassNames = {
org.codehaus.plexus.classworlds.ClassWorld.class.getName(),
org.apache.maven.api.classworlds.ClassWorld.class.getName()
};
Method method = null;
for (String cwClassName : cwClassNames) {
try {
Class<?> cwClass = Thread.currentThread().getContextClassLoader().loadClass(cwClassName);
Method m = clazz.getMethod("main", String[].class, cwClass);
if (Modifier.isStatic(m.getModifiers()) && Modifier.isPublic(m.getModifiers())) {
method = m;
break;
}
} catch (NoSuchMethodException ignored) {
// try next candidate
}
}
if (method == null) {
throw new NoSuchMethodException(mainClass + ".main(String[], ClassWorld)");
}
```
##########
impl/maven-classworlds/src/main/java/org/codehaus/plexus/classworlds/launcher/Launcher.java:
##########
@@ -0,0 +1,365 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.plexus.classworlds.launcher;
+
+/*
+ * Copyright 2001-2006 Codehaus Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+import org.codehaus.plexus.classworlds.ClassWorld;
+import org.codehaus.plexus.classworlds.realm.ClassRealm;
+import org.codehaus.plexus.classworlds.realm.DuplicateRealmException;
+import org.codehaus.plexus.classworlds.realm.NoSuchRealmException;
+
+/**
+ * <p>Command-line invokable application launcher.</p>
+ *
+ * <p>This launcher class assists in the creation of classloaders and <code>ClassRealm</code>s
+ * from a configuration file and the launching of the application's <code>main</code>
+ * method from the correct class loaded through the correct classloader.</p>
+ *
+ * <p> The path to the configuration file is specified using the <code>classworlds.conf</code>
+ * system property, typically specified using the <code>-D</code> switch to
+ * <code>java</code>.</p>
+ *
+ * @author <a href="mailto:bob@eng.werken.com">bob mcwhirter</a>
+ */
+public class Launcher {
+ protected static final String CLASSWORLDS_CONF = "classworlds.conf";
+
+ protected static final String UBERJAR_CONF_DIR = "WORLDS-INF/conf/";
+
+ protected ClassLoader systemClassLoader;
+
+ protected String mainClassName;
+
+ protected String mainRealmName;
+
+ protected ClassWorld world;
+
+ private int exitCode = 0;
+
+ public Launcher() {
+ this.systemClassLoader = Thread.currentThread().getContextClassLoader();
+ }
+
+ public void setSystemClassLoader(ClassLoader loader) {
+ this.systemClassLoader = loader;
+ }
+
+ public ClassLoader getSystemClassLoader() {
+ return this.systemClassLoader;
+ }
+
+ public int getExitCode() {
+ return exitCode;
+ }
+
+ public void setAppMain(String mainClassName, String mainRealmName) {
+ this.mainClassName = mainClassName;
+ this.mainRealmName = mainRealmName;
+ }
+
+ public String getMainRealmName() {
+ return this.mainRealmName;
+ }
+
+ public String getMainClassName() {
+ return this.mainClassName;
+ }
+
+ public void setWorld(ClassWorld world) {
+ this.world = world;
+ }
+
+ public ClassWorld getWorld() {
+ return this.world;
+ }
+
+ /**
+ * Configure from a file.
+ *
+ * @param is The config input stream.
+ * @throws IOException If an error occurs reading the config file.
+ * @throws MalformedURLException If the config file contains invalid URLs.
+ * @throws ConfigurationException If the config file is corrupt.
+ * @throws org.codehaus.plexus.classworlds.realm.DuplicateRealmException If the config file defines two realms
+ * with the same id.
+ * @throws org.codehaus.plexus.classworlds.realm.NoSuchRealmException If the config file defines a main entry
+ * point in a non-existent realm.
+ */
+ public void configure(InputStream is)
+ throws IOException, ConfigurationException, DuplicateRealmException, NoSuchRealmException {
+ Configurator configurator = new Configurator(this);
+ configurator.configure(is);
+ }
+
+ /**
+ * Retrieve the main entry class.
+ *
+ * @return The main entry class.
+ * @throws ClassNotFoundException If the class cannot be found.
+ * @throws NoSuchRealmException If the specified main entry realm does not exist.
+ */
+ public Class<?> getMainClass() throws ClassNotFoundException, NoSuchRealmException {
+ return getMainRealm().loadClass(getMainClassName());
+ }
+
+ /**
+ * Retrieve the main entry realm.
+ *
+ * @return The main entry realm.
+ * @throws NoSuchRealmException If the specified main entry realm does not exist.
+ */
+ public ClassRealm getMainRealm() throws NoSuchRealmException {
+ return getWorld().getRealm(getMainRealmName());
+ }
+
+ /**
+ * Retrieve the enhanced main entry method.
+ *
+ * @return The enhanced main entry method.
+ * @throws ClassNotFoundException If the main entry class cannot be found.
+ * @throws NoSuchMethodException If the main entry method cannot be found.
+ * @throws NoSuchRealmException If the main entry realm cannot be found.
+ */
+ protected Method getEnhancedMainMethod()
+ throws ClassNotFoundException, NoSuchMethodException, NoSuchRealmException {
+ Class<?> cwClass = getMainRealm().loadClass(ClassWorld.class.getName());
Review Comment:
⚠️ **`MavenValCling` broken — single-type lookup misses the API `ClassWorld`**
`getEnhancedMainMethod()` now only loads `org.codehaus.plexus.classworlds.ClassWorld` (the impl type). `MavenValCling.main(String[], ClassWorld)` declares `org.apache.maven.api.classworlds.ClassWorld` as its second parameter — `getMainClass().getMethod("main", String[].class, cwClass)` will throw `NoSuchMethodException` whenever the configured main class is `MavenValCling`.
The dual-candidate loop from the previous commit was the correct fix. Restore it:
```suggestion
// Try both the impl ClassWorld and the stable API ClassWorld interface.
// Entry points using impl ClassWorld (MavenCling, MavenEncCling, MavenShellCling, MavenUpCling)
// are found by the first candidate; MavenValCling (which uses the API type) by the second.
String[] cwClassNames = {ClassWorld.class.getName(), "org.apache.maven.api.classworlds.ClassWorld"};
for (String cwClassName : cwClassNames) {
try {
Class<?> cwClass = getMainRealm().loadClass(cwClassName);
Method m = getMainClass().getMethod("main", String[].class, cwClass);
int modifiers = m.getModifiers();
if (Modifier.isStatic(modifiers) && Modifier.isPublic(modifiers)) {
if (m.getReturnType() == Integer.TYPE || m.getReturnType() == Void.TYPE) {
return m;
}
}
} catch (NoSuchMethodException | ClassNotFoundException ignored) {
// try next candidate
}
}
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6212293823
This PR is tracked as **Layer 0** in #13223 (overall plan for classloader improvements in Maven 4.1).
_Claude Code on behalf of Guillaume Nodet_
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4292739723
##########
pom.xml:
##########
@@ -142,9 +142,8 @@ under the License.
<maven.site.path>ref/4-LATEST</maven.site.path>
<project.build.outputTimestamp>2025-06-24T20:18:00Z</project.build.outputTimestamp>
<!-- various versions -->
- <asmVersion>9.11</asmVersion>
+ <asmVersion>9.10.1</asmVersion>
Review Comment:
⚠️ **Version downgrades introduced by rebase divergence** — the PR branch is behind `master` and introduces three unintended version regressions:
- `asmVersion`: 9.10.1 (PR) vs 9.11 (master)
- `jlineVersion` (line 155): 4.4.7 (PR) vs 4.4.8 (master)
- `stax2ApiVersion` (line 169): 4.3.1 (PR) vs 4.3.2 (master)
Please rebase on current `master` before merging.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProject.java:
##########
@@ -58,7 +58,8 @@ public DefaultProject(InternalMavenSession session, MavenProject project) {
this.project = project;
ClassLoader ttcl = Thread.currentThread().getContextClassLoader();
try {
- Thread.currentThread().setContextClassLoader(project.getClassRealm());
+ Thread.currentThread()
+ .setContextClassLoader(project.getClassRealm() != null ? project.getClassRealm() : null);
Review Comment:
💡 **No-op ternary:** Same pattern as `DefaultMaven.java` — `x != null ? x : null` simplifies to `x`. `setContextClassLoader(null)` is actually valid (resets to bootstrap classloader) and was the pre-existing behavior when `getClassRealm()` returned null, so the ternary doesn't change anything.
```suggestion
Thread.currentThread().setContextClassLoader(project.getClassRealm());
```
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -467,7 +467,7 @@ protected <T> Collection<T> getProjectScopedExtensionComponents(Collection<Maven
ClassLoader originalContextClassLoader = currentThread.getContextClassLoader();
try {
for (MavenProject project : projects) {
- ClassLoader projectRealm = project.getClassRealm();
+ ClassLoader projectRealm = project.getClassRealm() != null ? project.getClassRealm() : null;
Review Comment:
💡 **No-op ternary:** `x != null ? x : null` is always equivalent to `x`. This is a style regression vs the original `ClassLoader projectRealm = project.getClassRealm();` — the null check before `scannedRealms.add(projectRealm)` on the next line already guards correctly.
```suggestion
ClassLoader projectRealm = project.getClassRealm();
```
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -484,15 +484,15 @@ fi
if [ -n "$MAVEN_DEBUG_SCRIPT" ]; then
printf '[DEBUG] Final MAVEN_OPTS: %s\n' "$MAVEN_OPTS" >&2
fi
-LAUNCHER_JAR=`echo "$MAVEN_HOME"/boot/plexus-classworlds-*.jar`
+LAUNCHER_JAR=`echo "$MAVEN_HOME"/boot/maven-classworlds-*.jar`
LAUNCHER_CLASS=org.codehaus.plexus.classworlds.launcher.Launcher
JLINE_NATIVE_PATH="$MAVEN_HOME/lib/jline-native"
# For Cygwin and MinGW, switch paths to Windows format before running java(1) command
if $cygwin || $mingw ; then
[ -n "$JAVA_HOME" ] &&
JAVA_HOME=`cygpath --windows "$JAVA_HOME"`
- LAUNCHER_JAR=`cygpath --windows "$LAUNCHER_JAR"`
+ LAUNCHER_JAR=`cygpath --path --windows "$LAUNCHER_JAR"`
Review Comment:
💡 **Minor:** `cygpath --path` is designed for colon-separated `PATH` strings (converts each component). `LAUNCHER_JAR` is a single file path with no colons, so `--path` is a no-op here. The other entries in this block (`JAVA_HOME`, `CLASSWORLDS_CONF`, `MAVEN_HOME`, `JLINE_NATIVE_PATH`) all use plain `--windows`. Consider aligning:
```suggestion
LAUNCHER_JAR=`cygpath --windows "$LAUNCHER_JAR"`
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4284334212
##########
apache-maven/src/test/scripts/test-mvn-path-conversion.sh:
##########
@@ -65,7 +65,7 @@ project_dir="$work_dir/project"
mkdir -p "$stub_dir" "$maven_home/bin" "$maven_home/boot" "$project_dir/.mvn" "$project_dir/module"
cp "$mvn_script" "$maven_home/bin/mvn"
-touch "$maven_home/bin/m2.conf" "$maven_home/boot/plexus-classworlds-9.9.9.jar"
+touch "$maven_home/bin/m2.conf" "$maven_home/boot/maven-classworlds-9.9.9.jar"
Review Comment:
⚠️ **Missing `maven-api-classworlds-9.9.9.jar` — compound `LAUNCHER_JAR` path conversion never exercised**
`test-mvn-fast-version.sh` (updated in the same commit) correctly adds both boot jars. This script only adds one. The `mvn` script builds `LAUNCHER_JAR=jar1:jar2` — without the second jar, the glob `echo .../maven-api-classworlds-*.jar` is unexpanded, so `LAUNCHER_JAR` contains a literal `*` as the second component. The Cygwin `cygpath --path --windows` conversion of a compound two-jar path is never tested.
```suggestion
touch "$maven_home/bin/m2.conf" "$maven_home/boot/maven-classworlds-9.9.9.jar" "$maven_home/boot/maven-api-classworlds-9.9.9.jar"
```
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/CliRequest.java:
##########
@@ -23,9 +23,9 @@
import java.util.Properties;
import org.apache.commons.cli.CommandLine;
+import org.apache.maven.api.classworlds.ClassWorld;
Review Comment:
⚠️ **Binary-incompatible return type change on `getClassWorld()` — breaks existing embedders**
This import swap changes the return type of the public `getClassWorld()` method from `org.codehaus.plexus.classworlds.ClassWorld` to `org.apache.maven.api.classworlds.ClassWorld`. Return types are part of the JVM method descriptor — any Maven embedder compiled against `maven-embedder` 4.0.x that calls `CliRequest.getClassWorld()` will get `NoSuchMethodError` at runtime (source-compatible because the impl implements the interface, but not binary-compatible).
The compat module exists to keep existing embedders working without surprises. Keep the return type as `org.codehaus.plexus.classworlds.ClassWorld` — callers that want the API type can widen it themselves, since the impl implements the interface.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6194227142
Thanks for the review @rmannibucau.
The goal here is to internalize plexus-classworlds as an Apache Maven artifact and expose a **minimal, stable API** for the concepts that plugin authors genuinely need today (mostly realm introspection). The API surface is already quite small — `ClassWorld`, `ClassRealm`, and `Strategy` are the three things plugins interact with.
That said, I agree the longer-term direction is worth discussing. The `Strategy` abstraction in particular could arguably be flattened. I'd propose keeping this PR focused on the internalization (replacing the external dependency) and tracking API refinement in a follow-up issue where the design can be discussed with more context. Would that work for you?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4317496163
##########
impl/maven-classworlds/src/test/java/org/codehaus/plexus/classworlds/TestJarGenerator.java:
##########
@@ -0,0 +1,554 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.plexus.classworlds;
+
+import javax.tools.DiagnosticCollector;
+import javax.tools.FileObject;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+import javax.tools.ToolProvider;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+/**
+ * Generates test fixture JARs at test time using the JDK's {@code javac} and {@code jar} tools,
+ * eliminating the need to commit pre-compiled binary JARs to version control.
+ *
+ * <p>All JARs are written to {@code target/test-jars/} under the module base directory.
+ * The generator is idempotent: if the directory already exists and is non-empty it is wiped
+ * and rebuilt to ensure freshness.
Review Comment:
💡 **Misleading Javadoc: “wiped and rebuilt” doesn’t match the implementation.**
The constructor calls `Files.createDirectories(outputDir)` (a no-op if the directory exists), and `jar()` calls `Files.newOutputStream(out)` which truncates-and-overwrites each file individually. No directory-level deletion happens — leftover JARs from a previous run whose names are no longer generated (e.g. if a JAR name changes) would survive silently. The description should match the actual behaviour:
```suggestion
* <p>All JARs are written to {@code target/test-jars/} under the module base directory.
* The generator is idempotent: existing JARs are overwritten on each run. Note that
* JARs from a previous run whose names are no longer generated are not deleted.
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4264830310
##########
impl/maven-core/src/main/resources/META-INF/maven/extension.xml:
##########
@@ -154,7 +151,7 @@ under the License.
<exportedArtifact>org.apache.maven:maven-api-xml</exportedArtifact>
<exportedArtifact>classworlds:classworlds</exportedArtifact>
- <exportedArtifact>org.codehaus.plexus:plexus-classworlds</exportedArtifact>
+ <exportedArtifact>org.apache.maven:maven-classworlds</exportedArtifact>
Review Comment:
⚠️ **`maven-api-classworlds` is missing from `<exportedArtifacts>`**
Every other `maven-api-*` artifact is listed (lines 142–151), but `maven-api-classworlds` has no entry. Without it, a plugin that declares a `maven-api-classworlds` dependency gets its own copy loaded into the plugin realm instead of being filtered to use the container's. The result is a `ClassCastException` the moment the plugin receives a `ClassRealm` or `ClassWorld` instance from Maven core — the plugin's class and Maven's class are different `Class<?>` objects from different loaders.
```suggestion
<exportedArtifact>org.apache.maven:maven-classworlds</exportedArtifact>
<exportedArtifact>org.apache.maven:maven-api-classworlds</exportedArtifact>
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -150,22 +147,6 @@ public final int invoke(InvokerRequest invokerRequest) {
} finally {
Thread.currentThread().setContextClassLoader(oldCL);
System.setProperties(oldProps);
Review Comment:
⚠️ **JUL handler leak when Maven runs embedded**
`restoreRootLoggerHandlers()` was removed from the `finally` block, but `SLF4JBridgeHandler.install()` is still called in `activateLogging()`. The deleted method's own Javadoc stated the exact problem:
> When Maven runs inside another JVM, the root logger belongs to that JVM, and a `SLF4JBridgeHandler` left on it would route the host's logging to Maven and fail once Maven's class realm is closed.
After Maven exits, the bridge handler remains on the JVM's root `java.util.logging` logger. The next JUL event in the host process routes through the closed Maven classloader, causing `NoClassDefFoundError: org/slf4j/Logger` in the host. This regresses any in-process Maven invocation (IDEs, CI tooling, the Verifier).
Fix: restore the handler cleanup in `finally`:
```suggestion
Thread.currentThread().setContextClassLoader(oldCL);
System.setProperties(oldProps);
SLF4JBridgeHandler.uninstall();
```
##########
impl/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java:
##########
@@ -364,4 +365,27 @@ private static Object getId(ClassLoader classLoader) {
}
return classLoader;
}
+
+ private void applyExportOrOpen(org.codehaus.plexus.classworlds.realm.ClassRealm realm, String spec, boolean open) {
Review Comment:
💡 **`applyExportOrOpen()` is dead code in this PR**
This method is added but never called from within this PR. Based on the PR description, it will be invoked by `applyModuleAccessDescriptors()` in PR #13491 (JLine on module-path). Shipping dead code here means CI cannot test this code path, and any bugs in the parsing logic (e.g. the `=`-target handling, the `ALL-UNNAMED` check) won't surface until #13491.
Consider either moving this method to PR #13491, or calling it in this PR so it gets test coverage.
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -285,17 +259,13 @@ public int doMain(String[] args, String workingDirectory, PrintStream stdout, Pr
final Set<String> realms;
if (classWorld != null) {
realms = new HashSet<>();
- for (ClassRealm realm : classWorld.getRealms()) {
+ for (ClassRealm realm : ((org.codehaus.plexus.classworlds.ClassWorld) classWorld).getRealms()) {
Review Comment:
⚠️ **Behavioral change in the compat layer: embedded terminal and color detection removed**
`installEmbeddedTerminal()` and its call in `doMain(args, workingDirectory, stdout, stderr)` were removed, along with the dumb-terminal color detection (`else if (MessageUtils.getTerminal().getType().startsWith("dumb")) { MessageUtils.setColorEnabled(false); }`).
`installEmbeddedTerminal()` was added specifically for #13357: without it, embedders calling `doMain()` with `--color=yes` get no color output because no terminal is installed. Removing it silently regresses that fix for all Maven-Verifier and IDE-tooling callers.
The dumb-terminal detection removal means terminals that do not support ANSI escapes will now receive escape codes when `--color` is not explicitly set, producing garbled output.
Both are behavioral changes in a `@Deprecated(since="4.0.0")` compat module whose contract is to keep existing Maven 3 embedders working without surprises. Per project convention, behavior changes in `compat/` require an equivalent port to Maven 3.x. If the removal is intentional (e.g., this is superseded by a change in `impl/maven-cli`), please document it in the PR description and confirm the 3.x disposition.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6215083309
for the jar asm or subproject "fixture" would work I think, i dont care much while we reach a stage we dont trust binaries like that (and not blaming know it was there)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6193207702
@gnodet didnt yet fully defined it but feel like it could just be exposing a MavenClassLoader or alike and not split that much concepts. integration can then be with other custom classloaders since it is what the abstractions are about most of the time and classloader are better defined than legacy classworld concept for java dev but it might need jpms careness with the work you do in parallel. So (maven)classloader+modulelayer sounds like the way to move forward for me but it is just a first feedback from my past interactions with plexus-classworld and OSGi/EE.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6195452299
+1 to not surface it as an API cause right now it is a great way to break extension/plugins (runtime behavior is not well intended in concurrent "embed" usages. Think something more contract centric and maybe enabling to create an auto closeable classloader (literally) would be saner for plugins - this is current pattern in practise instead of classrealm. ClassRealm is mainly useful to hack maven and this can always be done statically with dynamism done with this new classloader (mven resolver friendly) api. So let's postpone it for the API part.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4268128796
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/Strategy.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.Enumeration;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A strategy for defining how classes and resources are located in class realms.
+ * <p>
+ * Different strategies can implement different class loading behaviors, such as
+ * parent-first, child-first, or custom delegation patterns.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface Strategy {
+
+ /**
+ * Loads a class using this strategy.
+ *
+ * @param name the fully qualified class name
+ * @return the loaded class
+ * @throws ClassNotFoundException if the class cannot be found
+ */
+ @Nonnull
+ Class<?> loadClass(@Nonnull String name) throws ClassNotFoundException;
+
+ /**
+ * Finds a resource using this strategy.
+ *
+ * @param name the resource name
+ * @return the resource URL, or null if not found
+ */
+ @Nullable
+ URL getResource(@Nonnull String name);
+
+ /**
+ * Finds all resources with the given name using this strategy.
+ *
+ * @param name the resource name
+ * @return an enumeration of resource URLs
+ * @throws IOException if an I/O error occurs
+ */
+ @Nonnull
+ Enumeration<URL> getResources(@Nonnull String name) throws IOException;
+
+ /**
+ * Returns the class realm that this strategy operates on.
+ *
+ * @return the associated class realm
+ */
+ @Nonnull
+ ClassRealm getRealm();
Review Comment:
⚠️ **Noun-style accessor convention:** `getRealm()` → `realm()` on this `@Experimental` interface.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassRealm.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.Closeable;
+import java.net.URL;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A class loading realm that provides isolated class loading with controlled imports and exports.
+ * <p>
+ * A ClassRealm represents an isolated class loading environment with its own classpath
+ * and controlled access to classes from other realms through imports.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface ClassRealm extends Closeable {
+
+ /**
+ * Returns the unique identifier for this realm.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ String getId();
Review Comment:
⚠️ **Noun-style accessor convention:** Maven 4 API `@Experimental` interfaces must use noun-style no-arg accessors. This method and five others in this interface should be renamed:
- `getId()` → `id()`
- `getWorld()` → `world()`
- `getClassLoader()` → `classLoader()`
- `getStrategy()` → `strategy()`
- `getURLs()` → `urls()`
- `getParentClassLoader()` → `parentClassLoader()`
See existing API interfaces (`Session`, `Project`, `Artifact`, etc.) for established precedent.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassWorldException.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * Base exception for class world related errors.
+ * <p>
+ * This is the root exception type for all class world operations.
+ * Specific error conditions are represented by subclasses.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class ClassWorldException extends Exception {
+
+ /**
+ * The class world associated with this exception.
+ */
+ private final ClassWorld world;
+
+ /**
+ * Constructs a new ClassWorldException.
+ *
+ * @param world the class world associated with this exception
+ */
+ public ClassWorldException(@Nonnull ClassWorld world) {
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified detail message.
+ *
+ * @param world the class world associated with this exception
+ * @param message the detail message
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable String message) {
+ super(message);
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified detail message and cause.
+ *
+ * @param world the class world associated with this exception
+ * @param message the detail message
+ * @param cause the cause
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable String message, @Nullable Throwable cause) {
+ super(message, cause);
+ this.world = world;
+ }
+
+ /**
+ * Constructs a new ClassWorldException with the specified cause.
+ *
+ * @param world the class world associated with this exception
+ * @param cause the cause
+ */
+ public ClassWorldException(@Nonnull ClassWorld world, @Nullable Throwable cause) {
+ super(cause);
+ this.world = world;
+ }
+
+ /**
+ * Returns the class world associated with this exception.
+ *
+ * @return the class world
+ */
+ @Nonnull
+ public ClassWorld getWorld() {
Review Comment:
⚠️ **Noun-style accessor convention:** This exception is part of the public `maven-api-classworlds` contract. Consider renaming `getWorld()` → `world()` for consistency with the API interfaces.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/NoSuchRealmException.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Exception thrown when attempting to retrieve a class realm with an identifier
+ * that does not exist in the class world.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class NoSuchRealmException extends ClassWorldException {
+
+ /**
+ * The missing realm identifier.
+ */
+ private final String id;
+
+ /**
+ * Constructs a new NoSuchRealmException.
+ *
+ * @param world the class world
+ * @param id the missing realm identifier
+ */
+ public NoSuchRealmException(@Nonnull ClassWorld world, @Nonnull String id) {
+ super(world, "No such realm: " + id);
+ this.id = id;
+ }
+
+ /**
+ * Returns the missing realm identifier.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ public String getId() {
Review Comment:
⚠️ **Noun-style accessor convention:** `getId()` → `id()` for consistency with the API.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassWorld.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.Closeable;
+import java.util.function.Predicate;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A collection of {@link ClassRealm}s, indexed by id.
+ * <p>
+ * A ClassWorld provides a container for managing multiple class loading realms,
+ * each with their own classpath and import/export relationships.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface ClassWorld extends Closeable {
+
+ /**
+ * Creates a new class realm with the specified id.
+ *
+ * @param id the unique identifier for the realm
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id) throws DuplicateRealmException;
+
+ /**
+ * Creates a new class realm with the specified id and base class loader.
+ *
+ * @param id the unique identifier for the realm
+ * @param classLoader the base class loader for the realm, may be null
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id, @Nullable ClassLoader classLoader) throws DuplicateRealmException;
+
+ /**
+ * Creates a new filtered class realm with the specified id, base class loader, and filter.
+ *
+ * @param id the unique identifier for the realm
+ * @param classLoader the base class loader for the realm, may be null
+ * @param filter the filter to apply to class loading, may be null
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id, @Nullable ClassLoader classLoader, @Nullable Predicate<String> filter)
+ throws DuplicateRealmException;
+
+ /**
+ * Retrieves the class realm with the specified id.
+ *
+ * @param id the realm identifier
+ * @return the class realm
+ * @throws NoSuchRealmException if no realm with the specified id exists
+ */
+ @Nonnull
+ ClassRealm getRealm(@Nonnull String id) throws NoSuchRealmException;
Review Comment:
💡 **Minor:** `getRealm(id)` / `getClassRealm(id)` take an argument so the no-arg accessor rule doesn't strictly apply, but `realm(id)` / `classRealm(id)` would be more consistent with the Maven 4 API style.
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/DuplicateRealmException.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Exception thrown when attempting to create a class realm with an identifier
+ * that already exists in the class world.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class DuplicateRealmException extends ClassWorldException {
+
+ /**
+ * The duplicate realm identifier.
+ */
+ private final String id;
+
+ /**
+ * Constructs a new DuplicateRealmException.
+ *
+ * @param world the class world
+ * @param id the duplicate realm identifier
+ */
+ public DuplicateRealmException(@Nonnull ClassWorld world, @Nonnull String id) {
+ super(world, "Duplicate realm: " + id);
+ this.id = id;
+ }
+
+ /**
+ * Returns the duplicate realm identifier.
+ *
+ * @return the realm identifier
+ */
+ @Nonnull
+ public String getId() {
Review Comment:
⚠️ **Noun-style accessor convention:** `getId()` → `id()` for consistency with the API.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #13490:
URL: https://github.com/apache/maven/pull/13490
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13490:
URL: https://github.com/apache/maven/pull/13490#issuecomment-6192969349
Thanks for the input, @rmannibucau.
The API is explicitly `@Experimental`, so refinement is expected and welcome. That said, the world/realm model is intentional here rather than inherited clutter:
1. **Realm isolation is the core invariant** — a bare `ClassLoader` has no notion of a named, managed realm with controlled import/export relationships. Extensions already depend on realm identity (they call `ClassRealmManager` to look up realms by ID, import from specific realms, etc.). Exposing this through a simpler `ClassLoader`-alike would either lose those semantics or push the complexity somewhere else.
2. **`Strategy` is an extension point** — it controls load-order (self-first vs. import-first). Hiding it would make the API less powerful without reducing implementation complexity.
3. **Primary goal of this PR is internalization, not API design** — the API surface is the minimum needed to type-safely bridge the internalised impl to callers. It deliberately mirrors the existing classworlds contracts so extensions don't break. A redesign of the model itself is a separate, larger conversation.
Happy to iterate on the API shape in a follow-up once the internalization lands. Is there a specific concept you'd drop or collapse? That would make the design conversation more concrete.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4268405657
##########
api/maven-api-classworlds/src/main/java/org/apache/maven/api/classworlds/ClassWorld.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.classworlds;
+
+import java.io.Closeable;
+import java.util.function.Predicate;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * A collection of {@link ClassRealm}s, indexed by id.
+ * <p>
+ * A ClassWorld provides a container for managing multiple class loading realms,
+ * each with their own classpath and import/export relationships.
+ * </p>
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface ClassWorld extends Closeable {
+
+ /**
+ * Creates a new class realm with the specified id.
+ *
+ * @param id the unique identifier for the realm
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id) throws DuplicateRealmException;
+
+ /**
+ * Creates a new class realm with the specified id and base class loader.
+ *
+ * @param id the unique identifier for the realm
+ * @param classLoader the base class loader for the realm, may be null
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id, @Nullable ClassLoader classLoader) throws DuplicateRealmException;
+
+ /**
+ * Creates a new filtered class realm with the specified id, base class loader, and filter.
+ *
+ * @param id the unique identifier for the realm
+ * @param classLoader the base class loader for the realm, may be null
+ * @param filter the filter to apply to class loading, may be null
+ * @return the newly created class realm
+ * @throws DuplicateRealmException if a realm with the same id already exists
+ */
+ @Nonnull
+ ClassRealm newRealm(@Nonnull String id, @Nullable ClassLoader classLoader, @Nullable Predicate<String> filter)
+ throws DuplicateRealmException;
+
+ /**
+ * Retrieves the class realm with the specified id.
+ *
+ * @param id the realm identifier
+ * @return the class realm
+ * @throws NoSuchRealmException if no realm with the specified id exists
+ */
+ @Nonnull
+ ClassRealm getRealm(@Nonnull String id) throws NoSuchRealmException;
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable — the noun-style accessor convention applies to `@Immutable` value types only. `ClassWorld` and `ClassRealm` are mutable objects (realms are created, disposed, imports added, parent realms changed), so the standard `get`-prefixed accessors are correct here.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4268399705
##########
impl/maven-core/src/main/resources/META-INF/maven/extension.xml:
##########
@@ -154,7 +151,7 @@ under the License.
<exportedArtifact>org.apache.maven:maven-api-xml</exportedArtifact>
<exportedArtifact>classworlds:classworlds</exportedArtifact>
- <exportedArtifact>org.codehaus.plexus:plexus-classworlds</exportedArtifact>
+ <exportedArtifact>org.apache.maven:maven-classworlds</exportedArtifact>
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — `maven-api-classworlds` is now included in `<exportedArtifacts>` alongside `maven-classworlds`.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -150,22 +147,6 @@ public final int invoke(InvokerRequest invokerRequest) {
} finally {
Thread.currentThread().setContextClassLoader(oldCL);
System.setProperties(oldProps);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Not applicable to this PR — `LookupInvoker.java` has no diff against master. The JUL handler fix was already merged separately via #13363.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: internalize plexus-classworlds and expose stable API (fixes #11028) [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13490:
URL: https://github.com/apache/maven/pull/13490#discussion_r4271995665
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/MavenCling.java:
##########
@@ -64,18 +64,19 @@ public static void main(String[] args) throws IOException {
* This makes {@code MavenCling} the default entry point that external tools (such as the Maven Wrapper
* or IDEs) can rely on without needing to set the {@code maven.mainClass} property.
*/
- public static int main(String[] args, ClassWorld world) throws IOException {
+ public static int main(String[] args, org.codehaus.plexus.classworlds.ClassWorld world) throws IOException {
String mainClass = System.getProperty(MAVEN_MAIN_CLASS_PROPERTY);
if (mainClass != null && !MavenCling.class.getName().equals(mainClass)) {
return delegateMain(mainClass, args, world);
}
return new MavenCling(world).run(args, null, null, null, false);
}
- private static int delegateMain(String mainClass, String[] args, ClassWorld world) throws IOException {
+ private static int delegateMain(String mainClass, String[] args, org.codehaus.plexus.classworlds.ClassWorld world)
+ throws IOException {
try {
Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass(mainClass);
- Method method = clazz.getMethod("main", String[].class, ClassWorld.class);
+ Method method = clazz.getMethod("main", String[].class, org.apache.maven.api.classworlds.ClassWorld.class);
Review Comment:
⚠️ **`mvn --enc`, `mvn --shell`, and `mvn --up` are broken by this change.**
`delegateMain()` now exclusively looks up `main(String[], org.apache.maven.api.classworlds.ClassWorld)`, but `MavenEncCling`, `MavenShellCling`, and `MavenUpCling` all declare their enhanced main with the **impl** type `org.codehaus.plexus.classworlds.ClassWorld`:
```java
// MavenEncCling.java line 50
public static int main(String[] args, org.codehaus.plexus.classworlds.ClassWorld world) throws IOException {
// MavenShellCling.java line 50 — same
// MavenUpCling.java line 50 — same
```
The launcher scripts set `maven.mainClass` to these classes for `--enc`, `--shell`, and `--up`. At runtime, `getMethod("main", String[].class, api.ClassWorld.class)` throws `NoSuchMethodException`, wrapped as `IOException("maven.mainClass does not have main(String[], ClassWorld) method: ...")`.
Only `MavenValCling` was already migrated to the API type. Either migrate the remaining three to `api.ClassWorld` (preferred, consistent with the direction of this PR), or make `delegateMain()` defensive like `getEnhancedMainMethod()` does:
```java
Method method;
try {
method = clazz.getMethod("main", String[].class, org.apache.maven.api.classworlds.ClassWorld.class);
} catch (NoSuchMethodException ignored) {
method = clazz.getMethod("main", String[].class, org.codehaus.plexus.classworlds.ClassWorld.class);
}
```
The cleaner fix is to align all `*Cling` classes to use `org.apache.maven.api.classworlds.ClassWorld` — that's the direction the API introduction was meant to enable.
##########
impl/maven-classworlds/src/main/java/org/codehaus/plexus/classworlds/launcher/Launcher.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.plexus.classworlds.launcher;
+
+/*
+ * Copyright 2001-2006 Codehaus Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+import org.codehaus.plexus.classworlds.ClassWorld;
+import org.codehaus.plexus.classworlds.realm.ClassRealm;
+import org.codehaus.plexus.classworlds.realm.DuplicateRealmException;
+import org.codehaus.plexus.classworlds.realm.NoSuchRealmException;
+
+/**
+ * <p>Command-line invokable application launcher.</p>
+ *
+ * <p>This launcher class assists in the creation of classloaders and <code>ClassRealm</code>s
+ * from a configuration file and the launching of the application's <code>main</code>
+ * method from the correct class loaded through the correct classloader.</p>
+ *
+ * <p> The path to the configuration file is specified using the <code>classworlds.conf</code>
+ * system property, typically specified using the <code>-D</code> switch to
+ * <code>java</code>.</p>
+ *
+ * @author <a href="mailto:bob@eng.werken.com">bob mcwhirter</a>
+ */
+public class Launcher {
+ protected static final String CLASSWORLDS_CONF = "classworlds.conf";
+
+ protected static final String UBERJAR_CONF_DIR = "WORLDS-INF/conf/";
+
+ protected ClassLoader systemClassLoader;
+
+ protected String mainClassName;
+
+ protected String mainRealmName;
+
+ protected ClassWorld world;
+
+ private int exitCode = 0;
+
+ public Launcher() {
+ this.systemClassLoader = Thread.currentThread().getContextClassLoader();
+ }
+
+ public void setSystemClassLoader(ClassLoader loader) {
+ this.systemClassLoader = loader;
+ }
+
+ public ClassLoader getSystemClassLoader() {
+ return this.systemClassLoader;
+ }
+
+ public int getExitCode() {
+ return exitCode;
+ }
+
+ public void setAppMain(String mainClassName, String mainRealmName) {
+ this.mainClassName = mainClassName;
+ this.mainRealmName = mainRealmName;
+ }
+
+ public String getMainRealmName() {
+ return this.mainRealmName;
+ }
+
+ public String getMainClassName() {
+ return this.mainClassName;
+ }
+
+ public void setWorld(ClassWorld world) {
+ this.world = world;
+ }
+
+ public ClassWorld getWorld() {
+ return this.world;
+ }
+
+ /**
+ * Configure from a file.
+ *
+ * @param is The config input stream.
+ * @throws IOException If an error occurs reading the config file.
+ * @throws MalformedURLException If the config file contains invalid URLs.
+ * @throws ConfigurationException If the config file is corrupt.
+ * @throws org.codehaus.plexus.classworlds.realm.DuplicateRealmException If the config file defines two realms
+ * with the same id.
+ * @throws org.codehaus.plexus.classworlds.realm.NoSuchRealmException If the config file defines a main entry
+ * point in a non-existent realm.
+ */
+ public void configure(InputStream is)
+ throws IOException, ConfigurationException, DuplicateRealmException, NoSuchRealmException {
+ Configurator configurator = new Configurator(this);
+ configurator.configure(is);
+ }
+
+ /**
+ * Retrieve the main entry class.
+ *
+ * @return The main entry class.
+ * @throws ClassNotFoundException If the class cannot be found.
+ * @throws NoSuchRealmException If the specified main entry realm does not exist.
+ */
+ public Class<?> getMainClass() throws ClassNotFoundException, NoSuchRealmException {
+ return getMainRealm().loadClass(getMainClassName());
+ }
+
+ /**
+ * Retrieve the main entry realm.
+ *
+ * @return The main entry realm.
+ * @throws NoSuchRealmException If the specified main entry realm does not exist.
+ */
+ public ClassRealm getMainRealm() throws NoSuchRealmException {
+ return getWorld().getRealm(getMainRealmName());
+ }
+
+ /**
+ * Retrieve the enhanced main entry method.
+ *
+ * @return The enhanced main entry method.
+ * @throws ClassNotFoundException If the main entry class cannot be found.
+ * @throws NoSuchMethodException If the main entry method cannot be found.
+ * @throws NoSuchRealmException If the main entry realm cannot be found.
+ */
+ protected Method getEnhancedMainMethod()
+ throws ClassNotFoundException, NoSuchMethodException, NoSuchRealmException {
+ // Try both the impl ClassWorld and the stable API ClassWorld interface,
+ // so that entry points declared with org.apache.maven.api.classworlds.ClassWorld
+ // (e.g. MavenValCling, MavenCling) are discovered correctly.
Review Comment:
💬 **Inaccurate comment:** `MavenCling` uses the **impl** `ClassWorld` type, not the API type — it is found by the **first** candidate in the loop, not the second. The comment implies both classes require the API candidate, which is misleading.
Suggest:
```suggestion
// Try both the impl ClassWorld and the stable API ClassWorld interface.
// Entry points using impl ClassWorld (e.g. MavenCling) are found by the first candidate;
// entry points using the stable API (e.g. MavenValCling) are found by the second.
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org