gnodet commented on PR #13319: URL: https://github.com/apache/maven/pull/13319#issuecomment-6036027010
## Scope: should other extensible registries receive the same treatment? While working on this, I looked at whether the same project-extension visibility gap affects the other registries. Short answer: **yes, at least for `TypeRegistry`**, and it may be worth addressing them with a unified architecture rather than fixing each one individually. ### Registry inventory | Registry | Impl | Scope | Project-ext gap? | |---|---|---|---| | `PackagingRegistry` | `DefaultPackagingRegistry` | `@Singleton` | ✅ Fixed by this PR | | `TypeRegistry` | `DefaultTypeRegistry` | `@Singleton` | ⚠️ Yes — same gap | | `LifecycleRegistry` | `DefaultLifecycleRegistry` | `@Singleton` | ⚠️ Related, different shape | | `PathScopeRegistry` | `DefaultExtensibleEnumRegistry` (maven-impl) | `@SessionScoped` | ✅ None — built-in only | | `ProjectScopeRegistry` | `DefaultExtensibleEnumRegistry` (maven-impl) | `@SessionScoped` | ✅ None — built-in only | | `LanguageRegistry` | `DefaultExtensibleEnumRegistry` (maven-impl) | `@SessionScoped` | ✅ None — built-in only | ### `DefaultTypeRegistry` — same structural problem `DefaultTypeRegistry` already calls `lookup.lookupList(TypeProvider.class)` at call time in `require()`, which is the right approach. However it is `@Singleton`-scoped, so: - A project-extension-contributed `TypeProvider` registered in a project classloader won't be visible to the global SISU container. - The `LegacyArtifactHandlerManager` fallback path runs in global scope — custom `ArtifactHandler`s from project extensions aren't found there either. The fix would follow the same pattern as this PR. ### `DefaultLifecycleRegistry` — related but different shape The lifecycle provider lookup goes through a `LifecycleWrapperProvider` inner class that reads from `PlexusContainer.lookupMap()` at construction time, not at call time. Project-extension-contributed `LifecycleProvider`s won't be picked up. This one is harder to fix cleanly as it involves the Plexus→SISU boundary, but it has the same root cause. ### Suggested approach Rather than fixing each registry ad-hoc, it may be cleaner to generalize the fix introduced here — the `lookup.lookupList(Provider.class)` at call time pattern — into `DefaultExtensibleEnumRegistry` itself (or a new project-scoped variant of it), so all registries that support SPI extension benefit from correct project-classloader visibility automatically. This would also be a good opportunity to review the `@Singleton` vs `@SessionScoped` vs `@ProjectScoped` scoping of these registries, since project-extension contributions inherently require per-project visibility. **Proposed follow-up:** open a tracking issue to address `TypeRegistry` first (most concrete real-world impact), then `LifecycleRegistry`, and consider the unified architecture as the long-term target. -- 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]
