Changes made to project resolved artifacts are overriden when a plugin uses @requiresDependencyResolution

a simple test project that contains a log produced after patching maven and cloved plugin with the hack .

Notice how the expected-mvn.log contains:

[INFO] Building war: /tmp/clover-war-hello-world-trunk/webapp/target/clover/sayHello-1.0-clover.war
[DEBUG] adding directory META-INF/
[...]
[DEBUG] adding entry WEB-INF/lib/a-1.0-clover.jar
[DEBUG] adding entry WEB-INF/lib/clover-2.1.0.jar
[DEBUG] adding entry WEB-INF/lib/b-1.0-clover.jar

while maven will today include b-1.0-clover.jar and a-1.0.jar

The patch I've applied to maven before modifying the clover plugin

I forgot to say that the patch I've made above applies to 2.0.6. I expect the same patch to work more or less on 2.0.9. Also the patch isn't cleaned up as the real change should be on the MavenProject class. Will attach a cleaned up patch. Doesn't really matter as the solution isn't going to be accepted as is

We've tested the maven patch + pacthed clover on a large scale project (30 modules) and all WAR/EARs ended up generated with full clovered dependencies. Now I just need some input from the maven developers.

For into the clover issue is registered here:http://developer.atlassian.com/jira/browse/CLMVN-6

I'd prefer to see this as preventing re-resolution of project dependencies, unless the scope changes upward (compile to test, NOT test to compile)...then, you could impose a rule saying that any pre-existing dependency artifacts in the project are preferred over those new ones just resolved...that way, you'd preserve any swizzled artifacts for the duration of your forked execution.

This does get into some interesting side effects for plugins that don't use @requiresDependencyResolution, probably, so it's probably not appropriate for 2.0.x (who knows what sort of regressions this sort of change could introduce).

I'd prefer to do a full audit of the mutability of MavenProject for 2.1 from a plugin's point of view, to make sure that modifications like this are preserved for successive plugin-executions.

#maven
<lacostej> yo ! Is there anything I can do in exchange for some comments regarding MNG-3595 ? at least to know if this problem is something that is worth addressing. Thanks
<Brian> lacostej, i don't think we should make that change in 2.0.x, but perhaps 2.1
<Brian> it's a little scary to make changes in this area anymore because it will inevitably break someone's build
<lacostej> Brian: OK. I am willing to provide a cleaner patch for 2.1, I just need some comments on what approach to follow.
<lacostej> the current patch cannot build any existing build as it provides a new hook for adding the functionality. But I don't like changing MavenProject
<Brian> jdcasey, wdyt?
<jdcasey> looking
<jdcasey> lacostej: I'm not sure I get it, the clover plugin would register a post-processor for dependencies? or, would it attach a post-processor to the project for use ahead of successive plugin executions?
<jdcasey> lacostej, Brian: why not simply remember when/what scope we've resolved to for a given project, and avoid re-resolving? then swizzled artifacts would remain intact for the forked project instance
<Brian> jdcasey, that would be the preferred technique performance wise also, but i'm afraid of consequences in 2.0.x
<lacostej> jdcasey: the way I solved the problem was just the quickest solution I found. The problem is that successive plugin executions affect the list of transitive dependencies, because of the requireDependencyResolution tag. The idea was then to post process this dependency resolution operation, if it is to happen again. Another solution would be to prevent from the re-resolution to happen again,...
<lacostej> ...but I wasn't sure if it is possible to achieve this without locking maven into one corner. I wasn't sure why we were not already re-resolving for each plugin that required it
<jdcasey> Brian: I understand, but this should be relatively tame
<jdcasey> lacostej: I think as long as we never prune the list of transitive artifacts (i.e. going from test scope to compile scope only) in the project instance, we should be able to avoid re-resolving pretty safely
<Brian> there's a lot of sketchy stuff in there, like you may still get artifacts even if you didn't request resolution, depending on what plugin(s) ran before
<jdcasey> IIRC, there is another long-standing issue with re-resolving the artifacts...jesse filed it, but I'd have to dig around to find it again
<lacostej> jdcasey: what happens if we start adding a new scope ?
<jdcasey> Brian: true, we'd almost need a masked project instance for things that didn't request dependency resolution
<Brian> yeah which is why this scares me in 2.o
<jdcasey> yeah
<jdcasey> Brian: you don't think we could hem it in with a bunch of tests and get the possible pathways covered?
<jdcasey> I mean, it'd be a lot of work, and this is sort of an edge case (if you don't mind my saying so), but still...
<jdcasey> 2.1 is definitely safest
<Brian> i'm risk averse after the mess we went through with .9
<Brian> we're trying to reduce regressions not make more
<jdcasey> it's probably worth mentioning in connection that we ought to do a full audit of the mutability of the MavenProject instance, to make sure things are intuitive across the board...fwiw
<jdcasey> heh
<lacostej> jdcasey: that's a good think. In fact what I was missing when developing is a contract between the plugins and maven core
<jdcasey> Brian: that pretty much means just shoring up the behavior and adopting a sunset clause for 2.0.x asap, then?
<jdcasey>
<lacostej> think->thing
<jdcasey> lacostej: yeah, the formal contract thing keeps coming up
<lacostej> e.g. what should a plugin expect after using setArtifacts()
<jdcasey> I started looking at that awhile back, something outside the core that was fluent in both the 2.0.x core and the 2.1 core, but ran out of time
<jdcasey> sure
<Brian> jdcasey, that's essentially what i had in mind, continue to fix bugs and regressions only
<jdcasey> that's cool
<jdcasey> Brian: maybe it's worth talking about a less scary 2.1, then? something that will do a new feature-set in between the current plan and the 2.0.x features?
<jdcasey> I mean, we have enough code now to talk about killing bugs and doing a release

