maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Igor Fedorenko <i...@ifedorenko.com>
Subject Re: MavenPluginManager#setupPluginRealm "imports" parameter
Date Fri, 20 Feb 2015 05:24:54 GMT
I believe all maven class realms lookup classes and resources in the
following order

1. realm imports
2. self
3. parent

This means you should be okay unless reporting plugins and site plugin
need to agree on some common classes but the classes are present in both
plugin dependencies. It also means that ClassLoader#getResources (i.e.
plural) will return resources from both reporting and site plugin
dependencies, so things like ServiceLoader mey be confused by some
unexpected resources.

--
Regards,
Igor

On 2015-02-19 23:14, Hervé BOUTEMY wrote:
> Hi,
>
> Sorry for the delay: needed to take time to really think about the impact
>
> effective real world tests confirm that "imports" parameter is not taken into
> account: lately, we found the first plugin I know of that was bitten by this
> fact. It's Apache Rat Maven Plugin, that is bitten by Xerces injected by Maven
> Site Plugin even if these classes are not in the "import" parameter. See
> RAT-158 [1]
>
> The import parameter is used to export Doxia from maven-site-plugin to the
> reporting plugins, and only Doxia, not the full maven-site-plugin transitive
> dependencies [1]
>
> I don't think that any report plugin really /requires/ a dependency to be
> injected by maven-site-plugin, since a report plugin can be used standalone
> too, without the maven-site-plugin then its classes.
>
> On the other hand, a report mojo can be confused if its own dependency version
> is overridden by maven-site-plugin: do you know what's the order of
> dependencies for report plugin?
>
> If the order makes maven-site-plugin's dependencies override report plugins
> ones, I think the import parameter should be taken into acccount: of course,
> this should be tested to be sure there is no unexpected side effect.
>
> if the order makes report plugin hav its wn version override maven-site-
> plugin, I suppose it's safer to stay as is
>
> What is clear is that this bug wasn't found by anybody before, so it doesn't
> have too much consequences in real life: great code review :)
>
> Regards,
>
> Hervé
>
> [1] http://maven.apache.org/plugins/maven-site-plugin/dependencies.html
>
> [1] https://issues.apache.org/jira/browse/RAT-158
>
> Le mardi 3 février 2015 23:31:24 Igor Fedorenko a écrit :
>> MavenPluginManager#setupPluginRealm "imports" parameter is not used in
>> any meaningful way. No matter what packages are passed in, the created
>> plugin realm will have access to all classes from the provided parent
>> classloader. I did some more digging and looks like this is how Maven
>> behaved for last 4+ years, so I assume all existing plugins are happy
>> with this.
>>
>> By "fix" I meant to change the code to honour parent imports, but I now
>> think it's too risky given how long the current behaviour was in place.
>> I plan to update javadoc to state the parameter is ignored and possible
>> cleanup the implementation.
>>
>> --
>> Regards,
>> Igor
>>
>> On 2015-02-03 22:04, Hervé BOUTEMY wrote:
>>> Le mardi 3 février 2015 16:42:47 Igor Fedorenko a écrit :
>>>> Does anyone have a usecase that demonstrates use of
>>>> MavenPluginManager#setupPluginRealm "imports" parameter? I've found
>>>> DefaultMavenReportExecutor from maven-reporting-exec, which provides
>>>> list of imported packages, but not sure how to use it from a project.
>>>
>>> what do you mean by "use it from a project"?
>>>
>>> maven-reporting-exec is used by maven-site-plugin to execute reports:
>>> maven- site-plugin is the only plugin I know of that launches plugins
>>> (here called "reports"), but this is the general use case = "plugins
>>> launching plugins"
>>>
>>> I really need it for m-site-p
>>>
>>>>    From what I can tell, this parameter is currently masked by
>>>>
>>>> DefaultMavenPluginManager implementation and plugins can use any classes
>>>> from the provided parent classloader. So I am not sure if we should fix
>>>> parent imports implementation or get rid of it completely.
>>>
>>> "fix"? what is the bug you're trying to fix?
>>> is this API causing issue for some evolution?
>>>
>>> Regards,
>>>
>>> Hervé
>>>
>>>> Thank you in advance.
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Mime
View raw message