On Wed, Oct 18, 2017 at 07:55:31PM +0000, Guillaume Castagnino wrote:
> From: Guillaume Castagnino <casta@xxxxxxxxxx>
> [...]
Stefan raised a few meta issues, all of which I agree with. But I had
some questions about the patch itself:
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9208f42ed1753..0ee7f304ce2b1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3072,6 +3072,7 @@ sub git_get_projects_list {
> # only directories can be git repositories
> return unless (-d $_);
> # need search permission
> + use filetest 'access';
> return unless (-x $_);
This "use" will unconditionally at compile-time (such as "compile" is
for perl, anyway). Which raises a few questions:
- would we want to use "require" instead to avoid loading when we
don't enter this function?
- If the answer to the above is "no" (e.g., because we basically
always need it; I didn't check), should it go at the top of the
script with the other "use" directives?
I think this is a scoped pragma, so what you have here affects only
this particular "-x". But wouldn't other uses of "-x" potentially
want the same benefit?
- Do all relevant versions of perl ship with filetest? According to
Module::Corelist, it first shipped with perl 5.6. In general I think
we treat that as a minimum for our perl scripts, though I do notice
that the gitweb script says "use 5.008". I'm not sure how realistic
that is.
If we can't count on it everywhere, then we probably need to wrap it
in an eval, and fall back to the existing "-x".
-Peff
