maven icon indicating copy to clipboard operation
maven copied to clipboard

PoC/experiment: new scope "import-plugins"

Open cstamas opened this issue 2 years ago • 13 comments

Introduces new scope very similar to "import" but this one is "import-plugins" and as it's name says, imports pluginManagement.

To test it:

  • build this PR
  • use this POM https://gist.github.com/cstamas/8668205f68b0eaf42c8c34bba7d080a4
  • invoke mvn help:effective-pom

Enjoy!

cstamas avatar Jun 28 '23 10:06 cstamas

@cstamas This is interesting and could almost be a migration path for tiles-maven-plugin - I wonder if having a new scope is good tho, rather than reusing the existing import scope, but included in the <pluginManagement/> section itself.

I'll pull the PR in the morning and play with it.

talios avatar Jun 28 '23 11:06 talios

see https://issues.apache.org/jira/browse/MNG-5588 like @talios : why not simplly use import for pluginManagement like it is done for dependencyManagement? answer from MNG-5588: there is no scope in pluginManagement to abuse in MNG-5588, I abused instead the inherited scope... (I can't find my implementation for now...) https://issues.apache.org/jira/browse/MNG-5588?focusedCommentId=17318534&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17318534

hboutemy avatar Jun 28 '23 12:06 hboutemy

Um, inherited=import does not look intuitive at all to me :) while scope import-plugins does, and tells at first sight what it does IMHO. Also, as existing "import", the new scope is special is same way: is valid only where other "import" is.

cstamas avatar Jun 28 '23 12:06 cstamas

putting a plugin-related scope on a dependencyManagement does not look intuitive either

hboutemy avatar Jun 28 '23 12:06 hboutemy

"plugin-related scope"? Plugins does not have scopes :smile:

OTOH, if you look at original definition of "import" scope: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-scope

"This scope is only supported on a dependency of type pom in the <dependencyManagement> section. It indicates the dependency is to be replaced with the effective list of dependencies in the specified POM's <dependencyManagement> section. Since they are replaced, dependencies with a scope of import do not actually participate in limiting the transitivity of a dependency."

The new "import-plugins" works exactly the same, except just replace dependencyManagement with pluginManagement. So, is even intuitive in this way as well, and devs will understand it easier:

"This scope is only supported on a dependency of type pom in the <dependencyManagement> section. It indicates the dependency is to be REMOVED and the effective list of plugins in the specified POM's <pluginManagement> section ADDED. Since they are REMOVED, dependencies with a scope of import do not actually participate in limiting the transitivity of a dependency."

cstamas avatar Jun 28 '23 12:06 cstamas

@talios @hboutemy also, what is the problem with adding new scope (that is special,in very same way as existing "import" scope is special)?

cstamas avatar Jun 28 '23 12:06 cstamas

And an idea from comment on MNG-5588: IF you are importing a POM, why not import everything? (And then need for new scope really goes away). Users can prepare POMs with parts they want (depMgt only, pluginMgt only, or both) and import.

(but this carries potential of breaking builds, that do import some POM for it's depMgt but are not interested, or worse, would break them if pluginMgt is imported...)

cstamas avatar Jun 28 '23 12:06 cstamas

The issue with "import" magic is that very quickly you will also need the conflict resolution management until you import a single piece which is basically a parent.

Think extensions are a nice way to configure plugins (bad for dependencies since this part must always load in an IDE without code execution but it is not the case for plugins) but for the case you want to inherit from something done outside we could just use xml references, which would enable to import a template and customize it inline instead of importing as such which often leads to issues.

So from my window we shouldn't enable much on dependencies but we can go quite far now we have the consumer model transformation pipeline by enabling to reference a plugin from an imported resource (can need an import block but not a big deal) and customize it inline, even with required placeholders if needed. I'd really love to see it done within an extension and using the xml transformer feature and without hacking the core yet to validate the usage since this part is quite hard to design right upfront.

rmannibucau avatar Jun 28 '23 13:06 rmannibucau

And an idea from comment on MNG-5588: IF you are importing a POM, why not import everything? (And then need for new scope really goes away). Users can prepare POMs with parts they want (depMgt only, pluginMgt only, or both) and import.

That's essentially what tiles-maven-plugin does - only it does it by synthetically injected the tile-pom as a parent (and has its own transitive tiles, which I don't like - but users do).

Because it's pulling in everything - you get the plugin definition and the executions, so you don't need to repeat all the plugins in the child pom (but that I could probably accept).

talios avatar Jun 29 '23 02:06 talios

@talios @hboutemy also, what is the problem with adding new scope (that is special,in very same way as existing "import" scope is special)?

No real problem - more questioning the naming (which may be too early in the change to quibble over naming), tho reading the reasoning of 'plugins don't have scope' that means this is possibly more a trade-off due to current limitations of the POM format.

@rmannibucau's comment:

Think extensions are a nice way to configure plugins

makes me think back to the current tiles implementation which operates as an extension, hooking into the model builder - but with the immutable model in M4 doesn't work - an new extension point for model manipulation would be nice.

One nice thing with tiles, by pulling as parents - we also pull in any ` defined, which can also then be overridden/customised.

talios avatar Jun 29 '23 02:06 talios

Well, if v4 does not allow tiles to be implemented it means the plugin API is not yet sufficient and can't be used so we must fix it, can be nice to work on that, intent is to copy instead of mutating, not breaking use cases.

rmannibucau avatar Jun 29 '23 05:06 rmannibucau

[...] tho reading the reasoning of 'plugins don't have scope' that means this is possibly more a trade-off due to current limitations of the POM format.

Plugins does not and will not have "scope". Why would a plugin need "scope"? IMHO this is not a deficiency but was a deliberate design goal.

cstamas avatar Jun 29 '23 14:06 cstamas

As we all know, the "import" scope is not really a scope, it is a hack. And we do already have that hack in place, and it works. So, am really unsure why we either:

  • continue on that trail of hacks and implement other (very similar) hacks, or,
  • get rid of the original hack altogether (due all the issues it brings in)

So, why are we sitting comfy and not doing anything about it? This PR does 1st bullet, but for sure we can get a stab on 2nd bullet as well, as we decide...

cstamas avatar Dec 16 '23 22:12 cstamas