Re: [PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format

>
> On 1-Aug-09, at 5:11 PM, Jakub Narebski wrote:
>
>> Mark A Rada <marada@uwaterloo.ca> writes:
>>
>>> ------->8-------------
>>> From: Mark Rada <marada@uwaterloo.ca>
>>> 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 <marada@uwaterloo.ca>
>>
>> O.K.  I think this patch now covers everything needed.  Well, except
>> documentation.
>>
>
> Documentation documentation or code comments documentation?

>
>>> ---
>>> 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