| Summary: | Eclipse Clean... build uses incorrect lifecycle mappings | ||||||
|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Rob Cernich <rcernich> | ||||
| Component: | m2e | Assignee: | Project Inbox <m2e.core-inbox> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | igor, manderse, shr31223 | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
|
Description
Rob Cernich
m2e is expected to consider plugin executions bound to all standard project lifecycles (clean, default and site), so we need a sample project to understand why this does not work for you. Also note that m2e project configurator API allows any project configurator participate in workspace clean build, not only configurators associated with plugin executions bound to clean lifecycle. This allows project configurators to selectively clean after themselves (i.e. code generation configurator removes just the code it generated and nothing else). Hey Igor, If you simply trace through the code for MavenBuilder.clean() into AbstractCustomizableLifecycleMapping.getBuildParticipants(), you will see that the "deploy" execution plan is always used; as opposed to "clean," which is what one might expect for a "clean" build. This effectively prevents any build participants bound to clean phases from executing. This may be as simply as me not fully understanding the way the build participants are meant to be used. They contain both build() and clean() methods, so it makes sense that an extension could clean itself up. But, from the Maven side of things, I think it makes more sense for a "clean" build to be similar (if not exactly so) to a "mvn clean" execution. You can find a project which implements a build participant for maven-clean-plugin here: https://github.com/rcernich/fools-errands/tree/master/jboss-tools/eclipse/plugins/org.jboss.tools.m2e.extras. I created this extension so additional directories, outside "target," would be cleaned during clean builds. Best regards, Rob We welcome quality patches ;-) (In reply to comment #3) > We welcome quality patches ;-) I think it's bigger than me. ;-) I looked into this briefly and I think it would require a change to ILifecycleMapping, e.g. adding a getCleanParticipants() method. I started stubbing this out: creating a getBuildParticpantsForLifecycle(), which was essentially getBuildParticipants() with an added parameter, "lifecycle." The original getBuildParticipants() simply delegates to the new method passing ProjectRegistryManager.LIFECYCLE_DEFAULT. However, once I saw that this required exposing the new method through ILifecycleMapping, I bailed. That said, the changes are simple and easy, barring the side-effects to the published API. Why not simply consider all configurators, regardless of their corresponding lifecycle? (In reply to comment #5) > Why not simply consider all configurators, regardless of their corresponding > lifecycle? If that works, I don't see why not. The assumption being that clean() is invoked during a clean() build anyway, so it shouldn't affect existing extensions, so long as their written correctly. I'm assuming you're suggesting changing ProjectRegistryManager.LIFECYCLE_DEFAULT from "deploy" to "clean deploy" or creating a new constant that would be used in getBuildParticipants(). If that's the case, you'll probably also need modify the way the execution plans are looked up. Currently, ProjectRegistryManagement.calculateExecutionPlan() only takes a single "lifecycle" parameter, so it would need to be changed in some way: either converting "clean deploy" to { "clean", "deploy"} or accepting an array. Created attachment 203298 [details]
Proposed Change
Changed the lifecycle used to lookup build participants from "deploy" to "clean deploy".
We will need automated test that makes sure this change actually solves the problem and keeps the problem from reappearing in the future. I realize this is not obvious, but m2e tests currently live at github https://github.com/sonatype/m2e-core-tests. Upon further review, I think the patch is appropriate, but I think the way I implemented the build participant for the clean plugin in the referenced project above is incorrect. The documentation for IncrementalProjectBuilder.clean() states that this method should be used for clearing out markers and any saved state used by the builder. With that in mind, the work done by "clean" should be done in the build() method and should only be done during FULL_BUILDs. So, the lifecycle used to select the build participants (clean deploy) is correct, but build participants for goals contributed for the clean phase should only execute during FULL_BUILDs, which shouldn't be a problem since that's the default behavior for MojoExecutionBuildParticipant. What do you thinK? No. Removing derived resources is the job of IncrementalProjectBuilder#clean(IProgressMonitor) as it's clearly stated in javadoc [1] "builders override this method to delete all derived resources created by previous builds". Neither full nor incremental builds are expected to remove old derived resources, only generate all/some new ones. I am still not sure what is the problem that you are trying to solve, so please provide an automated test that fails without your patch so we can have more specific discussion about possible solutions. [1] http://git.eclipse.org/c/platform/eclipse.platform.resources.git/tree/bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/IncrementalProjectBuilder.java?id=R3_7#n146 (In reply to comment #10) > I am still not sure what is the problem that you are trying to solve, so please > provide an automated test that fails without your patch so we can have more > specific discussion about possible solutions. > I have the maven clean plugin configured to clean directories outside target. These additional directories are not cleaned when I select Project=>Clean... I've created a test case which can found here: https://github.com/rcernich/eclipse-m2e-core-tests/tree/BZ-357531 Unfortunately, the test fails because there is no build delegate for the maven clean plugin. I don't mind adding a delegate, but I don't know where it should live. Anyway, the test case highlights an issue with both the clean phase being omitted from the lifecycle and the lack of any support for the maven clean plugin. Best, Rob This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |