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

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

[Mark A Rada]

Comments? I've integrated Jakub Narebski's  suggestions.


--
Mark A Rada (ferrous26)
marada@uwaterloo.ca


------->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>
---
  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 = (
  		'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},
  );

  # 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) {
-- 
1.6.4

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

[Jakub Narebski]

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.

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

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

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

[Mark A Rada]

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

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

[Mark A Rada]

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

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