• jdcasey shrugs
  <Brian> yes but 2.1 hardly works without the artifact stuff
  <Brian> i've been pushing to close the scope on 2.1 for a while now
  <lacostej> what's the rough idea behind 2.1 ? E.g. do you plan on enforcing maximum compatibility or can new features be favored over compatiblity ?
  <jdcasey> Brian: there isn't that much change in the way it uses artifact...we could probably backport it for a quick release, then move forward to the new artifact stuff in 2.2
  <jdcasey> lacostej: addressing some of the design flaws in 2.0.x, like extension loading for instance
  <jdcasey> and plugin-versioning/-configuration differences
  <lacostej> ok.
  <lacostej> jdcasey: back to the issue, the reason I didn't try to lazy resolve the dependencies (and opted for a post-processing instead), is that I wasn't sure what the contract between the plugin and MavenProject would end up if one plugin uses setArtifacts() and at the same time, the plugin manager only resolves once. That could create issues for plugins that work later on in the lifecycle. That's...
  <lacostej> ...why I wondered if there should be some guidelines on how operations that for example use setArtifacts() should be implemented in order to avoid plugin conflicts
  <lacostej> Anyway. thanks for the comments. I will copy paste part of this chatlog in the issue. I will keep using my patched maven for now (it's only going to be used on our build server anyway) and see if I can get try the resolution caching instead.
  <jdcasey> lacostej: sure, I understand. In any case, Brian has an excellent point in that this could be a major source of regressions unless we managed it perfectly and got very lucky...I guess I'd prefer to put it off until 2.1 as well...
  <lacostej> jdcasey: I am still running 2.0.6 on one project due to regressions
  <jdcasey> lacostej: I'm not sure I'm against the postProcessor idea, but it does seem a little hackish to continue re-resolving and just put in a longer resolution process, rather than figuring out how to limit re-resolution

Jerome,
Thanks for the excellent analysis and diagnosis of this issue.

Would it be possible to add a parameter to @requiresDependencyResolution, that determines what happens to transitive dependencies in a forked lifecycle? This could default to the current behavior, thereby preserving backwards compatibility yet allow plugins to define the strategy they require.

Or, are there any other work-arounds the maven-clover2-plugin can do to prevent this?

Cheers,
Nick

Bump - has there been any decision as to the best way to move forward? This issue is rather limiting to Clover and any other plugins which create classified artifacts.
What about applying Jerome's patch to Maven, and seeing what breaks? I am guessing not a lot would, since it is an extra plugin point, and is backward compatible with previous functionality.