You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Benjamin Bentmann <be...@udo.edu> on 2009/03/17 11:10:05 UTC
Re: svn commit: r755086 - in /maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin:
DefaultPluginManager.java MavenPluginValidator.java
Hi Brett,
> Author: brett
> Date: Tue Mar 17 02:28:26 2009
> New Revision: 755086
>
> URL: http://svn.apache.org/viewvc?rev=755086&view=rev
> Log:
> [MNG-4091] Validate the plugin descriptors and improve error reporting when mismatched with artifact
>
> Modified: maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java
> URL: http://svn.apache.org/viewvc/maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java?rev=755086&r1=755085&r2=755086&view=diff
> ==============================================================================
> --- maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java (original)
> +++ maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java Tue Mar 17 02:28:26 2009
> @@ -296,10 +298,26 @@
>
> try
> {
> - child = container.createChildContainer( PluginUtils.constructVersionedKey( plugin ).intern(),
> + MavenPluginValidator validator = new MavenPluginValidator( pluginArtifact );
> +
> + String key = PluginUtils.constructVersionedKey( plugin ).intern();
> + child = container.createChildContainer( key,
> Collections.singletonList( pluginArtifact.getFile() ),
> Collections.EMPTY_MAP,
> - Collections.singletonList( pluginCollector ) );
> + Arrays.asList( new ComponentDiscoveryListener[] { validator, pluginCollector } ) );
> +
> + if ( validator.hasErrors() )
> + {
> + String msg = "Plugin '" + key + "' has an invalid descriptor:";
> + int count = 1;
> + for ( Iterator i = validator.getErrors().iterator(); i.hasNext(); )
> + {
> + msg += "\n" + count + ") " + i.next();
> + count++;
> + }
> + throw new PluginManagerException( msg );
> + }
> +
For the sake of reduced heap consumption, how about unregistering the
validator right after createChildContainer() returns such that the
validator can be garbage collected?
Imagine a plugin A which depends on another plugin artifact B, like
people would do when extending mojos (see also MNG-3217). When
ensurePluginContainerIsComplete() is called, the plugin descriptor from
artifact B will be discovered and will cause the validator to create
error strings. Furtheremore, those error strings will be created n
times, where n is the total number of plugin dependencies, each
triggering an invocation of MavenPluginValidator.componentDiscovered().
Having said this, we might as well consider unregistering the plugin
collector from the plugin container in a future Maven version. We're
only interested in the descriptor from the plugin main artifact so
spontaneously I don't see a reason to still have the collector around
when the plugin container is populated with the plugin dependencies.
Benjamin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r755086 - in /maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin: DefaultPluginManager.java MavenPluginValidator.java
Posted by Brett Porter <br...@apache.org>.
I've made both these changes - thanks Benjamin!
- Brett
On 17/03/2009, at 9:20 PM, Brett Porter wrote:
>
> On 17/03/2009, at 9:10 PM, Benjamin Bentmann wrote:
>
>>
>> For the sake of reduced heap consumption, how about unregistering
>> the validator right after createChildContainer() returns such that
>> the validator can be garbage collected?
>>
>> Imagine a plugin A which depends on another plugin artifact B, like
>> people would do when extending mojos (see also MNG-3217). When
>> ensurePluginContainerIsComplete() is called, the plugin descriptor
>> from artifact B will be discovered and will cause the validator to
>> create error strings. Furtheremore, those error strings will be
>> created n times, where n is the total number of plugin
>> dependencies, each triggering an invocation of
>> MavenPluginValidator.componentDiscovered().
>
> AH! Thanks, I'd planned to test that and it slipped my mind when I
> finished. I can fix this up.
>
>>
>> Having said this, we might as well consider unregistering the
>> plugin collector from the plugin container in a future Maven
>> version. We're only interested in the descriptor from the plugin
>> main artifact so spontaneously I don't see a reason to still have
>> the collector around when the plugin container is populated with
>> the plugin dependencies.
>
> I agree - I'll do this separately though in case it affects that use
> case you mentioned above.
>
> Thanks!
> Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r755086 - in /maven/components/branches/maven-2.1.0-RC/maven-core/src/main/java/org/apache/maven/plugin: DefaultPluginManager.java MavenPluginValidator.java
Posted by Brett Porter <br...@apache.org>.
On 17/03/2009, at 9:10 PM, Benjamin Bentmann wrote:
>
> For the sake of reduced heap consumption, how about unregistering
> the validator right after createChildContainer() returns such that
> the validator can be garbage collected?
>
> Imagine a plugin A which depends on another plugin artifact B, like
> people would do when extending mojos (see also MNG-3217). When
> ensurePluginContainerIsComplete() is called, the plugin descriptor
> from artifact B will be discovered and will cause the validator to
> create error strings. Furtheremore, those error strings will be
> created n times, where n is the total number of plugin dependencies,
> each triggering an invocation of
> MavenPluginValidator.componentDiscovered().
AH! Thanks, I'd planned to test that and it slipped my mind when I
finished. I can fix this up.
>
> Having said this, we might as well consider unregistering the plugin
> collector from the plugin container in a future Maven version. We're
> only interested in the descriptor from the plugin main artifact so
> spontaneously I don't see a reason to still have the collector
> around when the plugin container is populated with the plugin
> dependencies.
I agree - I'll do this separately though in case it affects that use
case you mentioned above.
Thanks!
Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org