Re: [PATCHv2] gitweb: gravatar support
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jun 20, 2009, 16:58 UTC
- Message-ID
- <200906201858.57346.jnareb@gmail.com>
- In-Reply-To
- <20090620141626.GK7675@aaroncrane.co.uk>
On Sat, 20 June 2009, Aaron Crane wrote:
> Giuseppe Bilotta writes:
Show 13 quoted lines
> > +# check if gravatars are enabled and dependencies are satisfied
> > +our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&
> > + (eval { use Digest::MD5 qw(md5_hex); 1; });
>
> This test for the availability of Digest::MD5 is broken: `use`
> statements are executed at compile time, so the whole program will
> fail if Digest::MD5 can't be loaded.
>
> A possible fix would be to move the compile-time actions to run time:
>
> our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&
> (eval { require Digest::MD5; Digest::MD5->import('md5_hex'); 1 });
>Good catch!!! But we don't need import if we are to use slightly longer form: Digest::MD5::md5_hex.
Show 6 quoted lines
> However, I don't recommend doing that. Digest::MD5 is a core module > in Perl 5.8.0 and later, so an installation of Perl 5.8 that doesn't > have it is broken. Since gitweb.perl already needs 5.8 (because of > the `binmode STDOUT, ':utf8'` at the top, if nothing else) I see no > value in jumping through hoops to make this work in the essentially > impossible situation where Digest::MD5 is unavailable.
In this case the problem is not with Digest::MD5 not being installed. The problem is with not loading this module in a CGI script if it is not required (i.e. problem is not dependencies but performance).
BTW. we sometimes unnecessary calculate MD5 hash repeatedly for the same author / committer in log-like views... but I don't see how to solve it in an easy way.
-- Jakub Narebski Poland