Skip to content

[MNG-7588] generate reader and some beans automatically for plugin#862

Draft
kwin wants to merge 2 commits intomasterfrom
feature/generate-plugin-descriptor-bean-and-reader-from-model
Draft

[MNG-7588] generate reader and some beans automatically for plugin#862
kwin wants to merge 2 commits intomasterfrom
feature/generate-plugin-descriptor-bean-and-reader-from-model

Conversation

@kwin
Copy link
Copy Markdown
Member

@kwin kwin commented Oct 30, 2022

descriptor

WIP (i.e. Core module is currently broken)

@kwin kwin requested a review from gnodet October 30, 2022 08:02
@kwin
Copy link
Copy Markdown
Member Author

kwin commented Oct 30, 2022

@gnodet Do we need to keep the old beans in org.apache.maven.plugin.descriptor for backwards compatibility or is that not considered public API?

Comment thread maven-plugin-api/pom.xml
</models>
<templates>
<template>src/main/mdo/model.vm</template>
<template>src/main/mdo/reader.vm</template>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we generate a writer here or rather separately in m-plugin-tools?

@gnodet
Copy link
Copy Markdown
Contributor

gnodet commented Oct 30, 2022

@gnodet Do we need to keep the old beans in org.apache.maven.plugin.descriptor for backwards compatibility or is that not considered public API?

We need to be very cautious for some time and avoid breaking compatibility as much as possible. The new api is still in alpha and we need to upgrade most plugins before even deprecating all the old parts.

Comment thread maven-plugin-api/pom.xml Outdated
<!-- remove those classes which need to ba maintained manually -->
<includes>
<include>MojoDescriptor.java</include>
<include>PluginDescriptor.java</include>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better idea imho would be to tweak the src/mdo/model.vm to add a list of classes to not generate instead of generating them and removing them. A variable similar to packageModelV4 should work.

Copy link
Copy Markdown
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated classes are incompatible with the existing ones, so I fear this is a big change. The maven-plugin-plugin will probably be broken by such a change.

That's why for the v4 api, I've rather created entirely new packages so that we can phase the old ones out while keeping compatibility.

Maybe I'm wrong, but I think the fact that the MojoDescriptor and PluginDescriptor inherits the plexus ComponentSetDescriptor is only used internally in maven. If that's actually the case, I would favor trying to build a brand new and clean object model (without plexus) and only create the plexus required classes from those generated classes when needed.

@kwin
Copy link
Copy Markdown
Member Author

kwin commented Oct 31, 2022

Ok, so when generating descriptor beans in a dedicated package, where should this live? IMHO it doesn't make sense to expose under https://github.com/apache/maven/tree/master/api/maven-api-core. This should rather be a private package (only for consumption of Maven Core and maybe Maven Plugin Tools). I don't think it would be useful to add this to the documented v4 API of Maven. So I am rather thinking about putting the new/clean/immutable plugin descriptor classes into org.apache.maven.plugin.descriptor.v11. WDYT?

@gnodet
Copy link
Copy Markdown
Contributor

gnodet commented Nov 7, 2022

Ok, so when generating descriptor beans in a dedicated package, where should this live? IMHO it doesn't make sense to expose under https://github.com/apache/maven/tree/master/api/maven-api-core. This should rather be a private package (only for consumption of Maven Core and maybe Maven Plugin Tools). I don't think it would be useful to add this to the documented v4 API of Maven. So I am rather thinking about putting the new/clean/immutable plugin descriptor classes into org.apache.maven.plugin.descriptor.v11. WDYT?

Sounds good to me.

@kwin kwin force-pushed the feature/generate-plugin-descriptor-bean-and-reader-from-model branch from 7229cd1 to 4c71859 Compare November 9, 2022 11:02
@kwin
Copy link
Copy Markdown
Member Author

kwin commented Nov 9, 2022

I would like to migrate all usage of Maven Core from the old descriptor to the new immutabledescriptor package. But the question is again, do we consider interfaces like https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/plugin/BuildPluginManager.java API, i.e. do we need to evolve it in a backwards-compatible way? If yes, should we introduce new interfaces relying on the immutable descriptor or just add new methods to the existing interfaces?
@gnodet You changed org.apache.maven.plugin.lifecycle.Lifecycle in a backwards-incompatible way, as Maven 3 exposes setter methods (https://maven.apache.org/ref/3-LATEST/apidocs/org/apache/maven/plugin/lifecycle/Lifecycle.html).

@jira-importer
Copy link
Copy Markdown

Resolve #8933

@slawekjaranowski
Copy link
Copy Markdown
Member

@kwin do you want plan to work on it?

@slawekjaranowski slawekjaranowski added the waiting-for-feedback Waiting for 90 days until issues or pull request will be closed label Apr 10, 2026
@kwin
Copy link
Copy Markdown
Member Author

kwin commented Apr 10, 2026

Not in the near future I fear

@github-actions github-actions Bot removed the waiting-for-feedback Waiting for 90 days until issues or pull request will be closed label Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants