-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MPOM-337] load plugins containing lifecycle mappings with extensions #92
base: master
Are you sure you want to change the base?
[MPOM-337] load plugins containing lifecycle mappings with extensions #92
Conversation
-1 Edit: wrong analysis, mixed Core Extensions and Build Extensions, see https://issues.apache.org/jira/browse/MNG-7574 |
@hboutemy What is the ultimate goal here? Not requiring plugins coming with lifecycle mappings to be loaded as extensions and still consider their mappings? What exactly is the concern you have in case those few are loaded as extensions? |
@gnodet I opened https://issues.apache.org/jira/browse/MNG-7574 to address the concern of @hboutemy. Hopefully soon the plugins themselves identifies those classes which should be exposed through the Core Classloader which would make this PR obsolete (but this would require a recent Maven version and an updated plugin version containing the new descriptor exposing the extension classes). |
Robert opened MNG-5697 as a long term target, that shows an ideal situation where plugins can provide their lifecycle mapping themselves instead of having core define everything for them https://maven.apache.org/ref/3.9.1/maven-core/default-bindings.html perfect objective. Has Maven core the features required for that? NO. Why is this "plugin as extension" feature not ok? for MNG-5697 to work, there are multiple Maven core feature required:
For plugins that have their bindings coded in Maven core, this is hard to see because you benefit from default bindings, and it's not to check that it's plugin-provided binding that has been used instead of core one |
notice: re-reading https://maven.apache.org/guides/mini/guide-maven-classloading.html#build-extension-classloaders , perhaps I'm confused on "Build Extension Classloaders" vs "Core Classloader": that would be a good thing, because that would solve one of my concerns the concern on early load remains |
one case to test is our doc on declaring it is https://maven.apache.org/archetype/archetype-packaging/ : the packaging is a separate artifact declared as extension you can try to add the packaging artifact as an additional dependency of m-archetype-p and declare the plugin as an extension, and you should see that the packaging is not available when the build plan is calculated |
But this only affects components and I fail to see the negative impact as most (if not all components) are actually necessary to register custom types.
Sorry, I cannot follow here. Can you elaborate a bit how this is related to this PR? |
ok, I've not been clear, sorry. I'll try to explain it another way. simply put, MNG-5697 does not work yet, and this PR does as if it worked (with a confusion between bindings provided by Maven core vs bindings that should be provided by the plugin) that's why I say that once we prove that it works for a packaging plugin that is not provided by Maven core (like archetype), we'll see how we transition for the currently Maven core provided ones |
But before removing bindings from core they should be loaded through plugin extensions (not the other way around). This PR prepares for future Maven version which do no longer include those bindings in core. |
yes, we agree: removing is far future |
I use maven-bundle plugin and https://jackrabbit.apache.org/filevault-package-maven-plugin/. Both use custom types and bindings and everything works fine when they are loaded with extensions. Don’t know why your example with archetype is different. |
I'm not really sure I understand the purpose of the extension element here, or how that relates to future changes to lifecycle mappings. However, I only expect the Apache parent POM to do version management for my plugins. I don't expect it to be responsible for execution decisions. Shouldn't these go in the child project's POM's |
The loading with extensions has no impact on goal execution. |
I wasn't referring to goal executions. I was referring to the explicit choice to execute whatever the |
For me the decision whether a plugin should be loaded with or without extensions should primarily be driven by the plugin developer (not by the consumer). In general plugins usually don't work correctly if they ship with extensions but they aren't loaded. Compare also with https://issues.apache.org/jira/browse/MNG-7572. The fact that this needs to be specified during loading and is not yet given by the plugin descriptor I consider a flaw in the design. Do you have a use case where it makes sense that a plugin is loaded without extensions although it provides one? |
I agree.
I agree with that, too.
Not off hand, but if it's configurable by a user, I imagine that there's use cases where a plugin's execution is functionally altered by that configuration. Otherwise, why is this even made available for configuration by users? So long as it's configurable by users, I just seems sensible that configuration should be determined by the user in their |
I think https://issues.apache.org/jira/browse/MNG-598 has the answer. It seems this decision has been made as a workaround to avoid having to scan all plugins for packaging handlers. It seems to me that this was a technical decision rather than a design one. |
For me the underlying design principle is that all plugins not being loaded with extensions |
as written in https://issues.apache.org/jira/browse/MNG-5697 , I think packaging and artifact handlers (aka dependency types) are 2 separate topics that have been conflated until now but should really be decoupled |
This is fine, but doesn't have any impact on this PR, does it? |
@hboutemy Do you still have concerns with merging this? If so can you please elaborate on the why? |
https://issues.apache.org/jira/browse/MPOM-337