Sorry, I shouldn't send email before fully waking up. If this is a change that is specifically required by reporting and the site plugin then I feel it would make more sense to isolate the required POM manipulation to a participant in the site plugin rather than changing core logic. We've done a lot of work to get reporting out of the core and we should work toward getting it fully out, not adding more logic to accommodate it.
On May 21, 2013, at 9:13 AM, Jason van Zyl <[email protected]> wrote: > Is there no other way to achieve this. We're going backward again letting > reporting creep back into the core which is not good. > > Reporting should be in the core at all. > > Possibly a participant in the site plugin that manipulates the model > according to its needs? If this is a change in the because it's a requirement > of the site plugin I feel this is not the right change to make. > > On May 20, 2013, at 6:17 PM, [email protected] wrote: > >> Updated Branches: >> refs/heads/master ed1501ecb -> 4db66fd06 >> >> >> [MNG-5477] inject pluginManagement and build plugins versions into >> report plugins to avoid validation warnings >> >> Project: http://git-wip-us.apache.org/repos/asf/maven/repo >> Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/4db66fd0 >> Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/4db66fd0 >> Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/4db66fd0 >> >> Branch: refs/heads/master >> Commit: 4db66fd06c4be19b85ee2398cbfbb2e5a6bef4e2 >> Parents: ed1501e >> Author: Hervé Boutemy <[email protected]> >> Authored: Tue May 21 00:17:25 2013 +0200 >> Committer: Hervé Boutemy <[email protected]> >> Committed: Tue May 21 00:17:25 2013 +0200 >> >> ---------------------------------------------------------------------- >> .../DefaultPluginManagementInjector.java | 37 +++++++++++ >> .../model/building/SimpleProblemCollector.java | 17 +++++ >> .../validation/DefaultModelValidatorTest.java | 50 +++++++++++++-- >> 3 files changed, 97 insertions(+), 7 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/maven/blob/4db66fd0/maven-model-builder/src/main/java/org/apache/maven/model/management/DefaultPluginManagementInjector.java >> ---------------------------------------------------------------------- >> diff --git >> a/maven-model-builder/src/main/java/org/apache/maven/model/management/DefaultPluginManagementInjector.java >> >> b/maven-model-builder/src/main/java/org/apache/maven/model/management/DefaultPluginManagementInjector.java >> index ba9f060..abd8d94 100644 >> --- >> a/maven-model-builder/src/main/java/org/apache/maven/model/management/DefaultPluginManagementInjector.java >> +++ >> b/maven-model-builder/src/main/java/org/apache/maven/model/management/DefaultPluginManagementInjector.java >> @@ -31,6 +31,8 @@ import org.apache.maven.model.Plugin; >> import org.apache.maven.model.PluginContainer; >> import org.apache.maven.model.PluginExecution; >> import org.apache.maven.model.PluginManagement; >> +import org.apache.maven.model.ReportPlugin; >> +import org.apache.maven.model.Reporting; >> import org.apache.maven.model.building.ModelBuildingRequest; >> import org.apache.maven.model.building.ModelProblemCollector; >> import org.apache.maven.model.merge.MavenModelMerger; >> @@ -67,6 +69,12 @@ public class DefaultPluginManagementInjector >> { >> mergePluginContainer_Plugins( build, pluginManagement ); >> } >> + >> + mergeReporting_Plugins( model.getReporting(), build ); >> + if ( pluginManagement != null ) >> + { >> + mergeReporting_Plugins( model.getReporting(), >> pluginManagement ); >> + } >> } >> } >> >> @@ -132,6 +140,35 @@ public class DefaultPluginManagementInjector >> } >> } >> >> + /** >> + * merge plugin version to reporting if report plugin version not >> set >> + */ >> + private void mergeReporting_Plugins( Reporting target, >> PluginContainer source ) >> + { >> + List<Plugin> src = source.getPlugins(); >> + if ( !src.isEmpty() ) >> + { >> + List<ReportPlugin> tgt = target.getPlugins(); >> + >> + Map<Object, Plugin> managedPlugins = new >> LinkedHashMap<Object, Plugin>( src.size() * 2 ); >> + >> + for ( Plugin element : src ) >> + { >> + Object key = getPluginKey( element ); >> + managedPlugins.put( key, element ); >> + } >> + >> + for ( ReportPlugin element : tgt ) >> + { >> + Object key = getReportPluginKey( element ); >> + Plugin managedPlugin = managedPlugins.get( key ); >> + if ( managedPlugin != null && element.getVersion() == >> null ) >> + { >> + element.setVersion( managedPlugin.getVersion() ); >> + } >> + } >> + } >> + } >> } >> >> } >> >> http://git-wip-us.apache.org/repos/asf/maven/blob/4db66fd0/maven-model-builder/src/test/java/org/apache/maven/model/building/SimpleProblemCollector.java >> ---------------------------------------------------------------------- >> diff --git >> a/maven-model-builder/src/test/java/org/apache/maven/model/building/SimpleProblemCollector.java >> >> b/maven-model-builder/src/test/java/org/apache/maven/model/building/SimpleProblemCollector.java >> index d8112ac..06f1b1b 100644 >> --- >> a/maven-model-builder/src/test/java/org/apache/maven/model/building/SimpleProblemCollector.java >> +++ >> b/maven-model-builder/src/test/java/org/apache/maven/model/building/SimpleProblemCollector.java >> @@ -22,6 +22,8 @@ package org.apache.maven.model.building; >> import java.util.ArrayList; >> import java.util.List; >> >> +import org.apache.maven.model.Model; >> + >> >> /** >> * A simple model problem collector for testing the model building components. >> @@ -31,6 +33,7 @@ import java.util.List; >> public class SimpleProblemCollector >> implements ModelProblemCollector >> { >> + private Model model; >> >> private List<String> warnings = new ArrayList<String>(); >> >> @@ -38,6 +41,20 @@ public class SimpleProblemCollector >> >> private List<String> fatals = new ArrayList<String>(); >> >> + public SimpleProblemCollector() >> + { >> + } >> + >> + public SimpleProblemCollector( Model model ) >> + { >> + this.model = model; >> + } >> + >> + public Model getModel() >> + { >> + return model; >> + } >> + >> public List<String> getWarnings() >> { >> return warnings; >> >> http://git-wip-us.apache.org/repos/asf/maven/blob/4db66fd0/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java >> ---------------------------------------------------------------------- >> diff --git >> a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java >> >> b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java >> index 6fb5de7..c093804 100644 >> --- >> a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java >> +++ >> b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java >> @@ -27,6 +27,7 @@ import >> org.apache.maven.model.building.DefaultModelBuildingRequest; >> import org.apache.maven.model.building.ModelBuildingRequest; >> import org.apache.maven.model.building.SimpleProblemCollector; >> import org.apache.maven.model.io.xpp3.MavenXpp3Reader; >> +import org.apache.maven.model.management.PluginManagementInjector; >> import org.codehaus.plexus.PlexusTestCase; >> >> /** >> @@ -38,6 +39,8 @@ public class DefaultModelValidatorTest >> >> private ModelValidator validator; >> >> + private PluginManagementInjector pluginManagementInjector; >> + >> private Model read( String pom ) >> throws Exception >> { >> @@ -64,9 +67,22 @@ public class DefaultModelValidatorTest >> { >> ModelBuildingRequest request = new >> DefaultModelBuildingRequest().setValidationLevel( level ); >> >> - SimpleProblemCollector problems = new SimpleProblemCollector(); >> + SimpleProblemCollector problems = new SimpleProblemCollector( read( >> pom ) ); >> + >> + validator.validateEffectiveModel( problems.getModel(), request, >> problems ); >> + >> + return problems; >> + } >> >> - validator.validateEffectiveModel( read( pom ), request, problems ); >> + private SimpleProblemCollector validateEffective( Model model ) >> + throws Exception >> + { >> + ModelBuildingRequest request = >> + new DefaultModelBuildingRequest().setValidationLevel( >> ModelBuildingRequest.VALIDATION_LEVEL_STRICT ); >> + >> + SimpleProblemCollector problems = new SimpleProblemCollector( model >> ); >> + >> + validator.validateEffectiveModel( problems.getModel(), request, >> problems ); >> >> return problems; >> } >> @@ -76,9 +92,9 @@ public class DefaultModelValidatorTest >> { >> ModelBuildingRequest request = new >> DefaultModelBuildingRequest().setValidationLevel( level ); >> >> - SimpleProblemCollector problems = new SimpleProblemCollector(); >> + SimpleProblemCollector problems = new SimpleProblemCollector( read( >> pom ) ); >> >> - validator.validateRawModel( read( pom ), request, problems ); >> + validator.validateRawModel( problems.getModel(), request, problems >> ); >> >> return problems; >> } >> @@ -95,12 +111,14 @@ public class DefaultModelValidatorTest >> super.setUp(); >> >> validator = lookup( ModelValidator.class ); >> + pluginManagementInjector = lookup( PluginManagementInjector.class ); >> } >> >> @Override >> protected void tearDown() >> throws Exception >> { >> + this.pluginManagementInjector = null; >> this.validator = null; >> >> super.tearDown(); >> @@ -620,8 +638,26 @@ public class DefaultModelValidatorTest >> >> assertViolations( result, 0, 0, 3 ); >> >> - assertContains( result.getWarnings().get( 0 ), >> "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-noversion-plugin is missing." ); >> - assertContains( result.getWarnings().get( 1 ), >> "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-from-plugins-plugin is missing." ); >> - assertContains( result.getWarnings().get( 2 ), >> "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-from-pluginManagement-plugin is missing." ); >> + assertContains( result.getWarnings().get( 0 ), >> + "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-noversion-plugin is missing." ); >> + assertContains( result.getWarnings().get( 1 ), >> + "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-from-plugins-plugin is missing." ); >> + assertContains( result.getWarnings().get( 2 ), >> + "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-from-pluginManagement-plugin is missing." ); >> + >> + // after pluginManagement injection >> + ModelBuildingRequest request = >> + new >> DefaultModelBuildingRequest().setValidationLevel( >> ModelBuildingRequest.VALIDATION_LEVEL_STRICT ); >> + >> + SimpleProblemCollector problems = new SimpleProblemCollector(); >> + >> + pluginManagementInjector.injectManagement( result.getModel(), >> request, problems ); >> + >> + result = validateEffective( result.getModel() ); >> + >> + assertViolations( result, 0, 0, 1 ); >> + >> + assertContains( result.getWarnings().get( 0 ), >> + "'reporting.plugins.plugin.version' for >> org.apache.maven.plugins:maven-noversion-plugin is missing." ); >> } >> } >> > > Thanks, > > Jason > > ---------------------------------------------------------- > Jason van Zyl > Founder & CTO, Sonatype > Founder, Apache Maven > http://twitter.com/jvanzyl > --------------------------------------------------------- > > Selfish deeds are the shortest path to self destruction. > > -- The Seven Samuari, Akira Kurosawa > > > > > Thanks, Jason ---------------------------------------------------------- Jason van Zyl Founder & CTO, Sonatype Founder, Apache Maven http://twitter.com/jvanzyl --------------------------------------------------------- In short, man creates for himself a new religion of a rational and technical order to justify his work and to be justified in it. -- Jacques Ellul, The Technological Society
