{"thread":{"id":"20341","subject":"[PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format","startedAt":"2009-08-01T20:06:49Z","lastAt":"2009-08-02T00:52:39Z","messageCount":4,"participants":["Mark A Rada","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"119355","messageId":"1CE4F545-1ACA-4786-B0F2-EE7C746E6585@uwaterloo.ca","threadId":"20341","inReplyTo":null,"subject":"[PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format","fromName":"Mark A Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2009-08-01T20:06:49Z","receivedAt":"2009-08-01T20:06:49Z","isPatch":false,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"Comments? I've integrated Jakub Narebski's  suggestions.\n\n\n--\nMark A Rada (ferrous26)\nmarada@uwaterloo.ca\n\n\n------->8-------------\nFrom: Mark Rada <marada@uwaterloo.ca>\nDate: Sat, 1 Aug 2009 13:24:03 -0400\nSubject: [PATCH 2/2] gitweb: support to globally enable/disable a  \nsnapshot format\n\nI added a 'disabled' variable to to the $known_snapshot_formats keys so\nthat a Gitweb administrator can globally enable or disable a specific\nformat for snapshots.\n\nAll formats are enabled by default because project specific overriding\nis disabled by default.\n\nSigned-off-by: Mark Rada <marada@uwaterloo.ca>\n---\n  gitweb/gitweb.perl |   17 ++++++++++++-----\n  1 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3398163..a59569f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -167,27 +167,31 @@ our %known_snapshot_formats = (\n  \t\t'type' => 'application/x-gzip',\n  \t\t'suffix' => '.tar.gz',\n  \t\t'format' => 'tar',\n-\t\t'compressor' => ['gzip']},\n+\t\t'compressor' => ['gzip'],\n+\t\t'disabled' => 0},\n\n  \t'tbz2' => {\n  \t\t'display' => 'tar.bz2',\n  \t\t'type' => 'application/x-bzip2',\n  \t\t'suffix' => '.tar.bz2',\n  \t\t'format' => 'tar',\n-\t\t'compressor' => ['bzip2']},\n+\t\t'compressor' => ['bzip2'],\n+\t\t'disabled' => 0},\n\n  \t'txz' => {\n  \t\t'display' => 'tar.xz',\n  \t\t'type' => 'application/x-xz',\n  \t\t'suffix' => '.tar.xz',\n  \t\t'format' => 'tar',\n-\t\t'compressor' => ['xz']},\n+\t\t'compressor' => ['xz'],\n+\t\t'disabled' => 0},\n\n  \t'zip' => {\n  \t\t'display' => 'zip',\n  \t\t'type' => 'application/x-zip',\n  \t\t'suffix' => '.zip',\n-\t\t'format' => 'zip'},\n+\t\t'format' => 'zip',\n+\t\t'disabled' => 0},\n  );\n\n  # Aliases so we understand old gitweb.snapshot values in repository\n@@ -502,7 +506,8 @@ sub filter_snapshot_fmts {\n  \t\texists $known_snapshot_format_aliases{$_} ?\n  \t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n  \t@fmts = grep {\n-\t\texists $known_snapshot_formats{$_} } @fmts;\n+\t\texists $known_snapshot_formats{$_} &&\n+\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n  }\n\n  our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n@@ -5171,6 +5176,8 @@ sub git_snapshot {\n  \t\tdie_error(400, \"Unknown snapshot format\");\n  \t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n  \t\tdie_error(403, \"Unsupported snapshot format\");\n+\t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n+\t\tdie_error(403, \"Snapshot format not allowed\");\n  \t}\n\n  \tif (!defined $hash) {\n-- \n1.6.4\n"},{"id":"119356","messageId":"m3hbwrqly9.fsf@localhost.localdomain","threadId":"20341","inReplyTo":"1CE4F545-1ACA-4786-B0F2-EE7C746E6585@uwaterloo.ca","subject":"Re: [PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-01T21:11:40Z","receivedAt":"2009-08-01T21:11:40Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Mark A Rada <marada@uwaterloo.ca> writes:\n\n> ------->8-------------\n> From: Mark Rada <marada@uwaterloo.ca>\n> Date: Sat, 1 Aug 2009 13:24:03 -0400\n> Subject: [PATCH 2/2] gitweb: support to globally enable/disable a snapshot format\n> \n> I added a 'disabled' variable to to the $known_snapshot_formats keys so\n> that a Gitweb administrator can globally enable or disable a specific\n> format for snapshots.\n> \n> All formats are enabled by default because project specific overriding\n> is disabled by default.\n> \n> Signed-off-by: Mark Rada <marada@uwaterloo.ca>\n\nO.K.  I think this patch now covers everything needed.  Well, except\ndocumentation.\n\n> ---\n>   gitweb/gitweb.perl |   17 ++++++++++++-----\n>   1 files changed, 12 insertions(+), 5 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 3398163..a59569f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -167,27 +167,31 @@ our %known_snapshot_formats = (\n\nAt the beginning of %known_snapshot_formats definition there is a\ncomment with description of structure of this hash.  You should have\nupdated it, for example in the following way:\n\n@@ -168,7 +168,8 @@ our %known_snapshot_formats = (\n \t# \t'suffix' => filename suffix,\n \t# \t'format' => --format for git-archive,\n \t# \t'compressor' => [compressor command and arguments]\n-\t# \t                (array reference, optional)}\n+\t# \t                (array reference, optional),\n+\t# \t'disabled' => boolean (optional)}\n \t#\n \t'tgz' => {\n \t\t'display' => 'tar.gz',\n\nThe above is, of course, not the only possible way.\n\n>   \t\t'type' => 'application/x-gzip',\n>   \t\t'suffix' => '.tar.gz',\n>   \t\t'format' => 'tar',\n> -\t\t'compressor' => ['gzip']},\n> +\t\t'compressor' => ['gzip'],\n> +\t\t'disabled' => 0},\n> \n>   \t'tbz2' => {\n>   \t\t'display' => 'tar.bz2',\n>   \t\t'type' => 'application/x-bzip2',\n>   \t\t'suffix' => '.tar.bz2',\n>   \t\t'format' => 'tar',\n> -\t\t'compressor' => ['bzip2']},\n> +\t\t'compressor' => ['bzip2'],\n> +\t\t'disabled' => 0},\n> \n>   \t'txz' => {\n>   \t\t'display' => 'tar.xz',\n>   \t\t'type' => 'application/x-xz',\n>   \t\t'suffix' => '.tar.xz',\n>   \t\t'format' => 'tar',\n> -\t\t'compressor' => ['xz']},\n> +\t\t'compressor' => ['xz'],\n> +\t\t'disabled' => 0},\n> \n>   \t'zip' => {\n>   \t\t'display' => 'zip',\n>   \t\t'type' => 'application/x-zip',\n>   \t\t'suffix' => '.zip',\n> -\t\t'format' => 'zip'},\n> +\t\t'format' => 'zip',\n> +\t\t'disabled' => 0},\n>   );\n\nAlthough I though that we don't need to put \"'disabled' => 0\", as \nthe fact that 'disabled' key does not exist means that it is enabled.\n\nBut if you chose to have all known formats not disabled, then I think\nit is a correct solution.  (And then choosing between 'enabled' and\n'disabled' is mainly a matter of taste.)\n\n> \n>   # Aliases so we understand old gitweb.snapshot values in repository\n> @@ -502,7 +506,8 @@ sub filter_snapshot_fmts {\n>   \t\texists $known_snapshot_format_aliases{$_} ?\n>   \t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n>   \t@fmts = grep {\n> -\t\texists $known_snapshot_formats{$_} } @fmts;\n> +\t\texists $known_snapshot_formats{$_} &&\n> +\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n>   }\n> \n>   our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n> @@ -5171,6 +5176,8 @@ sub git_snapshot {\n>   \t\tdie_error(400, \"Unknown snapshot format\");\n>   \t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n>   \t\tdie_error(403, \"Unsupported snapshot format\");\n> +\t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n> +\t\tdie_error(403, \"Snapshot format not allowed\");\n>   \t}\n> \n>   \tif (!defined $hash) {\n\nNow I think that covers everything: preventing displaying known but\ndisabled snapshot formats in snapshots links in gitweb, and preventing\nusing known but disabled snapshot format.\n\n-- \nJakub Narebski\n\nGit User's Survey 2009\nhttp://tinyurl.com/GitSurvey2009\n"},{"id":"119362","messageId":"D33D9C93-300E-4565-8040-BED425A6F2FE@uwaterloo.ca","threadId":"20341","inReplyTo":"m3hbwrqly9.fsf@localhost.localdomain","subject":"Re: [PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format","fromName":"Mark A Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2009-08-01T23:50:38Z","receivedAt":"2009-08-01T23:50:38Z","isPatch":false,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"\nOn 1-Aug-09, at 5:11 PM, Jakub Narebski wrote:\n\n> Mark A Rada <marada@uwaterloo.ca> writes:\n>\n>> ------->8-------------\n>> From: Mark Rada <marada@uwaterloo.ca>\n>> Date: Sat, 1 Aug 2009 13:24:03 -0400\n>> Subject: [PATCH 2/2] gitweb: support to globally enable/disable a  \n>> snapshot format\n>>\n>> I added a 'disabled' variable to to the $known_snapshot_formats  \n>> keys so\n>> that a Gitweb administrator can globally enable or disable a specific\n>> format for snapshots.\n>>\n>> All formats are enabled by default because project specific  \n>> overriding\n>> is disabled by default.\n>>\n>> Signed-off-by: Mark Rada <marada@uwaterloo.ca>\n>\n> O.K.  I think this patch now covers everything needed.  Well, except\n> documentation.\n>\n\nDocumentation documentation or code comments documentation?\n\n>> ---\n>>  gitweb/gitweb.perl |   17 ++++++++++++-----\n>>  1 files changed, 12 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 3398163..a59569f 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -167,27 +167,31 @@ our %known_snapshot_formats = (\n>\n> At the beginning of %known_snapshot_formats definition there is a\n> comment with description of structure of this hash.  You should have\n> updated it, for example in the following way:\n>\n> @@ -168,7 +168,8 @@ our %known_snapshot_formats = (\n> \t# \t'suffix' => filename suffix,\n> \t# \t'format' => --format for git-archive,\n> \t# \t'compressor' => [compressor command and arguments]\n> -\t# \t                (array reference, optional)}\n> +\t# \t                (array reference, optional),\n> +\t# \t'disabled' => boolean (optional)}\n> \t#\n> \t'tgz' => {\n> \t\t'display' => 'tar.gz',\n>\n> The above is, of course, not the only possible way.\n>\n>>  \t\t'type' => 'application/x-gzip',\n>>  \t\t'suffix' => '.tar.gz',\n>>  \t\t'format' => 'tar',\n>> -\t\t'compressor' => ['gzip']},\n>> +\t\t'compressor' => ['gzip'],\n>> +\t\t'disabled' => 0},\n>>\n>>  \t'tbz2' => {\n>>  \t\t'display' => 'tar.bz2',\n>>  \t\t'type' => 'application/x-bzip2',\n>>  \t\t'suffix' => '.tar.bz2',\n>>  \t\t'format' => 'tar',\n>> -\t\t'compressor' => ['bzip2']},\n>> +\t\t'compressor' => ['bzip2'],\n>> +\t\t'disabled' => 0},\n>>\n>>  \t'txz' => {\n>>  \t\t'display' => 'tar.xz',\n>>  \t\t'type' => 'application/x-xz',\n>>  \t\t'suffix' => '.tar.xz',\n>>  \t\t'format' => 'tar',\n>> -\t\t'compressor' => ['xz']},\n>> +\t\t'compressor' => ['xz'],\n>> +\t\t'disabled' => 0},\n>>\n>>  \t'zip' => {\n>>  \t\t'display' => 'zip',\n>>  \t\t'type' => 'application/x-zip',\n>>  \t\t'suffix' => '.zip',\n>> -\t\t'format' => 'zip'},\n>> +\t\t'format' => 'zip',\n>> +\t\t'disabled' => 0},\n>>  );\n>\n> Although I though that we don't need to put \"'disabled' => 0\", as\n> the fact that 'disabled' key does not exist means that it is enabled.\n>\n> But if you chose to have all known formats not disabled, then I think\n> it is a correct solution.  (And then choosing between 'enabled' and\n> 'disabled' is mainly a matter of taste.)\n>\n\nI think spelling it out explicitly in each case makes things more  \nclear, but\nwould it not increase the memory footprint ever so slightly (I'm not  \nfamiliar\nwith PERL internals and whether having a field in one hash entry means\nit will have memory allocated in all the rest). I'm also not convinced  \nthat\nworrying about this micro-optimization is worth it.\n\n\n>>\n>>  # Aliases so we understand old gitweb.snapshot values in repository\n>> @@ -502,7 +506,8 @@ sub filter_snapshot_fmts {\n>>  \t\texists $known_snapshot_format_aliases{$_} ?\n>>  \t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n>>  \t@fmts = grep {\n>> -\t\texists $known_snapshot_formats{$_} } @fmts;\n>> +\t\texists $known_snapshot_formats{$_} &&\n>> +\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n>>  }\n>>\n>>  our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n>> @@ -5171,6 +5176,8 @@ sub git_snapshot {\n>>  \t\tdie_error(400, \"Unknown snapshot format\");\n>>  \t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n>>  \t\tdie_error(403, \"Unsupported snapshot format\");\n>> +\t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n>> +\t\tdie_error(403, \"Snapshot format not allowed\");\n>>  \t}\n>>\n>>  \tif (!defined $hash) {\n>\n> Now I think that covers everything: preventing displaying known but\n> disabled snapshot formats in snapshots links in gitweb, and preventing\n> using known but disabled snapshot format.\n>\n> -- \n> Jakub Narebski\n>\n> Git User's Survey 2009\n> http://tinyurl.com/GitSurvey2009\n\n--\nMark A Rada (ferrous26)\nmarada@uwaterloo.ca\n"},{"id":"119363","messageId":"99F483B0-B5F0-44FB-94EF-7C071E3C3524@gmail.com","threadId":"20341","inReplyTo":"D33D9C93-300E-4565-8040-BED425A6F2FE@uwaterloo.ca","subject":"Re: [PATCHv2 2/2] gitweb: support to globally enable/disable a snapshot format","fromName":"Mark A Rada","fromEmail":"markrada26@gmail.com","sentAt":"2009-08-02T00:52:39Z","receivedAt":"2009-08-02T00:52:39Z","isPatch":false,"sender":{"key":"markrada26@gmail.com","avatar":"https://gravatar.com/avatar/01650ee0e05cc7d0d23b4fc6163279a6c09a4dada1ed9a04ffadbd2c6a347170?d=mp&s=160"},"body":"\nOn 1-Aug-09, at 7:50 PM, Mark A Rada wrote:\n\n>\n> On 1-Aug-09, at 5:11 PM, Jakub Narebski wrote:\n>\n>> Mark A Rada <marada@uwaterloo.ca> writes:\n>>\n>>> ------->8-------------\n>>> From: Mark Rada <marada@uwaterloo.ca>\n>>> Date: Sat, 1 Aug 2009 13:24:03 -0400\n>>> Subject: [PATCH 2/2] gitweb: support to globally enable/disable a  \n>>> snapshot format\n>>>\n>>> I added a 'disabled' variable to to the $known_snapshot_formats  \n>>> keys so\n>>> that a Gitweb administrator can globally enable or disable a  \n>>> specific\n>>> format for snapshots.\n>>>\n>>> All formats are enabled by default because project specific  \n>>> overriding\n>>> is disabled by default.\n>>>\n>>> Signed-off-by: Mark Rada <marada@uwaterloo.ca>\n>>\n>> O.K.  I think this patch now covers everything needed.  Well, except\n>> documentation.\n>>\n>\n> Documentation documentation or code comments documentation?\n\nNevermind, I just discovered there was an INSTALL and README file for\nGitweb...I'll update those.\n\n\n>\n>>> ---\n>>> gitweb/gitweb.perl |   17 ++++++++++++-----\n>>> 1 files changed, 12 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>>> index 3398163..a59569f 100755\n>>> --- a/gitweb/gitweb.perl\n>>> +++ b/gitweb/gitweb.perl\n>>> @@ -167,27 +167,31 @@ our %known_snapshot_formats = (\n>>\n>> At the beginning of %known_snapshot_formats definition there is a\n>> comment with description of structure of this hash.  You should have\n>> updated it, for example in the following way:\n>>\n>> @@ -168,7 +168,8 @@ our %known_snapshot_formats = (\n>> \t# \t'suffix' => filename suffix,\n>> \t# \t'format' => --format for git-archive,\n>> \t# \t'compressor' => [compressor command and arguments]\n>> -\t# \t                (array reference, optional)}\n>> +\t# \t                (array reference, optional),\n>> +\t# \t'disabled' => boolean (optional)}\n>> \t#\n>> \t'tgz' => {\n>> \t\t'display' => 'tar.gz',\n>>\n>> The above is, of course, not the only possible way.\n>>\n>>> \t\t'type' => 'application/x-gzip',\n>>> \t\t'suffix' => '.tar.gz',\n>>> \t\t'format' => 'tar',\n>>> -\t\t'compressor' => ['gzip']},\n>>> +\t\t'compressor' => ['gzip'],\n>>> +\t\t'disabled' => 0},\n>>>\n>>> \t'tbz2' => {\n>>> \t\t'display' => 'tar.bz2',\n>>> \t\t'type' => 'application/x-bzip2',\n>>> \t\t'suffix' => '.tar.bz2',\n>>> \t\t'format' => 'tar',\n>>> -\t\t'compressor' => ['bzip2']},\n>>> +\t\t'compressor' => ['bzip2'],\n>>> +\t\t'disabled' => 0},\n>>>\n>>> \t'txz' => {\n>>> \t\t'display' => 'tar.xz',\n>>> \t\t'type' => 'application/x-xz',\n>>> \t\t'suffix' => '.tar.xz',\n>>> \t\t'format' => 'tar',\n>>> -\t\t'compressor' => ['xz']},\n>>> +\t\t'compressor' => ['xz'],\n>>> +\t\t'disabled' => 0},\n>>>\n>>> \t'zip' => {\n>>> \t\t'display' => 'zip',\n>>> \t\t'type' => 'application/x-zip',\n>>> \t\t'suffix' => '.zip',\n>>> -\t\t'format' => 'zip'},\n>>> +\t\t'format' => 'zip',\n>>> +\t\t'disabled' => 0},\n>>> );\n>>\n>> Although I though that we don't need to put \"'disabled' => 0\", as\n>> the fact that 'disabled' key does not exist means that it is enabled.\n>>\n>> But if you chose to have all known formats not disabled, then I think\n>> it is a correct solution.  (And then choosing between 'enabled' and\n>> 'disabled' is mainly a matter of taste.)\n>>\n>\n> I think spelling it out explicitly in each case makes things more  \n> clear, but\n> would it not increase the memory footprint ever so slightly (I'm not  \n> familiar\n> with PERL internals and whether having a field in one hash entry means\n> it will have memory allocated in all the rest). I'm also not  \n> convinced that\n> worrying about this micro-optimization is worth it.\n>\n>\n>>>\n>>> # Aliases so we understand old gitweb.snapshot values in repository\n>>> @@ -502,7 +506,8 @@ sub filter_snapshot_fmts {\n>>> \t\texists $known_snapshot_format_aliases{$_} ?\n>>> \t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n>>> \t@fmts = grep {\n>>> -\t\texists $known_snapshot_formats{$_} } @fmts;\n>>> +\t\texists $known_snapshot_formats{$_} &&\n>>> +\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n>>> }\n>>>\n>>> our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n>>> @@ -5171,6 +5176,8 @@ sub git_snapshot {\n>>> \t\tdie_error(400, \"Unknown snapshot format\");\n>>> \t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n>>> \t\tdie_error(403, \"Unsupported snapshot format\");\n>>> +\t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n>>> +\t\tdie_error(403, \"Snapshot format not allowed\");\n>>> \t}\n>>>\n>>> \tif (!defined $hash) {\n>>\n>> Now I think that covers everything: preventing displaying known but\n>> disabled snapshot formats in snapshots links in gitweb, and  \n>> preventing\n>> using known but disabled snapshot format.\n>>\n>> -- \n>> Jakub Narebski\n>>\n>> Git User's Survey 2009\n>> http://tinyurl.com/GitSurvey2009\n>\n> --\n> Mark A Rada (ferrous26)\n> marada@uwaterloo.ca\n>\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"}]}