| Subject: | Bug in Log::Log4perl::Appender |
| Date: | Sat, 25 Aug 2007 15:14:01 -0400 |
| To: | bug-Log-Log4perl [...] rt.cpan.org |
| From: | "Gabriel Berriz" <gberriz [...] gmail.com> |
Here's the patch:
--- lib/Log/Log4perl/Appender.pm.dist 2006-08-09 00:54:54.000000000 -0400
+++ lib/Log/Log4perl/Appender.pm 2007-08-25 14:06:09.899333850 -0400
@@ -43,10 +43,9 @@ sub new {
# anything in there that can't be class name.
die "'$appenderclass' not a valid class name " if $appenderclass =~
/[^:\w]/;
- # Check if the class/package is already in the namespace
because
+ # Check if the class/package is already available because
# something like Class::Prototyped injected it previously.
- no strict 'refs';
- if(!scalar(keys %{"$appenderclass\::"})) {
+ if(!UNIVERSAL::can($appenderclass, 'new')) {
# Not available yet, try to pull it in.
# see 'perldoc -f require' for why two evals
eval "require $appenderclass";
The original test in the if statement will fail spuriously if earlier some
other part of the code had accessed a global variable in the appender
class's package (e.g. with an expression such as "...if
$My::Appender::Class::DEBUG;"). Note that Perl honors such expressions even
if the corresponding package has not been loaded. Therefore, they should
not be penalized.
The proposed solution does not have this problem. It regards a package as
missing only if its constructor is not available. After applying this patch
all the distribution's tests succeed; at least all those that are run when
installing under Linux. I have not tested the patch under any other OS, but
I have little reason to expect that it would not work on any OS on which the
original version works.
BTW, there's a more fundamental bug here (which the patch above does not
address): implicit in this code is the incorrect assumption that a class
name corresponds to the name of the file, when in fact no such constraint
exists: Perl has no problem if one chooses to define class Foo in a file
called Bar.pm. The code above (with or without the patch) will fail in such
situations. Since I don't see why this code should penalize what perl
itself does not, IMHO, I think this code should treat all classes equally.
It's less confusing this way. Therefore, I would get rid of the attempt to
load missing classes altogether; that should be the calling code's
responsibility. Just my 2ยข.
FWIW, I include the uuencoded version of the patch below.
Best regards,
Gabriel Berriz
If the uuencoded version of the patch is needed, save everything between
(but excluding) the ==BEGIN== and the ==END== lines to a file (say patch.uue)
and restore with
perl -ne 'print unpack("u*",$_)' patch.uue > patch
== BEGIN ==
M+2TM(&QI8B],;V<O3&]G-'!E<FPO07!P96YD97(N<&TN9&ES=`DR,#`V+3`X
=+3`Y(#`P.C4T.C4T+C`P,#`P,#`P,"`M,#0P,`H`
M*RLK(&QI8B],;V<O3&]G-'!E<FPO07!P96YD97(N<&T),C`P-RTP."TR-2`Q
8-#HP-CHP.2XX.3DS,S,X-3`@+3`T,#`*
=0$`@+30S+#$P("LT,RPY($!`('-U8B!N97<@>PH`
M("`@("`@("`@("`@(R!A;GET:&EN9R!I;B!T:&5R92!T:&%T(&-A;B=T(&)E
-(&-L87-S(&YA;64N"@``
M("`@("`@("`@9&EE("(G)&%P<&5N9&5R8VQA<W,G(&YO="!A('9A;&ED(&-L
J87-S(&YA;64@(B!I9B`D87!P96YD97)C;&%S<R`]?B`O6UXZ7'==+SL*
"(`H`
M+2`@("`@("`@("`@(",@0VAE8VL@:68@=&AE(&-L87-S+W!A8VMA9V4@:7,@
A86QR96%D>2!I;B!T:&4@;F%M97-P86-E(&)E8V%U<V4*
M*R`@("`@("`@("`@(",@0VAE8VL@:68@=&AE(&-L87-S+W!A8VMA9V4@:7,@
:86QR96%D>2!A=F%I;&%B;&4@8F5C875S90H`
M("`@("`@("`@("`@(",@<V]M971H:6YG(&QI:V4@0VQA<W,Z.E!R;W1O='EP
;960@:6YJ96-T960@:70@<')E=FEO=7-L>2X*
;+2`@("`@("`@;F\@<W1R:6-T("=R969S)SL*
M+2`@("`@("`@:68H(7-C86QA<BAK97ES("5[(B1A<'!E;F1E<F-L87-S7#HZ
'(GTI*2!["@``
M*R`@("`@("`@:68H(55.259%4E-!3#HZ8V%N*"1A<'!E;F1E<F-L87-S+"`G
);F5W)RDI('L*
M("`@("`@("`@("`@(",@3F]T(&%V86EL86)L92!Y970L('1R>2!T;R!P=6QL
((&ET(&EN+@H`
M("`@("`@("`@("`@(",@<V5E("=P97)L9&]C("UF(')E<75I<F4G(&9O<B!W
-:'D@='=O(&5V86QS"@``
L("`@("`@("`@("`@(&5V86P@(G)E<75I<F4@)&%P<&5N9&5R8VQA<W,B.PH`
==END==