On Thu, 22 Nov 2007, Merijn Broeren wrote: > There is an existing RT bug for the issue of module_available being > broken already: > > http://rt.cpan.org/Public/Bug/Display.html?id=24884 > > which is 10 months older then the release done yesterday. Since there > have been tickets closed since then I am assuming you guys are aware of > this issue already.
Yeah, that's an old issue that dates back to Log4perl 0.42: There we figured out that if someone wrote $SIG{__DIE__} = sub { print "die handler!\n"; }; and used Log4perl and some module like Time::HiRes is missing, which is acceptable, the handler gets triggered because eval { require } causes an exception. Confusion ensued. Now as it has been pointed out, it's actually the user's fault. A correct die-Handler checks if it was called because of an eval and ignores the exception. Now, I've always thought that this is a bug in perl, nobody writes die die handlers 'correctly'. To comply with users who get confused with this confusing behaviour, I put this module_available() hack in place, which, admittedly, sucks. What to do? Going back to eval { require } would break every user with sloppy die handlers. We could add a load time option to establish the correct behavior: use Log::Log4perl qw(:module_check_via_eval); Other ideas? -- Mike Mike Schilli [EMAIL PROTECTED] > > I just wanted to reiterate that this code really is broken, both in > spirit and in implementation. Please reconsider having this broken code > in such a useful package. The patch in RT is sufficient to restore > functionality, but I would personally take out all module_available > calls everywhere from Log4perl and replace it with a simple eval {require }. > > To illustrate implementation brokenness, see line 33 of Util.pm: > > ref $dir !~ /^(GLOB|SCALAR|HASH|REF|LVALUE)$/) { > > which tries to determine the reference status on the result of a regular > expression. I assume the author wanted to write this : > > ref($dir) !~ /^(GLOB|SCALAR|HASH|REF|LVALUE)$/) { > > but forgot about operator precedence. So this test will *never* be true. > Which is good, as the code on line 34 inside the if branch is bad as well. > > return 1 if $dir->INC(); > > as the INC method is supposed to return the open file handle of the file > requested. Since you do not request a file, the method is under no > obligation to actually return a true value. > > But it would be pointless to try fixing this implementation since > whatever the result of the module_available function is, it just leads > to an eval { require } afterwards anyway, so you are doubling the > effort. It would be very nice if you would consider to modify Log4perl > to not use module_available. Or accept the RT patch. The current code > does not work with @INC objects. > > Regards, > -- > Merijn Broeren | We take risks, we know we take them. Therefore, when things > | come out against us, we have no cause for complaint. > | - Scott, last journal entry, march 1912 > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2005. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > log4perl-devel mailing list > log4perl-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/log4perl-devel > ------------------------------------------------------------------------- SF.Net email is sponsored by: The Future of Linux Business White Paper from Novell. From the desktop to the data center, Linux is going mainstream. Let it simplify your IT future. http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4 _______________________________________________ log4perl-devel mailing list log4perl-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/log4perl-devel