Re: [log4perl-devel] Util::module_available

[Eric W. <scr...@gm...>]

# from Mike Schilli
# on Sunday 28 January 2007 07:30 pm:

>On Fri, 26 Jan 2007, Eric Wilhelm wrote:
>> The issue is that the code never gets compiled.  The @INC hook
>> sets[1] $INC{"foo/bar.pm"}, so any subsequent require()'s don't try
>> to load it. The errors will be equivalent to something like:
>>
>>   mkdir -p foo/Time
>>   echo "1;" > foo/Time/HiRes.pm
>>   perl -Ifoo -e 'use Time::HiRes';

Sorry, that last line should have read

  perl -Ifoo -e 'use Log::Log4perl'

Which should get you a huge pile of errors starting with:

  Undefined subroutine &Time::HiRes::gettimeofday called   
  at /usr/.../Log/Log4perl/Layout/PatternLayout.pm
  line 34.

Of course, nothing would work in this circumstance, but exactly the same 
thing is happening with your module_available() call in the presence of 
an @INC hook.

>> Because the @INC hook only returns a filehandle, to fully
>> re-implement require (which I don't suggest), you must
>> eval("$string") the contents of that.
>
>Can you send some perl code that shows the problem? I still don't
> understand what the problem is, especially where the @INC hook comes
> from.

Please see the attached demo.  The @INC hook in par is basically 
_find_par_internals(), which you can read here:

  http://search.cpan.org/src/SMUELLER/PAR-0.972/lib/PAR.pm

If you're still not seeing what's wrong with calling those coderefs and 
not eval($string)'ing the result, please read 'perldoc -f require' 
again.  "Note that these hooks are also permitted to set the %INC entry 
corresponding to the files they have loaded."

>> My question is:  why not eval {require Foo} ?
>Try this:
>    $SIG{__DIE__} = sub { print STDERR "Die! What?\n"; exit 0 };

I can understand the concern, but if that is the only reason, then I 
have to say it is bad trade-off to break a documented perl feature to 
allow for improper usage of a discouraged one.

That is a poorly written die hook which didn't follow the documentation.  
See perlfunc/die.  Any well-behaved SIG{__DIE__} should have:

  die @_ if $^S; # get out if we're in an eval

Though, you could always put local $SIG{__DIE__} in the eval if you're 
feeling lenient.  If you rework module_available() to use eval, that 
would be a good place for it.

Note, the attached (academic) example will demonstrate the issue.  
Calling it with no arguments means the first call to the hook is from 
module_available(), with a command-line argument, we'll require 
Sys::Hostname ourselves and module_available() will be happy.

Also attached is a patch which implements my proposed fix.

As I said, you could check whether $@ =~ /Can't locate $mod_path/, but 
that's only if you think compilation errors should be propagated (hint: 
put another curly around the "local $SIG{DIE}; eval {...}" chunk before 
you throw an error so it propagates correctly.)

Hopefully, the break_log4perl.pl attachment can be easily adapted to 
your test suite.  Please let me know if there's anything else I can do 
to help get this fixed.

Thanks,
Eric
-- 
"Left to themselves, things tend to go from bad to worse."
--Murphy's Corollary
---------------------------------------------------
    http://scratchcomputing.com
---------------------------------------------------