Re: [PATCHv2] gitweb: gravatar support

On Sat, 20 June 2009, Aaron Crane wrote:

Giuseppe Bilotta writes:

quoted

+# 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.
 

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