You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Hiteshsai007 (via GitHub)" <gi...@apache.org> on 2026/10/19 08:49:11 UTC
[PR] Add scoped plugin service discovery API [maven]
Hiteshsai007 opened a new pull request, #13625:
URL: https://github.com/apache/maven/pull/13625
Expose a Maven 4 PluginManager service that resolves declared project plugins into public runtime metadata and performs component lookups with the plugin realm as the context classloader.
This removes the need for build extensions to combine internal plugin manager and container APIs for plugin-contributed services.
<!--
Please open pull requests against one of the following branches:
- master — default target for all changes (new features + bug fixes).
If the change should also go to maven-4.0.x, request a backport in the PR.
- maven-3.10.x — target for 3.x changes (new features + bug fixes).
If the change should also go to maven-3.9.x, request a backport in the PR.
maven-3.9.x should receive only serious bug fixes, as it is approaching EOL.
Backports are typically handled by maintainers;
however, after agreement you may open a separate backport PR to the target maintenance branch.
-->
<!-- Add your description here... what, why and how you resolve the issue -->
---
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
check one of the following:
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
- [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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] Add scoped plugin service discovery API [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13625:
URL: https://github.com/apache/maven/pull/13625#discussion_r4326388142
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Realm.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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;
+
+import java.util.Optional;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.services.Lookup;
+
+/**
+ * A runtime scope that bundles classloading with scoped component discovery.
+ * Used uniformly for sessions, projects, and plugins — the difference is
+ * what the {@link #lookup()} sees, not the type.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface Realm {
+
+ /**
+ * Returns the classloader for this realm.
+ *
+ * @return the classloader, never {@code null}
+ */
+ @Nonnull
+ ClassLoader classLoader();
+
+ /**
+ * Returns the JPMS module layer when this realm runs in a modular configuration.
+ *
+ * @return the module layer, or an empty optional for a classpath-based realm
+ */
+ @Nonnull
+ Optional<ModuleLayer> moduleLayer();
+
+ /**
+ * Returns component lookup scoped to this realm's visibility.
+ *
+ * @return the realm-scoped component lookup, never {@code null}
+ */
Review Comment:
💡 The Javadoc says "Returns component lookup scoped to this realm's visibility" — but the actual scoping mechanism is TCCL-switching only. `DefaultRealm` wraps the **global** `Lookup` injected at construction; `RealmLookup` switches the thread context classloader to the plugin realm before delegating, but whether Sisu restricts returned components to those visible in that realm is a container-internal behavior, not a contract of this API.
Consider softening the claim to describe what the implementation actually guarantees:
```suggestion
/**
* Returns component lookup that delegates to the container with this realm's
* classloader set as the thread context classloader. The visibility of discovered
* components depends on the container's TCCL-based filtering behavior.
*
* @return the realm-aware component lookup, never {@code null}
*/
```
--
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] Add scoped plugin service discovery API [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13625:
URL: https://github.com/apache/maven/pull/13625#discussion_r4315381332
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/PluginManager.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.services;
+
+import org.apache.maven.api.Plugin;
+import org.apache.maven.api.Project;
+import org.apache.maven.api.Service;
+import org.apache.maven.api.Session;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Provides access to plugin runtimes and the services contributed by them.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface PluginManager extends Service {
+
+ /**
+ * Resolves a plugin, creates its classloader when necessary, and returns its runtime metadata.
+ * The returned plugin exposes its descriptor, resolved dependencies, artifact, and runtime realm.
+ *
+ * @param session the session in which to resolve the plugin
+ * @param project the project declaring the plugin
+ * @param plugin the plugin declaration from the project's model
+ * @return the resolved plugin runtime
+ * @throws PluginManagerException if the plugin cannot be resolved or loaded
+ * @throws NullPointerException if any argument is {@code null}
Review Comment:
💡 **Missing `@throws IllegalArgumentException`** — `findPlugin()` throws `IllegalArgumentException` when the requested plugin is not declared in the project’s build plugins. This exception propagates unchecked through `loadPlugin()` but is absent from the Javadoc contract. Callers have no way to distinguish “plugin not declared” from “plugin failed to load” without reading the implementation.
```suggestion
* @throws PluginManagerException if the plugin cannot be resolved or loaded
* @throws IllegalArgumentException if the plugin is not declared by the given project
* @throws NullPointerException if any argument is {@code null}
```
--
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] Add scoped plugin service discovery API [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #13625:
URL: https://github.com/apache/maven/pull/13625#issuecomment-6264417046
We've updated the classloader improvements tracking issue (#13223) to include a **Layer 0b — Public Realm API** that aligns with the direction of this PR but proposes a slightly different shape.
The idea: instead of exposing `Plugin.getClassLoader()` and having `PluginManager.lookupList()` do manual TCCL swapping, introduce a `PluginRealm` concept that bundles a classloader with a scoped `Lookup`:
```java
public interface PluginRealm {
ClassLoader getClassLoader();
Optional<ModuleLayer> getModuleLayer(); // for future JPMS ModuleLayer support
Lookup getLookup(); // scoped to this realm's import chain
}
```
`Plugin.getRealm()` replaces `Plugin.getClassLoader()`, and callers do `plugin.getRealm().getLookup().lookupList(type)` — no manual `Thread.setContextClassLoader()` needed.
Benefits:
- **Encapsulates TCCL handling** inside the `Lookup` implementation
- **Abstracts classpath vs JPMS** — same API for classpath-based and `ModuleLayer`-based realms
- **Enforces visibility** — the `Lookup` is pre-scoped, so component visibility follows the realm's import chain by construction
- **Removes `PluginManager.lookupList()`** — the realm owns its scoped lookup directly
See #13223 for the full plan, specifically the "Layer 0b" and "Design Principle — Component Visibility Follows Classloading Rules" sections.
_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] Add scoped plugin service discovery API [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13625:
URL: https://github.com/apache/maven/pull/13625#discussion_r4315574096
##########
api/maven-api-core/src/main/java/org/apache/maven/api/PluginRealm.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;
+
+import java.util.Optional;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.services.Lookup;
+
+/**
+ * Represents the runtime scope of a Maven plugin.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface PluginRealm {
+
+ /**
+ * Returns the classloader for this realm.
+ *
+ * @return the classloader, never {@code null}
+ */
+ @Nonnull
+ ClassLoader classLoader();
+
+ /**
+ * Returns the JPMS module layer when this plugin runs as a modular plugin.
+ *
+ * @return the module layer, or an empty optional for a classpath-based realm
+ */
+ @Nonnull
+ Optional<ModuleLayer> moduleLayer();
+
+ /**
+ * Returns component lookup scoped to this realm's visibility.
+ *
+ * @return the realm-scoped component lookup, never {@code null}
Review Comment:
💡 The Javadoc says "Returns component lookup scoped to this realm's visibility" — but `DefaultPluginRealm.RealmLookup.scoped()` only swaps the Thread Context Class Loader (TCCL). No ClassWorlds realm-scoped visibility isolation occurs. The wording overstates the contract.
```suggestion
* Returns component lookup with this realm's class loader set as the Thread Context Class Loader.
*
* @return the lookup with TCCL bound to this realm's class loader, never {@code null}
```
--
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] Add scoped plugin service discovery API [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13625:
URL: https://github.com/apache/maven/pull/13625#discussion_r4313771635
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Plugin.java:
##########
@@ -46,7 +46,7 @@ public interface Plugin {
List<Lifecycle> getLifecycles();
@Nonnull
- ClassLoader getClassLoader();
+ PluginRealm getRealm();
Review Comment:
⚠️ **Breaking change** — `getClassLoader()` is removed from the `Plugin` API and replaced by `getRealm()`. This is a source and binary breaking change for all existing embedders calling `plugin.getClassLoader()`.
Please also add `@since 4.1.0` on `getRealm()` — the class-level `@since 4.0.0` does not cover new methods added later.
```suggestion
/**
* Returns the runtime realm for this plugin.
*
* @return the plugin realm
* @since 4.1.0
*/
@Nonnull
PluginRealm getRealm();
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/PluginManagerException.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.services;
+
+import org.apache.maven.api.annotations.Experimental;
+
+/**
+ * Thrown when a plugin runtime cannot be resolved or loaded.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public class PluginManagerException extends MavenException {
+
+ public PluginManagerException(String message, Throwable cause) {
Review Comment:
💡 Two issues with `PluginManagerException`:
1. **Missing `@Serial serialVersionUID`** — every `Serializable` subclass in maven-api should declare a `serialVersionUID` (see `BuildException`, `VersionRangeResolverException`, etc.).
2. **Single constructor** — consider adding a message-only constructor for cases where there is no underlying cause (e.g. a validation failure in `findPlugin`), following the convention of other `MavenException` subclasses.
```suggestion
@java.io.Serial
private static final long serialVersionUID = 1L;
public PluginManagerException(String message) {
super(message);
}
public PluginManagerException(String message, Throwable cause) {
super(message, cause);
}
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/PluginRealm.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+import java.util.Optional;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.services.Lookup;
+
+/**
+ * Represents the runtime scope of a Maven plugin.
+ *
+ * @since 4.1.0
+ */
+@Experimental
+public interface PluginRealm {
+
+ /**
+ * Returns the classloader for this realm.
+ */
+ @Nonnull
+ ClassLoader getClassLoader();
Review Comment:
💡 **Missing `@return` tags** — all three methods have "Returns…" prose in their Javadoc but no `@return` tag. Maven API convention requires the `@return` tag. Same applies to `getModuleLayer()` (L45) and `getLookup()` (L51).
```suggestion
/**
* Returns the classloader for this realm.
*
* @return the classloader, never {@code null}
*/
@Nonnull
ClassLoader getClassLoader();
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultPluginManager.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.internal.impl;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Objects;
+
+import org.apache.maven.api.Plugin;
+import org.apache.maven.api.Project;
+import org.apache.maven.api.Session;
+import org.apache.maven.api.services.Lookup;
+import org.apache.maven.api.services.PluginManager;
+import org.apache.maven.api.services.PluginManagerException;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.lifecycle.internal.LifecyclePluginResolver;
+import org.apache.maven.plugin.MavenPluginManager;
+import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.project.MavenProject;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.maven.internal.impl.CoreUtils.cast;
+
+@Named
+@Singleton
+public final class DefaultPluginManager implements PluginManager {
+
+ private final MavenPluginManager pluginManager;
+ private final LifecyclePluginResolver lifecyclePluginResolver;
+ private final Lookup lookup;
+
+ @Inject
+ public DefaultPluginManager(
+ MavenPluginManager pluginManager, LifecyclePluginResolver lifecyclePluginResolver, Lookup lookup) {
+ this.pluginManager = requireNonNull(pluginManager, "pluginManager");
+ this.lifecyclePluginResolver = requireNonNull(lifecyclePluginResolver, "lifecyclePluginResolver");
+ this.lookup = requireNonNull(lookup, "lookup");
+ }
+
+ @Override
+ public Plugin loadPlugin(Session session, Project project, org.apache.maven.api.model.Plugin plugin) {
+ requireNonNull(session, "session");
+ requireNonNull(project, "project");
+ requireNonNull(plugin, "plugin");
+
+ InternalMavenSession internalSession = InternalMavenSession.from(session);
+ MavenProject mavenProject =
+ cast(DefaultProject.class, project, "project").getProject();
+ MavenSession executionSession = internalSession.getMavenSession().clone();
+ executionSession.setCurrentProject(mavenProject);
+
+ try {
+ lifecyclePluginResolver.resolveMissingPluginVersions(mavenProject, executionSession);
+ org.apache.maven.model.Plugin legacyPlugin = findPlugin(mavenProject, plugin);
+ PluginDescriptor descriptor = pluginManager.getPluginDescriptor(
+ legacyPlugin, mavenProject.getRemotePluginRepositories(), executionSession.getRepositorySession());
+ pluginManager.setupPluginRealm(descriptor, executionSession, null, null, null);
+ return new DefaultPlugin(internalSession, legacyPlugin, descriptor, lookup);
+ } catch (Exception e) {
+ throw new PluginManagerException(
+ "Unable to load plugin " + plugin.getGroupId() + ":" + plugin.getArtifactId(), e);
Review Comment:
⚠️ **`catch(Exception)` masks `IllegalArgumentException` from `findPlugin()`** — `findPlugin()` throws `IllegalArgumentException` when the plugin is not declared by the project. That exception gets caught here and re-thrown as a generic `"Unable to load plugin …"` message, losing the specific context ("Plugin X:Y is not declared by project Z").
Consider catching the specific infrastructure exceptions from `pluginManager` separately and letting `IllegalArgumentException` propagate directly, or at minimum wrapping `findPlugin` outside the `try` block:
```suggestion
requireNonNull(session, "session");
requireNonNull(project, "project");
requireNonNull(plugin, "plugin");
InternalMavenSession internalSession = InternalMavenSession.from(session);
MavenProject mavenProject =
cast(DefaultProject.class, project, "project").getProject();
MavenSession executionSession = internalSession.getMavenSession().clone();
executionSession.setCurrentProject(mavenProject);
lifecyclePluginResolver.resolveMissingPluginVersions(mavenProject, executionSession);
org.apache.maven.model.Plugin legacyPlugin = findPlugin(mavenProject, plugin);
try {
PluginDescriptor descriptor = pluginManager.getPluginDescriptor(
legacyPlugin, mavenProject.getRemotePluginRepositories(), executionSession.getRepositorySession());
pluginManager.setupPluginRealm(descriptor, executionSession, null, null, null);
return new DefaultPlugin(internalSession, legacyPlugin, descriptor, lookup);
} catch (Exception e) {
throw new PluginManagerException(
"Unable to load plugin " + plugin.getGroupId() + ":" + plugin.getArtifactId(), e);
}
```
--
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