Issue Details (XML | Word | Printable)

Key: MNG-2184
Type: Bug Bug
Status: Open Open
Priority: Blocker Blocker
Assignee: John Casey
Reporter: Vincent Massol
Votes: 21
Watchers: 19
Operations

If you were logged in you would be able to see more operations.
Maven 2

Possible problem with @aggregator and forked lifecycles

Created: 28/Mar/06 11:15 AM   Updated: 13/Dec/08 06:34 PM
Component/s: Design, Patterns & Best Practices, Plugins and Lifecycle
Affects Version/s: 2.0.3
Fix Version/s: 3.x

Time Tracking:
Not Specified

File Attachments: 1. Text File cargo.log (59 kB)

Environment: Revision 974 of Cargo (https://svn.codehaus.org/cargo/cargo/trunk/core/api/)
Issue Links:
Related
 

Complexity: Intermediate


 Description  « Hide
In the Clover plugin the report mojo is an aggregator (http://svn.apache.org/repos/asf/maven/plugins/trunk/maven-clover-plugin/src/main/java/org/apache/maven/plugin/clover/CloverReportMojo.java). It also spawns a forked lifecycle:
* @execute phase="test" lifecycle="clover"
* @aggregator

When I run this clover report on the Cargo API module, which contains children modules, all the modules are executed several times as shown in the attached logs. They should be executed only once. The @aggregator is supposed to execute only once and it executes several times.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Vincent Massol added a comment - 28/Mar/06 11:16 AM
In order to understand how the clover report works, here's a diagram: http://svn.apache.org/repos/asf/maven/plugins/trunk/maven-clover-plugin/src/site/resources/images/clover-report-architecture.jpg

Please note that this diagram was done before I modified the clover report mojo to be an aggregator.


Mike Perham added a comment - 28/Mar/06 01:34 PM
Could this be due to MNG-2054? Have you tried the new 2.0.3 release?

Vincent Massol added a comment - 28/Mar/06 02:27 PM
Hi Mike,

I was using 2.0.3-SNAPSHOT. Just tried with 2.0.3 final and I'm getting the same result...

Thanks


Vincent Massol added a comment - 30/Mar/06 10:37 AM
Moving to blocker as the clover plugin cannot be released before this is fixed

Brett Porter added a comment - 30/Mar/06 05:00 PM
This only happens as part of the site, because it doesn't support using @aggregator for reports. I think this is a dupe - I'll search.

This is a feature request, so can't be done for 2.0.4. I'd suggest, for this release, that you split into clover:clover and clover:report, with the first being @aggregator and not implementing MavenReport, and the latter not aggregating and implementing MavenReport.


Brett Porter added a comment - 30/Mar/06 05:05 PM
ok, I think I was mistaken. I was proabbly thinking of when reports couldn't fork lifecycles. So this indeed seems to be a bug, but a very risky one to change in the short term. Can yo use the above workaround?

The other thing you can do is to have an isAggregating flag in the clover plugin, and set that in the forked lifecycle.


John Casey added a comment - 20/Jun/06 10:20 PM
I think it may be simpler to try looking at this in the context of any new lifecycle features we might plan for 2.1. Aggregation is a bit of a problem across the board right now, with clover being one of the more extreme use cases.

John Casey added a comment - 07/Feb/08 07:03 PM
See revId: 619720 on trunk...does this do enough to prevent bound-aggregator weirdness?

John Casey added a comment - 07/Feb/08 07:07 PM
619720 improves the logic of skipping vs. warning of deprecation (deprecated to bind aggregator mojos to the lifecycle now), to prevent printing a warning message when the mojo will be skipped.

John Casey added a comment - 07/Feb/08 07:23 PM
thinking about this more, if an aggregator mojo is bound simultaneously to two module poms in the same reactor, where should it execute? If it's bound to a pom at all, then merely the root location of the reactor may change whether the aggregator runs or not, using the logic from revId: 619721.

I'm commenting that code out until I can think about this more, and come up with something. I really think the safest solution may be to prohibit aggregators in the lifecycle at all.


Brian Fox added a comment - 08/Feb/08 12:54 AM
We need to support aggregators in the lifecycle, i'm convinced of that, but they should only run at the root of the execution, just like its done from the cli.

John Casey added a comment - 08/Feb/08 09:11 AM
But what are the implications for that? If they're bound to the usual root pom of a project hierarchy, then they will always be included in the build process, because all modules should inherit from that top-level pom, and maven will get the opportunity to limit the mojo to one execution. However, if the aggregator is bound to one of the child modules, then it would run when the child is built in isolation (since it would be included in the build-plan of the root project), but NOT when the build is run from the top-level POM (which would be the root project, and the aggregator isn't bound to this POM but one of its modules.

So, that leaves us with some sort of session attribute that tracks the execution of aggregators, to limit their execution to once-per-build.

Now, what if two of the child modules specify a binding to the same aggregator mojo, with different configurations? In this case, it will all fall apart, since the binding project is not the root project, and both aggregator bindings really are meant to execute.

What do we do in those cases??

Why do we absolutely need aggregator mojos that are bound to the lifecycle? What are the real use cases, and is there a better solution that we could design? Personally, I'd much rather see the pre- and post-execution mojo thing that we talked about in the comments of:

http://docs.codehaus.org/display/MAVEN/Deterministic+Lifecycle+Planning

IF we could find a way to make their binding and execution behavior consistent regardless of how a collection of POMs is built (i.e. which ones are included should not modify the build of any one of them).


Brian Fox added a comment - 08/Feb/08 10:55 AM
The ounce:application goal is an aggregator that is intended to be bound to a pom. We need to collect the information about all child projects. I think that where ever in the tree an aggregator is defined, it should run once there. Meaning if it shows up in two children, well then it runs for each of those (once for each of those trees)

The aggregation part isn't really what is needed if there is another way to get the fully interpolated child projects....this is what is really needed.


John Casey added a comment - 08/Feb/08 10:59 AM
you're not talking about an aggregator mojo, just a regular one that uses ${reactorProjects}...and @inheritedByDefault false (or whatever that annotation is).