You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Benjamin Gandon <be...@gandon.org> on 2015/10/06 15:14:10 UTC
Minor issue in jar packaging of tomcat-juli-adapters, in extras
Hi there,
Working on my JULI-to-SLF4J bridge library (see <https://github.com/bgandon/juli-to-slf4>),
I have identified a small and invisible issue in the jar packaging of tomcat-juli-adapters in the extras.
Indeed, the LogFactoryImpl is shipped with the adapters, but it is not supposed to.
1. Because it is already shipped with the tomcat-juli jar.
2. Because the ${files.tomcat-extras-juli-adapters} in build.xml specifies an exclusion on it.
But the exclusion is ineffective because it lacks a star at the end.
The issue is invisible because of class loading delegation. Children class loaders accessing the adapters favor delegation to the System loader.
So the LogFactoryImpl from tomcat-juli (System classpath) always masks the one erroneously shipped with tomcat-juli-adapters (Catalina classpath).
I would be happy to submit a PR on github for this, I mean at <https://github.com/apache/tomcat80/pulls>,
but it just looks like it’s not the way you guys are working. :)
Do you need a BZ issue for this?
Or could someone just commit the fix for me please? I include the diff below.
Cheers,
/Benjamin
diff --git a/build.xml b/build.xml
index 4f69f33..492d248 100644
--- a/build.xml
+++ b/build.xml
@@ -484,7 +484,7 @@
<patternset id="files.tomcat-extras-juli-adapters">
<include name="org/apache/juli/logging/impl/**" />
<exclude name="org/apache/juli/logging/impl/WeakHashtable*" />
- <exclude name="org/apache/juli/logging/impl/LogFactoryImpl" />
+ <exclude name="org/apache/juli/logging/impl/LogFactoryImpl*" />
<!-- Javadoc and i18n exclusions -->
<exclude name="**/package.html" />
<exclude name="**/LocalStrings_*" />
Re: Minor issue in jar packaging of tomcat-juli-adapters, in extras
Posted by Konstantin Kolinko <kn...@gmail.com>.
2015-10-06 16:43 GMT+03:00 Mark Thomas <ma...@apache.org>:
> On 06/10/2015 14:14, Benjamin Gandon wrote:
>> Hi there,
>>
>> Working on my JULI-to-SLF4J bridge library (see <https://github.com/bgandon/juli-to-slf4>),
>> I have identified a small and invisible issue in the jar packaging of tomcat-juli-adapters in the extras.
>>
>> Indeed, the LogFactoryImpl is shipped with the adapters, but it is not supposed to.
>> 1. Because it is already shipped with the tomcat-juli jar.
>> 2. Because the ${files.tomcat-extras-juli-adapters} in build.xml specifies an exclusion on it.
>>
>> But the exclusion is ineffective because it lacks a star at the end.
>>
>> The issue is invisible because of class loading delegation. Children class loaders accessing the adapters favor delegation to the System loader.
>> So the LogFactoryImpl from tomcat-juli (System classpath) always masks the one erroneously shipped with tomcat-juli-adapters (Catalina classpath).
>>
>
>> Do you need a BZ issue for this?
>
> No need. If a patch isn't picked up fairly quickly (say within 24 hours)
> I'd recommend using a pull request or opening a Bugzilla issue since
> both those mechanisms reduce the chances of an issue being forgotten about.
>
>> Or could someone just commit the fix for me please? I include the diff below.
>
> Done. For trunk, 8.0.x and 7.0.x.
>
Fixed for Tomcat 6 as well (r1712571)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Minor issue in jar packaging of tomcat-juli-adapters, in extras
Posted by Benjamin Gandon <be...@gandon.org>.
Thanks for the (lightning fast) response Mark!
> Le 6 oct. 2015 à 15:43, Mark Thomas <ma...@apache.org> a écrit :
>
> On 06/10/2015 14:14, Benjamin Gandon wrote:
>> Hi there,
>>
>> Working on my JULI-to-SLF4J bridge library (see <https://github.com/bgandon/juli-to-slf4>),
>> I have identified a small and invisible issue in the jar packaging of tomcat-juli-adapters in the extras.
>>
>> Indeed, the LogFactoryImpl is shipped with the adapters, but it is not supposed to.
>> 1. Because it is already shipped with the tomcat-juli jar.
>> 2. Because the ${files.tomcat-extras-juli-adapters} in build.xml specifies an exclusion on it.
>>
>> But the exclusion is ineffective because it lacks a star at the end.
>>
>> The issue is invisible because of class loading delegation. Children class loaders accessing the adapters favor delegation to the System loader.
>> So the LogFactoryImpl from tomcat-juli (System classpath) always masks the one erroneously shipped with tomcat-juli-adapters (Catalina classpath).
>>
>> I would be happy to submit a PR on github for this, I mean at <https://github.com/apache/tomcat80/pulls>,
>> but it just looks like it’s not the way you guys are working. :)
>
> Pull requests work for us. It is just that not that many folks have used
> that route so far.
>
> (Thanks for the reminder that I need to review the current pull requests.)
>
>> Do you need a BZ issue for this?
>
> No need. If a patch isn't picked up fairly quickly (say within 24 hours)
> I'd recommend using a pull request or opening a Bugzilla issue since
> both those mechanisms reduce the chances of an issue being forgotten about.
>
>> Or could someone just commit the fix for me please? I include the diff below.
>
> Done. For trunk, 8.0.x and 7.0.x.
>
> Many thanks.
>
> Mark
>
>
>>
>> Cheers,
>> /Benjamin
>>
>>
>>
>>
>> diff --git a/build.xml b/build.xml
>> index 4f69f33..492d248 100644
>> --- a/build.xml
>> +++ b/build.xml
>> @@ -484,7 +484,7 @@
>> <patternset id="files.tomcat-extras-juli-adapters">
>> <include name="org/apache/juli/logging/impl/**" />
>> <exclude name="org/apache/juli/logging/impl/WeakHashtable*" />
>> - <exclude name="org/apache/juli/logging/impl/LogFactoryImpl" />
>> + <exclude name="org/apache/juli/logging/impl/LogFactoryImpl*" />
>> <!-- Javadoc and i18n exclusions -->
>> <exclude name="**/package.html" />
>> <exclude name="**/LocalStrings_*" />
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org <ma...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org <ma...@tomcat.apache.org>
Re: Minor issue in jar packaging of tomcat-juli-adapters, in extras
Posted by Mark Thomas <ma...@apache.org>.
On 06/10/2015 14:14, Benjamin Gandon wrote:
> Hi there,
>
> Working on my JULI-to-SLF4J bridge library (see <https://github.com/bgandon/juli-to-slf4>),
> I have identified a small and invisible issue in the jar packaging of tomcat-juli-adapters in the extras.
>
> Indeed, the LogFactoryImpl is shipped with the adapters, but it is not supposed to.
> 1. Because it is already shipped with the tomcat-juli jar.
> 2. Because the ${files.tomcat-extras-juli-adapters} in build.xml specifies an exclusion on it.
>
> But the exclusion is ineffective because it lacks a star at the end.
>
> The issue is invisible because of class loading delegation. Children class loaders accessing the adapters favor delegation to the System loader.
> So the LogFactoryImpl from tomcat-juli (System classpath) always masks the one erroneously shipped with tomcat-juli-adapters (Catalina classpath).
>
> I would be happy to submit a PR on github for this, I mean at <https://github.com/apache/tomcat80/pulls>,
> but it just looks like it’s not the way you guys are working. :)
Pull requests work for us. It is just that not that many folks have used
that route so far.
(Thanks for the reminder that I need to review the current pull requests.)
> Do you need a BZ issue for this?
No need. If a patch isn't picked up fairly quickly (say within 24 hours)
I'd recommend using a pull request or opening a Bugzilla issue since
both those mechanisms reduce the chances of an issue being forgotten about.
> Or could someone just commit the fix for me please? I include the diff below.
Done. For trunk, 8.0.x and 7.0.x.
Many thanks.
Mark
>
> Cheers,
> /Benjamin
>
>
>
>
> diff --git a/build.xml b/build.xml
> index 4f69f33..492d248 100644
> --- a/build.xml
> +++ b/build.xml
> @@ -484,7 +484,7 @@
> <patternset id="files.tomcat-extras-juli-adapters">
> <include name="org/apache/juli/logging/impl/**" />
> <exclude name="org/apache/juli/logging/impl/WeakHashtable*" />
> - <exclude name="org/apache/juli/logging/impl/LogFactoryImpl" />
> + <exclude name="org/apache/juli/logging/impl/LogFactoryImpl*" />
> <!-- Javadoc and i18n exclusions -->
> <exclude name="**/package.html" />
> <exclude name="**/LocalStrings_*" />
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org