yes, i know we want to avoid site logic into Maven core: I therefore limited the inheritance to version, not configuration, for example this has to be done fast, because the warning happens when validating the model, before executing any plugin
Regards, Hervé Le mardi 21 mai 2013 09:20:28 Jason van Zyl a écrit : > 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-bu > >> ilder/src/main/java/org/apache/maven/model/management/DefaultPluginManage > >> mentInjector.java > >> ---------------------------------------------------------------------- > >> diff --git > >> a/maven-model-builder/src/main/java/org/apache/maven/model/management/De > >> faultPluginManagementInjector.java > >> b/maven-model-builder/src/main/java/org/apache/maven/model/management/De > >> faultPluginManagementInjector.java index ba9f060..abd8d94 100644 > >> --- > >> a/maven-model-builder/src/main/java/org/apache/maven/model/management/De > >> faultPluginManagementInjector.java +++ > >> b/maven-model-builder/src/main/java/org/apache/maven/model/management/De > >> faultPluginManagementInjector.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-bu > >> ilder/src/test/java/org/apache/maven/model/building/SimpleProblemCollecto > >> r.java > >> ---------------------------------------------------------------------- > >> diff --git > >> a/maven-model-builder/src/test/java/org/apache/maven/model/building/Simp > >> leProblemCollector.java > >> b/maven-model-builder/src/test/java/org/apache/maven/model/building/Simp > >> leProblemCollector.java index d8112ac..06f1b1b 100644 > >> --- > >> a/maven-model-builder/src/test/java/org/apache/maven/model/building/Simp > >> leProblemCollector.java +++ > >> b/maven-model-builder/src/test/java/org/apache/maven/model/building/Simp > >> leProblemCollector.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-bu > >> ilder/src/test/java/org/apache/maven/model/validation/DefaultModelValidat > >> orTest.java > >> ---------------------------------------------------------------------- > >> diff --git > >> a/maven-model-builder/src/test/java/org/apache/maven/model/validation/De > >> faultModelValidatorTest.java > >> b/maven-model-builder/src/test/java/org/apache/maven/model/validation/De > >> faultModelValidatorTest.java index 6fb5de7..c093804 100644 > >> --- > >> a/maven-model-builder/src/test/java/org/apache/maven/model/validation/De > >> faultModelValidatorTest.java +++ > >> b/maven-model-builder/src/test/java/org/apache/maven/model/validation/De > >> faultModelValidatorTest.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 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
