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:[email protected]";>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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
