Skip to content
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

Bug 486737 - Support for Maven Extensions (LifeCycleParticipants) #118

Closed
mickaelistria opened this issue Mar 30, 2021 · 21 comments · Fixed by #1455
Closed

Bug 486737 - Support for Maven Extensions (LifeCycleParticipants) #118

mickaelistria opened this issue Mar 30, 2021 · 21 comments · Fixed by #1455
Assignees
Labels
usability Improvements that can give a better usability or user experience

Comments

@mickaelistria
Copy link
Contributor

mickaelistria commented Mar 30, 2021

See https://bugs.eclipse.org/bugs/show_bug.cgi?id=486737

It would be nice if m2e supported Maven's lifecycle participants by instantiating them and calling their AbstractMavenLifecycleParticipant.afterProjectsRead() method.

The other lifecycle methods do not make sense in m2e context.

To make things as safe as possible, one might explicitly whitelist those participants that shall be called during an m2e build.

@laeubi
Copy link
Member

laeubi commented Dec 9, 2021

I think m2e already supports extensions but not extensions defined in the .mvn folder, maybe we should rephrase the ticket?

@cpfeiffer
Copy link
Contributor

AFAIK only the patch from this ticket supports lifecycle participant extensions inside m2e. The extensions in the .mvn folder are Maven Core Extensions (see #116).

@laeubi
Copy link
Member

laeubi commented Dec 12, 2021

Okay It seems I confused this with

as far as I understood the discussion at the Bugzilla there where concerns regarding the set of "reactor projects" are there any progress regarding that discussion?

@laeubi
Copy link
Member

laeubi commented Dec 12, 2021

I just discovered there is a org.eclipse.m2e.core.mavenComponentContributors extension point maybe it would be more suitable for plugins that want to contribute to m2e to add such an extension?

@laeubi
Copy link
Member

laeubi commented Feb 7, 2022

Requires

@cpfeiffer
Copy link
Contributor

@laeubi hi from eclipsecon 😄 This is the ticket we were talking about, yesterday. All the details are in the bugzilla issue at https://bugs.eclipse.org/bugs/show_bug.cgi?id=486737

@laeubi laeubi self-assigned this Oct 27, 2022
@laeubi
Copy link
Member

laeubi commented Oct 27, 2022

@cpfeiffer Tanks for bringing attention to this long outstanding issue.
It seems we have most things at hand to implement this, to bring this to life I think we should probably do th follwoing two steps first:

  1. Enhance the m2e lif-cycle mapping format in a way a plugin can choose if it want to participate or not, we currently already have pluginExecutions > pluginExecution > action what the can take a configurator and we should enhance this to additional can contain lifeCycleParticipants > lifeCycleParticipant mentioning the class that should be enabled in the lif-cycle of project reads and sessions. One might think to also explicticly state the supported methods, but thats probably to much here
  2. It would be great to have a test for the particular plugin that uses the extension and asserts some basic functionality, https://github.com/eclipse-m2e/m2e-core/blob/master/org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/ExtensionsTest.java would be a good place for this it already contains some examples (e.g. pomless) extensions are loaded and a project then gains some properties e.g. testCoreExtension would be a good example.

So if you can provide a test, that would help already much so we have something that could be used for devlopment and final verification of the feature. The maven tiles prject seems activly devloped and thus it might be able to adapt this then quite soon, in the meanwhile one could use workspace global settings to enable that.

@Hanmac
Copy link

Hanmac commented Oct 27, 2022

@laeubi one test example for you from my related issue:

@laeubi
Copy link
Member

laeubi commented Oct 29, 2022

@Hanmac thanks for linking your example here, do you think you can try to put this together into a testcase that then asserts what the expected outcome? I think the tiles example from @cpfeiffer would still be required as such LifeCycleParticipants can serve different purpose, e.g. the aar seem to extract some stuff after resolving while tiles even rewrites the maven model! And to be honest I assume the last one be much more harder, but of course having different examples will help in getting things right.

If this is crucial to someones business and likes to speed up the development in that area a sponsoring would allow me to assign more time-slots particular issue, so if that feasible for anyone let me know ... also feel free to contact me if a direct contract is more appropriate!

@laeubi
Copy link
Member

laeubi commented Oct 29, 2022

It seems there is also a test case with a dummy-listener:

@laeubi
Copy link
Member

laeubi commented Oct 30, 2022

I have played around a bit with all that stuff now and it already works to some extend, but there is one conceptual difference:

In maven there is one constant, single flow of events:

  1. parameter parsing and session setup
  2. calling AbstractMavenLifecycleParticipant.afterSessionStart(...) in case of a core-extension
  3. calling the GraphBuilder and reading the project models
  4. calling AbstractMavenLifecycleParticipant.afterProjectsRead(...) for core and project extensions
  5. calling the GraphBuilder again for final ordering
  6. execute the build
  7. calling AbstractMavenLifecycleParticipant.afterSessionEnd(...) for core and project extensions

In m2e we actually have multiple different executions, especially we don't know where the "root" of execution is and execute each project one-by-one. Then we have phases where we setup the MavenProject or execute incremental builds.

So actually we would need to invoke the LifecycleParticipant multiple times to get things "right" and not only e.g. on building the project model (as a LifecycleParticipant.can literally change everything). Also participants can see "strange" flows, e.g. projects are read but nothing is executed in terms of mojos.

@laeubi
Copy link
Member

laeubi commented Nov 2, 2022

Container setup is now rather clean and consistent, but it seems the project facade could need some more work to make this work consistently...

@lfvjimisola
Copy link

This issue is a bit of blocker. @laeubi As you are assigned to it, what are you thoughts? I saw that it has been around since 2016.
Is it not affecting as many as one would have thought?

@laeubi
Copy link
Member

laeubi commented Feb 15, 2023

@lfvjimisola The main problem is that there are still no full examples / tests especially with expected outcomes for the "blocking" case as asked here #118 (comment)

So yes this is kind of annoying but seems people have workaround or it is not annoying enough for them on the long term, but I'm currently working on it and already have improved extension support but it is not finalized so whenever I have time adding more options.

If it is blocking for you you probably want to provide an as small as possible project, (best as a test-case) where you then describe (or assert) what is expected to be the result for your case. You can even sponsoring me so I can use more time-slots for this.

If it is even crucial for your business, I can offer to contact me for entering a contract to fix this specific issue (or others as well), just let me know.

@lfvjimisola
Copy link

@laeubi It's a bit of blocker. As a long time advocate, supporter and contributor of/to open source I complete understand you. However, I don't have my own business but work for a government agency so sponsoring is not an option (if it is I would not dear to guess how long it would take to get all the approvals 😆 ). So, our team simply has to wait.

@laeubi
Copy link
Member

laeubi commented Feb 17, 2023

@lfvjimisola of course your team can help in other ways, e.g. providing samples, test cases and even providing patches.

So if you can provide a sample with an extension that should "work" together with whats currently not working, this can still help to move the issue forward. If you even setup an IDE and transform it into a test-case then one can even validate that if there are changes / progress your use-case is covered.

@lfvJonas
Copy link

lfvJonas commented Apr 6, 2023

@laeubi here is a basic example with something that should "work". We use a tile to set the Java version (17) and a composite to lock down maven plugin versions. But in the applicaiton defaults to Java version (J2SE-1.5) instead of (17).

MavenExtentions.zip

@laeubi
Copy link
Member

laeubi commented Apr 6, 2023

@lfvjimisola thanks for the example, I'll try to take a look at it, would you mind to contribute it as a PR to this test-case:

https://github.com/eclipse-m2e/m2e-core/blob/master/org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/ExtensionsTest.java

this will actually ensure:

  1. We see currently something is failing (e.g. you can assert for the java version is detected as Java 17)
  2. If I start fixing this we see that the test now succeeds
  3. Any changes in the future are checked to not break existing behavior

@lfvjimisola
Copy link

@laeubi Great. It was actually my colleague @lfvJonas (see above) that provided the example. So, I'll leave it to him to answer if he can provide a test-case.

@laeubi
Copy link
Member

laeubi commented Jun 26, 2023

@lfvjimisola @lfvJonas I now added execution of the participant but the execution fails, I can't see any immediate problem in m2e but maybe the tiles plugin is not compatible with Maven 3.9.1 or expects some setup here:

java.lang.NullPointerException: Cannot invoke "org.eclipse.aether.RepositorySystemSession.getWorkspaceReader()" because "session" is null
	at org.apache.maven.RepositoryUtils.getWorkspace(RepositoryUtils.java:376)
	at org.apache.maven.plugin.DefaultPluginArtifactsCache$CacheKey.<init>(DefaultPluginArtifactsCache.java:70)
	at org.apache.maven.plugin.DefaultPluginArtifactsCache.createKey(DefaultPluginArtifactsCache.java:135)
	at org.apache.maven.plugin.internal.DefaultMavenPluginManager.setupExtensionsRealm(DefaultMavenPluginManager.java:824)
	at org.apache.maven.project.DefaultProjectBuildingHelper.createProjectRealm(DefaultProjectBuildingHelper.java:197)
	at org.apache.maven.project.DefaultModelBuildingListener.buildExtensionsAssembled(DefaultModelBuildingListener.java:101)
	at org.apache.maven.model.building.ModelBuildingEventCatapult$1.fire(ModelBuildingEventCatapult.java:44)
	at org.apache.maven.model.building.DefaultModelBuilder.fireEvent(DefaultModelBuilder.java:1450)
	at org.apache.maven.model.building.DefaultModelBuilder.build(DefaultModelBuilder.java:530)
	at org.apache.maven.model.building.DefaultModelBuilder.build(DefaultModelBuilder.java:454)
	at org.apache.maven.model.building.DefaultModelBuilder.build(DefaultModelBuilder.java:267)
	at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:173)
	at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:124)
	at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.resolveTile(TilesMavenLifecycleParticipant.groovy:220)
	at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.loadAllDiscoveredTiles(TilesMavenLifecycleParticipant.groovy:777)
	at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.orchestrateMerge(TilesMavenLifecycleParticipant.groovy:422)
	at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.afterProjectsRead(TilesMavenLifecycleParticipant.groovy:324)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.executeParticipants(ProjectRegistryManager.java:822)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.lambda$15(ProjectRegistryManager.java:791)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.executeBare(MavenExecutionContext.java:394)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:275)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:214)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.lambda$14(ProjectRegistryManager.java:790)
	at java.base/java.util.HashMap$Values.forEach(HashMap.java:1065)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.lambda$11(ProjectRegistryManager.java:788)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.executeBare(MavenExecutionContext.java:394)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:275)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:214)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.readMavenProjectFacades(ProjectRegistryManager.java:761)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:393)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:367)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:319)
	at org.eclipse.m2e.core.internal.builder.MavenBuilder$BuildMethod.getProjectFacade(MavenBuilder.java:146)
	at org.eclipse.m2e.core.internal.builder.MavenBuilder$BuildMethod.lambda$0(MavenBuilder.java:84)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.executeBare(MavenExecutionContext.java:394)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:275)
	at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:214)
	at org.eclipse.m2e.core.internal.builder.MavenBuilder$BuildMethod.execute(MavenBuilder.java:83)
	at org.eclipse.m2e.core.internal.builder.MavenBuilder.build(MavenBuilder.java:192)
	at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:1020)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:247)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:303)
	at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:392)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:395)
	at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:506)
	at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:454)
	at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:536)
	at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:196)
	at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:289)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)

@laeubi
Copy link
Member

laeubi commented Jun 26, 2023

I now was able to "fix" the problem at least now the project shows up with Java 17!

But I really want to recommend adding the example as a test so we can make sure it keeps working in the future without manual testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Improvements that can give a better usability or user experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants