From: Mark A Rada Date: Sun, 02 Aug 2009 00:52:39 GMT Subject: Re: [PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format Message-ID: <99F483B0-B5F0-44FB-94EF-7C071E3C3524@gmail.com> In-Reply-To: On 1-Aug-09, at 7:50 PM, Mark A Rada wrote: > > On 1-Aug-09, at 5:11 PM, Jakub Narebski wrote: > >> Mark A Rada writes: >> >>> ------->8------------- >>> From: Mark Rada >>> Date: Sat, 1 Aug 2009 13:24:03 -0400 >>> Subject: [PATCH 2/2] gitweb: support to globally enable/disable a >>> snapshot format >>> >>> I added a 'disabled' variable to to the $known_snapshot_formats >>> keys so >>> that a Gitweb administrator can globally enable or disable a >>> specific >>> format for snapshots. >>> >>> All formats are enabled by default because project specific >>> overriding >>> is disabled by default. >>> >>> Signed-off-by: Mark Rada >> >> O.K. I think this patch now covers everything needed. Well, except >> documentation. >> > > Documentation documentation or code comments documentation? Nevermind, I just discovered there was an INSTALL and README file for Gitweb...I'll update those. > >>> --- >>> gitweb/gitweb.perl | 17 ++++++++++++----- >>> 1 files changed, 12 insertions(+), 5 deletions(-) >>> >>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >>> index 3398163..a59569f 100755 >>> --- a/gitweb/gitweb.perl >>> +++ b/gitweb/gitweb.perl >>> @@ -167,27 +167,31 @@ our %known_snapshot_formats = ( >> >> At the beginning of %known_snapshot_formats definition there is a >> comment with description of structure of this hash. You should have >> updated it, for example in the following way: >> >> @@ -168,7 +168,8 @@ our %known_snapshot_formats = ( >> # 'suffix' => filename suffix, >> # 'format' => --format for git-archive, >> # 'compressor' => [compressor command and arguments] >> - # (array reference, optional)} >> + # (array reference, optional), >> + # 'disabled' => boolean (optional)} >> # >> 'tgz' => { >> 'display' => 'tar.gz', >> >> The above is, of course, not the only possible way. >> >>> 'type' => 'application/x-gzip', >>> 'suffix' => '.tar.gz', >>> 'format' => 'tar', >>> - 'compressor' => ['gzip']}, >>> + 'compressor' => ['gzip'], >>> + 'disabled' => 0}, >>> >>> 'tbz2' => { >>> 'display' => 'tar.bz2', >>> 'type' => 'application/x-bzip2', >>> 'suffix' => '.tar.bz2', >>> 'format' => 'tar', >>> - 'compressor' => ['bzip2']}, >>> + 'compressor' => ['bzip2'], >>> + 'disabled' => 0}, >>> >>> 'txz' => { >>> 'display' => 'tar.xz', >>> 'type' => 'application/x-xz', >>> 'suffix' => '.tar.xz', >>> 'format' => 'tar', >>> - 'compressor' => ['xz']}, >>> + 'compressor' => ['xz'], >>> + 'disabled' => 0}, >>> >>> 'zip' => { >>> 'display' => 'zip', >>> 'type' => 'application/x-zip', >>> 'suffix' => '.zip', >>> - 'format' => 'zip'}, >>> + 'format' => 'zip', >>> + 'disabled' => 0}, >>> ); >> >> Although I though that we don't need to put "'disabled' => 0", as >> the fact that 'disabled' key does not exist means that it is enabled. >> >> But if you chose to have all known formats not disabled, then I think >> it is a correct solution. (And then choosing between 'enabled' and >> 'disabled' is mainly a matter of taste.) >> > > I think spelling it out explicitly in each case makes things more > clear, but > would it not increase the memory footprint ever so slightly (I'm not > familiar > with PERL internals and whether having a field in one hash entry means > it will have memory allocated in all the rest). I'm also not > convinced that > worrying about this micro-optimization is worth it. > > >>> >>> # Aliases so we understand old gitweb.snapshot values in repository >>> @@ -502,7 +506,8 @@ sub filter_snapshot_fmts { >>> exists $known_snapshot_format_aliases{$_} ? >>> $known_snapshot_format_aliases{$_} : $_} @fmts; >>> @fmts = grep { >>> - exists $known_snapshot_formats{$_} } @fmts; >>> + exists $known_snapshot_formats{$_} && >>> + !$known_snapshot_formats{$_}{'disabled'}} @fmts; >>> } >>> >>> our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++"; >>> @@ -5171,6 +5176,8 @@ sub git_snapshot { >>> die_error(400, "Unknown snapshot format"); >>> } elsif (!grep($_ eq $format, @snapshot_fmts)) { >>> die_error(403, "Unsupported snapshot format"); >>> + } elsif ($known_snapshot_formats{$format}{'disabled'}) { >>> + die_error(403, "Snapshot format not allowed"); >>> } >>> >>> if (!defined $hash) { >> >> Now I think that covers everything: preventing displaying known but >> disabled snapshot formats in snapshots links in gitweb, and >> preventing >> using known but disabled snapshot format. >> >> -- >> Jakub Narebski >> >> Git User's Survey 2009 >> http://tinyurl.com/GitSurvey2009 > > -- > Mark A Rada (ferrous26) > marada@uwaterloo.ca > > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html