{"thread":{"id":"8762","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","startedAt":"2007-06-28T18:02:13Z","lastAt":"2007-08-27T11:01:05Z","messageCount":35,"participants":["Matt McCutchen","Junio C Hamano","Jakub Narebski","Luben Tuikov","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45993","messageId":"1183053733.6108.0.camel@mattlaptop2","threadId":"8762","inReplyTo":null,"subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-06-28T18:02:13Z","receivedAt":"2007-06-28T18:02:13Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and the \"_snapshot_\" link is replaced with, say,\n \"snapshot (_tbz2_ _zip_)\".\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nSigned-off-by: Matt McCutchen \n---\n\nI implemented this a while ago for my Web site. You can see it in action at:\n\nhttp://www.kepreon.com/~matt/mgear/mgear.git/\n\nI thought I would submit it to the main project so you can adopt it if you like\nit.\n\nMatt\n\n gitweb/gitweb.perl | 89 +++++++++++++++++++++++++++++----------------------\n 1 files changed, 51 insertions(+), 38 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex dbfb044..f36428e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -101,6 +101,15 @@ our $mimetypes_file = undef;\n # could be even 'utf-8' for the old behavior)\n our $fallback_encoding = 'latin1';\n \n+# information about snapshot formats that gitweb is capable of serving\n+# name => [mime type, filename suffix, --format for git-archive,\n+# compressor command suffix]\n+our %known_snapshot_formats = (\n+\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n+\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n+\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -131,20 +140,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -243,28 +254,15 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /,/, $val);\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn grep exists $known_snapshot_formats{$_}, @fmts;\n }\n \n sub feature_grep {\n@@ -542,6 +540,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1236,6 +1235,21 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"snapshot (tbz2 zip)\", linked.\n+# Pass the hash.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tif (@snapshot_fmts) {\n+\t\treturn \"snapshot (\" . join(' ', map $cgi->a(\n+\t\t\t{-href => href(action=>\"snapshot\", hash=>$hash, snapshot_format=>$_)}, \"$_\"),\n+\t\t\t@snapshot_fmts)\n+\t\t. \")\";\n+\t} else {\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3280,8 +3294,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3308,8 +3320,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4194,11 +4207,16 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n+\tmy ($ctype, $suffix, $ga_format, $pipe_compressor) =\n+\t\t@{$known_snapshot_formats{$format}};\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n@@ -4211,16 +4229,11 @@ sub git_snapshot {\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n-\t}\n+\t$filename .= \"-$hash$suffix\";\n+\t$cmd = \"$git archive --format=$ga_format --prefix=\\'$name\\'/ $hash $pipe_compressor\";\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => \"$ctype\",\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n-- \n1.5.2.2.552.gc32f\n"},{"id":"46716","messageId":"7vir8w6inf.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"1183053733.6108.0.camel@mattlaptop2","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-07T20:52:36Z","receivedAt":"2007-07-07T20:52:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index dbfb044..f36428e 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -101,6 +101,15 @@ our $mimetypes_file = undef;\n> # could be even 'utf-8' for the old behavior)\n> our $fallback_encoding = 'latin1';\n> \n> +# information about snapshot formats that gitweb is capable of serving\n> +# name => [mime type, filename suffix, --format for git-archive,\n> +# compressor command suffix]\n> +our %known_snapshot_formats = (\n> +\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n> +\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n> +\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n> +);\n> +\n> # You define site-wide feature defaults here; override them with\n> # $GITWEB_CONFIG as necessary.\n> our %feature = (\n> @@ -131,20 +140,22 @@ our %feature = (\n> ...\n> \t'snapshot' => {\n> \t\t'sub' => \\&feature_snapshot,\n> \t\t'override' => 0,\n> -\t\t# => [content-encoding, suffix, program]\n> -\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n> +\t\t'default' => ['tgz']},\n> \n> \t# Enable text search, which will list the commits which match author,\n> \t# committer or commit text to a given string. Enabled by default.\n\nThis is a very nice clean-up, and I agree we should go this\nroute in the longer term.\n\nThis however will break people's existing gitweb configuration,\nso if we were to do this it should be post 1.5.3, I would say.\n"},{"id":"46742","messageId":"7v8x9r2rjy.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"7vir8w6inf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T09:06:09Z","receivedAt":"2007-07-08T09:06:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This however will break people's existing gitweb configuration,\n> so if we were to do this it should be post 1.5.3, I would say.\n\nWe have swallowed some changes that breaks details of user\nexperience so far, and compared to one of them, the\nincompatibility this brings in is much more benign. We haven't\ndeclared -rc1 when the command set and features for the next\nrelease is cast in stone.\n\nI am tempted to change my mind and am inclined to apply this.\n"},{"id":"46799","messageId":"7v1wfi1rz6.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"1183053733.6108.0.camel@mattlaptop2","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T21:54:37Z","receivedAt":"2007-07-08T21:54:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt McCutchen writes:\n\n> -sub gitweb_have_snapshot {\n> -\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n> -\tmy $have_snapshot = (defined $ctype && defined $suffix);\n> -\n> -\treturn $have_snapshot;\n\nAlthough you are removing this function, you still have a couple\nof callers left in the code.\n"},{"id":"46895","messageId":"3bbc18d20707091552l29fb81b6v34da9cef3ec0df58@mail.gmail.com","threadId":"8762","inReplyTo":"7v1wfi1rz6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-09T22:52:56Z","receivedAt":"2007-07-09T22:52:56Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On 7/8/07, Junio C Hamano wrote:\n> Matt McCutchen writes:\n>\n> > -sub gitweb_have_snapshot {\n> > - my ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n> > - my $have_snapshot = (defined $ctype && defined $suffix);\n> > -\n> > - return $have_snapshot;\n>\n> Although you are removing this function, you still have a couple\n> of callers left in the code.\n\nOK, I will revise the patch, submit it and see if I can get it to\nappear as a reply to this thread. Incidentally, when only one format\nis offered, would you prefer the snapshot link to appear as\n\"_snapshot_\" (the same as before) or \"_snapshot (tgz)_\" instead of the\n\"snapshot (_tgz_)\" that the current patch does?\n\nMatt\n"},{"id":"46897","messageId":"1184023318.9703.1.camel@mattlaptop2","threadId":"8762","inReplyTo":"3bbc18d20707091552l29fb81b6v34da9cef3ec0df58@mail.gmail.com","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-09T23:21:58Z","receivedAt":"2007-07-09T23:21:58Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and the \"_snapshot_\" link is replaced with, say,\n \"snapshot (_tbz2_ _zip_)\".\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nAlert for gitweb site administrators: This patch changes the format of\n$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of three\npieces of information about a single format to a list of one or more formats\nyou wish to offer from the set ('tgz', 'tbz2', 'zip'). Update your\ngitweb_config.perl appropriately.\n\nSigned-off-by: Matt McCutchen \n---\nThis revised patch converts the two remaining gitweb_have_snapshot callers and\nadds the alert in the commit message.\n\n gitweb/gitweb.perl | 106 ++++++++++++++++++++++++++++------------------------\n 1 files changed, 57 insertions(+), 49 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex dc609f4..4313671 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -101,6 +101,15 @@ our $mimetypes_file = undef;\n # could be even 'utf-8' for the old behavior)\n our $fallback_encoding = 'latin1';\n \n+# information about snapshot formats that gitweb is capable of serving\n+# name => [mime type, filename suffix, --format for git-archive,\n+# compressor command suffix]\n+our %known_snapshot_formats = (\n+\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n+\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n+\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -131,20 +140,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -243,28 +254,15 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /,/, $val);\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn grep exists $known_snapshot_formats{$_}, @fmts;\n }\n \n sub feature_grep {\n@@ -542,6 +540,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1236,6 +1235,21 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"snapshot (tbz2 zip)\", linked.\n+# Pass the hash.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tif (@snapshot_fmts) {\n+\t\treturn \"snapshot (\" . join(' ', map $cgi->a(\n+\t\t\t{-href => href(action=>\"snapshot\", hash=>$hash, snapshot_format=>$_)}, \"$_\"),\n+\t\t\t@snapshot_fmts)\n+\t\t. \")\";\n+\t} else {\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3299,8 +3313,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3327,8 +3339,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4110,8 +4123,6 @@ sub git_blob {\n }\n \n sub git_tree {\n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tif (!defined $hash_base) {\n \t\t$hash_base = \"HEAD\";\n \t}\n@@ -4145,11 +4156,10 @@ sub git_tree {\n \t\t\t\t hash_base=>\"HEAD\", file_name=>$file_name)},\n \t\t\t\t \"HEAD\"),\n \t\t}\n-\t\tif ($have_snapshot) {\n+\t\tmy $snapshot_links = format_snapshot_links($hash);\n+\t\tif (defined $snapshot_links) {\n \t\t\t# FIXME: Should be available when we have no hash base as well.\n-\t\t\tpush @views_nav,\n-\t\t\t\t$cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)},\n-\t\t\t\t \"snapshot\");\n+\t\t\tpush @views_nav, $snapshot_links;\n \t\t}\n \t\tgit_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);\n@@ -4213,11 +4223,16 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n+\tmy ($ctype, $suffix, $ga_format, $pipe_compressor) =\n+\t\t@{$known_snapshot_formats{$format}};\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n@@ -4230,16 +4245,11 @@ sub git_snapshot {\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n-\t}\n+\t$filename .= \"-$hash$suffix\";\n+\t$cmd = \"$git archive --format=$ga_format --prefix=\\'$name\\'/ $hash $pipe_compressor\";\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => \"$ctype\",\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n@@ -4368,8 +4378,6 @@ sub git_commit {\n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $co{'id'});\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tgit_header_html(undef, $expires);\n \tgit_print_page_nav('commit', '',\n \t $hash, $co{'tree'}, $hash,\n@@ -4408,9 +4416,9 @@ sub git_commit {\n \t \"\" .\n \t $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$hash)},\n \t \"tree\");\n-\tif ($have_snapshot) {\n-\t\tprint \" | \" .\n-\t\t $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)}, \"snapshot\");\n+\tmy $snapshot_links = format_snapshot_links($hash);\n+\tif (defined $snapshot_links) {\n+\t\tprint \" | \" . $snapshot_links;\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-- \n1.5.3.rc0.82.g2bad2-dirty\n"},{"id":"46898","messageId":"7vr6nht9yq.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"3bbc18d20707091552l29fb81b6v34da9cef3ec0df58@mail.gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-09T23:48:29Z","receivedAt":"2007-07-09T23:48:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matt McCutchen\" writes:\n\n> On 7/8/07, Junio C Hamano wrote:\n>> Matt McCutchen writes:\n>>\n>> > -sub gitweb_have_snapshot {\n>> > - my ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n>> > - my $have_snapshot = (defined $ctype && defined $suffix);\n>> > -\n>> > - return $have_snapshot;\n>>\n>> Although you are removing this function, you still have a couple\n>> of callers left in the code.\n>\n> OK, I will revise the patch, submit it and see if I can get it to\n> appear as a reply to this thread.\n\nThanks. For future reference, I caught it not by code\ninspection, but by running one of the tests (t9500).\n\n> Incidentally, when only one format\n> is offered, would you prefer the snapshot link to appear as\n> \"_snapshot_\" (the same as before) or \"_snapshot (tgz)_\" instead of the\n> \"snapshot (_tgz_)\" that the current patch does?\n\nWhen only one format is offerred by the site, it is not like the\nend user has any choice, so \"_snapshot_\" is probably the most\nappropriate from the screen real-estate point-of-view.\n\nThe end user _might_ complain \"Geez, I cannot grok zip, I can\nonly expand tar. I would not have clicked the link if it said\n'snapshot (_zip_)', but the stupid gitweb said '_snapshot_' and\nnothing else.\" So in that sense, we are robbing one choice the\nuser has (i.e. \"to decide not to click the link, based on the\nformat of the data that would be given\"), but I do not think\nthat is something we would seriously want to worry about.\n"},{"id":"46900","messageId":"1184030060.11726.0.camel@mattlaptop2","threadId":"8762","inReplyTo":"7vr6nht9yq.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-10T01:14:20Z","receivedAt":"2007-07-10T01:14:20Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and if more than one format is offered, the \"_snapshot_\"\n link becomes something like \"snapshot (_tbz2_ _zip_)\".\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nAlert for gitweb site administrators: This patch changes the format of\n$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of three\npieces of information about a single format to a list of one or more formats\nyou wish to offer from the set ('tgz', 'tbz2', 'zip'). Update your\ngitweb_config.perl appropriately.\n\nSigned-off-by: Matt McCutchen \n---\nThis third revision of the patch keeps the link as \"_snapshot_\" when only one\nformat is offered.\n\n gitweb/gitweb.perl | 110 +++++++++++++++++++++++++++++-----------------------\n 1 files changed, 61 insertions(+), 49 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex dc609f4..b520342 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -101,6 +101,15 @@ our $mimetypes_file = undef;\n # could be even 'utf-8' for the old behavior)\n our $fallback_encoding = 'latin1';\n \n+# information about snapshot formats that gitweb is capable of serving\n+# name => [mime type, filename suffix, --format for git-archive,\n+# compressor command suffix]\n+our %known_snapshot_formats = (\n+\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n+\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n+\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -131,20 +140,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -243,28 +254,15 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /,/, $val);\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn grep exists $known_snapshot_formats{$_}, @fmts;\n }\n \n sub feature_grep {\n@@ -542,6 +540,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1236,6 +1235,25 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"snapshot (tbz2 zip)\", linked.\n+# Pass the hash.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tmy $num_fmts = @snapshot_fmts;\n+\tif ($num_fmts > 1) {\n+\t\treturn \"snapshot (\" . join(' ', map $cgi->a(\n+\t\t\t{-href => href(action=>\"snapshot\", hash=>$hash, snapshot_format=>$_)}, \"$_\"),\n+\t\t\t@snapshot_fmts)\n+\t\t. \")\";\n+\t} elsif ($num_fmts == 1) {\n+\t\treturn $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash,\n+\t\t\tsnapshot_format=>$snapshot_fmts[0])}, \"snapshot\");\n+\t} else { # $num_fmts == 0\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3299,8 +3317,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3327,8 +3343,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4110,8 +4127,6 @@ sub git_blob {\n }\n \n sub git_tree {\n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tif (!defined $hash_base) {\n \t\t$hash_base = \"HEAD\";\n \t}\n@@ -4145,11 +4160,10 @@ sub git_tree {\n \t\t\t\t hash_base=>\"HEAD\", file_name=>$file_name)},\n \t\t\t\t \"HEAD\"),\n \t\t}\n-\t\tif ($have_snapshot) {\n+\t\tmy $snapshot_links = format_snapshot_links($hash);\n+\t\tif (defined $snapshot_links) {\n \t\t\t# FIXME: Should be available when we have no hash base as well.\n-\t\t\tpush @views_nav,\n-\t\t\t\t$cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)},\n-\t\t\t\t \"snapshot\");\n+\t\t\tpush @views_nav, $snapshot_links;\n \t\t}\n \t\tgit_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);\n@@ -4213,11 +4227,16 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n+\tmy ($ctype, $suffix, $ga_format, $pipe_compressor) =\n+\t\t@{$known_snapshot_formats{$format}};\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n@@ -4230,16 +4249,11 @@ sub git_snapshot {\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n-\t}\n+\t$filename .= \"-$hash$suffix\";\n+\t$cmd = \"$git archive --format=$ga_format --prefix=\\'$name\\'/ $hash $pipe_compressor\";\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => \"$ctype\",\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n@@ -4368,8 +4382,6 @@ sub git_commit {\n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $co{'id'});\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tgit_header_html(undef, $expires);\n \tgit_print_page_nav('commit', '',\n \t $hash, $co{'tree'}, $hash,\n@@ -4408,9 +4420,9 @@ sub git_commit {\n \t \"\" .\n \t $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$hash)},\n \t \"tree\");\n-\tif ($have_snapshot) {\n-\t\tprint \" | \" .\n-\t\t $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)}, \"snapshot\");\n+\tmy $snapshot_links = format_snapshot_links($hash);\n+\tif (defined $snapshot_links) {\n+\t\tprint \" | \" . $snapshot_links;\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-- \n1.5.3.rc0.83.gb18a6\n"},{"id":"46901","messageId":"1184030092.14364.0.camel@mattlaptop2","threadId":"8762","inReplyTo":"7vr6nht9yq.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-10T01:14:52Z","receivedAt":"2007-07-10T01:14:52Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and if more than one format is offered, the \"_snapshot_\"\n link becomes something like \"snapshot (_tbz2_ _zip_)\".\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nAlert for gitweb site administrators: This patch changes the format of\n$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of three\npieces of information about a single format to a list of one or more formats\nyou wish to offer from the set ('tgz', 'tbz2', 'zip'). Update your\ngitweb_config.perl appropriately.\n\nSigned-off-by: Matt McCutchen \n---\nThis third revision of the patch keeps the link as \"_snapshot_\" when only one\nformat is offered.\n\n gitweb/gitweb.perl | 110 +++++++++++++++++++++++++++++-----------------------\n 1 files changed, 61 insertions(+), 49 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex dc609f4..b520342 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -101,6 +101,15 @@ our $mimetypes_file = undef;\n # could be even 'utf-8' for the old behavior)\n our $fallback_encoding = 'latin1';\n \n+# information about snapshot formats that gitweb is capable of serving\n+# name => [mime type, filename suffix, --format for git-archive,\n+# compressor command suffix]\n+our %known_snapshot_formats = (\n+\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n+\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n+\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -131,20 +140,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -243,28 +254,15 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /,/, $val);\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn grep exists $known_snapshot_formats{$_}, @fmts;\n }\n \n sub feature_grep {\n@@ -542,6 +540,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1236,6 +1235,25 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"snapshot (tbz2 zip)\", linked.\n+# Pass the hash.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tmy $num_fmts = @snapshot_fmts;\n+\tif ($num_fmts > 1) {\n+\t\treturn \"snapshot (\" . join(' ', map $cgi->a(\n+\t\t\t{-href => href(action=>\"snapshot\", hash=>$hash, snapshot_format=>$_)}, \"$_\"),\n+\t\t\t@snapshot_fmts)\n+\t\t. \")\";\n+\t} elsif ($num_fmts == 1) {\n+\t\treturn $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash,\n+\t\t\tsnapshot_format=>$snapshot_fmts[0])}, \"snapshot\");\n+\t} else { # $num_fmts == 0\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3299,8 +3317,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3327,8 +3343,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4110,8 +4127,6 @@ sub git_blob {\n }\n \n sub git_tree {\n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tif (!defined $hash_base) {\n \t\t$hash_base = \"HEAD\";\n \t}\n@@ -4145,11 +4160,10 @@ sub git_tree {\n \t\t\t\t hash_base=>\"HEAD\", file_name=>$file_name)},\n \t\t\t\t \"HEAD\"),\n \t\t}\n-\t\tif ($have_snapshot) {\n+\t\tmy $snapshot_links = format_snapshot_links($hash);\n+\t\tif (defined $snapshot_links) {\n \t\t\t# FIXME: Should be available when we have no hash base as well.\n-\t\t\tpush @views_nav,\n-\t\t\t\t$cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)},\n-\t\t\t\t \"snapshot\");\n+\t\t\tpush @views_nav, $snapshot_links;\n \t\t}\n \t\tgit_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);\n@@ -4213,11 +4227,16 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n+\tmy ($ctype, $suffix, $ga_format, $pipe_compressor) =\n+\t\t@{$known_snapshot_formats{$format}};\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n@@ -4230,16 +4249,11 @@ sub git_snapshot {\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n-\t}\n+\t$filename .= \"-$hash$suffix\";\n+\t$cmd = \"$git archive --format=$ga_format --prefix=\\'$name\\'/ $hash $pipe_compressor\";\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => \"$ctype\",\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n@@ -4368,8 +4382,6 @@ sub git_commit {\n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $co{'id'});\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tgit_header_html(undef, $expires);\n \tgit_print_page_nav('commit', '',\n \t $hash, $co{'tree'}, $hash,\n@@ -4408,9 +4420,9 @@ sub git_commit {\n \t \"\" .\n \t $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$hash)},\n \t \"tree\");\n-\tif ($have_snapshot) {\n-\t\tprint \" | \" .\n-\t\t $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)}, \"snapshot\");\n+\tmy $snapshot_links = format_snapshot_links($hash);\n+\tif (defined $snapshot_links) {\n+\t\tprint \" | \" . $snapshot_links;\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-- \n1.5.3.rc0.83.gb18a6\n"},{"id":"46992","messageId":"f715g6$2sv$1@sea.gmane.org","threadId":"8762","inReplyTo":"1184023318.9703.1.camel@mattlaptop2","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-10T23:41:59Z","receivedAt":"2007-07-10T23:41:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Matt McCutchen wrote:\n\n> - Centralize knowledge about snapshot formats (mime types, extensions,\n> commands) in %known_snapshot_formats and improve how some of that\n> information is specified. In particular, zip files are no longer a\n> special case.\n> \n> - Add support for offering multiple snapshot formats to the user so\n> that he/she can download a snapshot in the format he/she prefers.\n> The site-wide or project configuration now gives a list of formats\n> to offer, and the \"_snapshot_\" link is replaced with, say,\n> \"snapshot (_tbz2_ _zip_)\".\n> \n> - Fix out-of-date \"tarball\" -> \"archive\" in comment.\n> \n> Alert for gitweb site administrators: This patch changes the format of\n> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of three\n> pieces of information about a single format to a list of one or more formats\n> you wish to offer from the set ('tgz', 'tbz2', 'zip'). Update your\n> gitweb_config.perl appropriately.\n\nQuite nice and I think needed refactoring of a snapshot code. Nevertheless\nI have some comments on the changes introduced by this patch; not only\nchange to gitweb_config.perl is needed (which gitweb admin has control\nover), but also repo config for individual repositories might need to\nbe changed (which gitweb admin might not have control over, and which is\nmuch harder to do).\n\n> +# information about snapshot formats that gitweb is capable of serving\n> +# name => [mime type, filename suffix, --format for git-archive,\n> +# compressor command suffix]\n> +our %known_snapshot_formats = (\n> + 'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n> + 'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n> + 'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n> +);\n\nFirst, is full mimetype really needed? Earlier code assumed that mimetype\nfor snapshot is of the form of application/, and it provided\nonly .\n\nSecond, I'd rather have 'gzip' and 'bzip2' aliases to 'tgz' and 'tbz2',\nso the old config continues to work. I can see that it would be hard\nto do without special-casing code, or changing the assumption that list\nof default available snapshot formats is keys of above hash.\n\n> - # and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n> + # and in project config, a comma-separated list of formats or \"none\"\n> + # to disable. Example: gitweb.snapshot = tbz2,zip;\n\nI would relax the syntax, so \"tbz2, zip\" would also work, or even\n\"tbz2 zip\". I'd like for old config to also work, meaning that \"gzip\"\nwould be the same as \"tgz\" and \"bzip2\" as \"tbz2\".\n\n> - if ($val eq 'gzip') {\n> - return ('x-gzip', 'gz', 'gzip');\n> - } elsif ($val eq 'bzip2') {\n> - return ('x-bzip2', 'bz2', 'bzip2');\n> - } elsif ($val eq 'zip') {\n> - return ('x-zip', 'zip', '');\n> - } elsif ($val eq 'none') {\n> - return ();\n\nVery nice getting rid of this swith-like statement...\n\n> + if ($val) {\n> + @fmts = ($val eq 'none' ? () : split /,/, $val);\n\n... but I would relax this regexp.\n\n> +# Generates undef or something like \"snapshot (tbz2 zip)\", linked.\n> +# Pass the hash.\n> +sub format_snapshot_links {\n> + my ($hash) = @_;\n> + my @snapshot_fmts = gitweb_check_feature('snapshot');\n> + if (@snapshot_fmts) {\n> + return \"snapshot (\" . join(' ', map $cgi->a(\n> + {-href => href(action=>\"snapshot\", hash=>$hash, snapshot_format=>$_)}, \"$_\"),\n> + @snapshot_fmts)\n> + . \")\";\n> + } else {\n> + return undef;\n> + }\n> +}\n\nNice separation into subroutine.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"47045","messageId":"200707111755.50018.jnareb@gmail.com","threadId":"8762","inReplyTo":"7vir8w6inf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-11T15:55:49Z","receivedAt":"2007-07-11T15:55:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 7 Jul 2007, Junio C Hamano wrote:\n> Matt McCutchen writes:\n\n>> +# information about snapshot formats that gitweb is capable of serving\n>> +# name => [mime type, filename suffix, --format for git-archive,\n>> +# compressor command suffix]\n>> +our %known_snapshot_formats = (\n>> +\t'tgz' => ['application/x-gzip' , '.tar.gz' , 'tar', '| gzip' ],\n>> +\t'tbz2' => ['application/x-bzip2', '.tar.bz2', 'tar', '| bzip2'],\n>> +\t'zip' => ['application/zip' , '.zip' , 'zip', '' ],\n>> +);\n\n> This is a very nice clean-up, and I agree we should go this\n> route in the longer term.\n\nI agree that is a nice cleanup.\n\nI'm not sure if we want to store whole 'application/x-gzip' or only\n'x-gzip' part of mime type, and if we want to store compressor as\n'| gzip' or simply as 'gzip'.\n \n> This however will break people's existing gitweb configuration,\n> so if we were to do this it should be post 1.5.3, I would say.\n\nThis would break not only existing _gitweb_ configuration (when\ngitweb admin installs new gitweb it isn't that hard to correct\ngitweb config), but also git _repositories_ config: gitweb.snapshot\nno longer work as it worked before, for example neither 'gzip'\nnor 'bzip2' values work anymore ('zip' doesn't stop working).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"47075","messageId":"7vir8q4opc.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"200707111755.50018.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-11T21:26:07Z","receivedAt":"2007-07-11T21:26:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> I'm not sure if we want to store whole 'application/x-gzip' or only\n> 'x-gzip' part of mime type, and if we want to store compressor as\n> '| gzip' or simply as 'gzip'.\n> \n>> This however will break people's existing gitweb configuration,\n>> so if we were to do this it should be post 1.5.3, I would say.\n>\n> This would break not only existing _gitweb_ configuration (when\n> gitweb admin installs new gitweb it isn't that hard to correct\n> gitweb config), but also git _repositories_ config: gitweb.snapshot\n> no longer work as it worked before, for example neither 'gzip'\n> nor 'bzip2' values work anymore ('zip' doesn't stop working).\n\nI realized after seeing your other message on this patch that\nthis can be done while retaining backward compatibility, as you\nsuggested. Matt, does Jakub's suggestion make sense to you?\n"},{"id":"47099","messageId":"3bbc18d20707111815i1de3cb35sadfa316ddee7f3f6@mail.gmail.com","threadId":"8762","inReplyTo":"7vir8q4opc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-12T01:15:45Z","receivedAt":"2007-07-12T01:15:45Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On 7/11/07, Junio C Hamano wrote:\n> Jakub Narebski writes:\n>\n> > I'm not sure if we want to store whole 'application/x-gzip' or only\n> > 'x-gzip' part of mime type, and if we want to store compressor as\n> > '| gzip' or simply as 'gzip'.\n\nStoring only 'x-gzip' assumes that all archive formats have MIME types\nbeginning with 'application/'. Even if this assumption is justified\nby the MIME specification, I felt it was inappropriate to code it into\ngitweb. The advantage of '| gzip' is that the lack of a compressor is\nnot a special case. This is why I wrote %known_snapshot_formats the\nway I did, but of course you all are welcome to overrule me.\n\n> > This would break not only existing _gitweb_ configuration (when\n> > gitweb admin installs new gitweb it isn't that hard to correct\n> > gitweb config), but also git _repositories_ config: gitweb.snapshot\n> > no longer work as it worked before, for example neither 'gzip'\n> > nor 'bzip2' values work anymore ('zip' doesn't stop working).\n>\n> I realized after seeing your other message on this patch that\n> this can be done while retaining backward compatibility, as you\n> suggested. Matt, does Jakub's suggestion make sense to you?\n\nIt's not clear to me what the suggestion is: offer format names 'gzip'\nand 'bzip2' instead of 'tgz' and 'tbz2', or in addition to them, or\nwhat? I prefer 'tgz' and 'tbz2' because they carry more information\nand are properly analogous to 'zip', so I don't want to offer 'gzip'\nand 'bzip2' instead of them. Furthermore, I would like the user to\nsee 'snapshot (tgz tbz2)' even if the repository owner wrote 'gzip\nbzip2', so just adding two rows to %known_snapshot_formats is\ninsufficient. Either an additional column could be added to\n%known_snapshot_formats for the display name, or 'gzip' and 'bzip2'\ncould be specified as aliases in %known_snapshot_formats and\nfeature_snapshot could be taught to resolve them. I would prefer the\nsecond option; shall I implement it?\n\nIt would be possible to make the gitweb site configuration\nbackward-compatible too; here's one possible approach. On startup,\ngitweb would check whether $feature{'snapshot'}{'default'} is a\nthree-element list that appears to be in the old format. If so, it\nwould save the settings in $known_snapshot_formats{'default'} and then\nset $feature{'snapshot'}{'default'} = 'default' . This is a hack; is\nit justified by the compatibility benefit?\n\nMatt\n"},{"id":"47139","messageId":"200707121307.03612.jnareb@gmail.com","threadId":"8762","inReplyTo":"3bbc18d20707111815i1de3cb35sadfa316ddee7f3f6@mail.gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-12T11:07:03Z","receivedAt":"2007-07-12T11:07:03Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 12 July 2007, Matt McCutchen wrote:\n> On 7/11/07, Junio C Hamano wrote:\n>> Jakub Narebski writes:\n>>\n>>> I'm not sure if we want to store whole 'application/x-gzip' or only\n>>> 'x-gzip' part of mime type, and if we want to store compressor as\n>>> '| gzip' or simply as 'gzip'.\n> \n> Storing only 'x-gzip' assumes that all archive formats have MIME types\n> beginning with 'application/'. Even if this assumption is justified\n> by the MIME specification, I felt it was inappropriate to code it into\n> gitweb. \n\nGood argument. Besides, now we use it only in one place, but if we\nwould in the future use it in other place having full mimetype would\nmake it easier.\n\n> The advantage of '| gzip' is that the lack of a compressor is \n> not a special case. This is why I wrote %known_snapshot_formats the\n> way I did, but of course you all are welcome to overrule me.\n\nI wrote about this because I'm thinking about replacing the few \npipelines we use in gitweb[*1*], including the snapshot one, with\nthe list form, which has the advantage of avoiding spawning the shell \n(performance) and not dealing with shell quoting of arguments (security \nand errors), like we did for simple calling of git commands to read \nfrom in the commit b9182987a80f7e820cbe1f8c7c4dc26f8586e8cd\n \"gitweb: Use list for of open for running git commands, thorougly\"\n\nThus I'd rather have list of extra commands and arguments instead of\npipe as a string, i.e. 'gzip' instead of '| gzip', and 'gzip', '-9'\ninstead of '| gzip -9'.\n\n[*1*] We currently use pipelines for snapshots which need external \ncompressor, like tgz and tbz2, and for pickaxe search.\n\n>>> This would break not only existing _gitweb_ configuration (when\n>>> gitweb admin installs new gitweb it isn't that hard to correct\n>>> gitweb config), but also git _repositories_ config: gitweb.snapshot\n>>> no longer work as it worked before, for example neither 'gzip'\n>>> nor 'bzip2' values work anymore ('zip' doesn't stop working).\n>>\n>> I realized after seeing your other message on this patch that\n>> this can be done while retaining backward compatibility, as you\n>> suggested. Matt, does Jakub's suggestion make sense to you?\n> \n> It's not clear to me what the suggestion is: offer format names 'gzip'\n> and 'bzip2' instead of 'tgz' and 'tbz2', or in addition to them, or\n> what? I prefer 'tgz' and 'tbz2' because they carry more information\n> and are properly analogous to 'zip', so I don't want to offer 'gzip'\n> and 'bzip2' instead of them. Furthermore, I would like the user to\n> see 'snapshot (tgz tbz2)' even if the repository owner wrote 'gzip\n> bzip2', so just adding two rows to %known_snapshot_formats is\n> insufficient. Either an additional column could be added to\n> %known_snapshot_formats for the display name, or 'gzip' and 'bzip2'\n> could be specified as aliases in %known_snapshot_formats and\n> feature_snapshot could be taught to resolve them. I would prefer the\n> second option; shall I implement it?\n\nI also prefer the second option, perhaps as simple as 'gzip' => 'tbz'\nand 'bzip2' => 'tbz2', and of course accompaning code to deal with this.\n\nAs to \"display name\" column: I'm not sure if for example 'tar.gz' \ninstead of 'tgz' would be not easier to understand.\n\n> It would be possible to make the gitweb site configuration\n> backward-compatible too; here's one possible approach. On startup,\n> gitweb would check whether $feature{'snapshot'}{'default'} is a\n> three-element list that appears to be in the old format. If so, it\n> would save the settings in $known_snapshot_formats{'default'} and then\n> set $feature{'snapshot'}{'default'} = 'default' . This is a hack; is\n> it justified by the compatibility benefit?\n\nIf you implement 'gzip' and 'bzip2' as aliases, it could be as simple\nas just taking last non-false element of array if the array has more\nthan one element. But I'm not sure if it is worth it.\n\nBut we should probably error out with some error message on \nincompatibile gitweb site configuration; I'm not sure...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"47657","messageId":"3bbc18d20707171103q262eaa8amb319ca9f835dbf67@mail.gmail.com","threadId":"8762","inReplyTo":"200707121307.03612.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-17T18:03:21Z","receivedAt":"2007-07-17T18:03:21Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On 7/12/07, Jakub Narebski wrote:\n> > The advantage of '| gzip' is that the lack of a compressor is\n> > not a special case. This is why I wrote %known_snapshot_formats the\n> > way I did, but of course you all are welcome to overrule me.\n>\n> I wrote about this because I'm thinking about replacing the few\n> pipelines we use in gitweb[*1*], including the snapshot one, with\n> the list form, which has the advantage of avoiding spawning the shell\n> (performance) and not dealing with shell quoting of arguments (security\n> and errors), like we did for simple calling of git commands to read\n> from in the commit b9182987a80f7e820cbe1f8c7c4dc26f8586e8cd\n> \"gitweb: Use list for of open for running git commands, thorougly\"\n>\n> Thus I'd rather have list of extra commands and arguments instead of\n> pipe as a string, i.e. 'gzip' instead of '| gzip', and 'gzip', '-9'\n> instead of '| gzip -9'.\n>\n> [*1*] We currently use pipelines for snapshots which need external\n> compressor, like tgz and tbz2, and for pickaxe search.\n\nOK, I changed the extra commands to list form. The field is now a\nreference to the compressor argv like ['gzip'] or undef if there is no\ncompressor.\n\n> I also prefer the second option, perhaps as simple as 'gzip' => 'tbz'\n> and 'bzip2' => 'tbz2', and of course accompaning code to deal with this.\n>\n> As to \"display name\" column: I'm not sure if for example 'tar.gz'\n> instead of 'tgz' would be not easier to understand.\n\nI went ahead and added both aliases and display names (currently\n'tar.gz', 'tar.bz2', and 'zip'). Aliases are resolved in\n&feature_snapshot for repository configuration but not for side-wide\nconfiguration because then aliases in side-wide configuration would be\nresolved only if override is on, which would be weird. For the same\nreason, I changed the filtering out of unknown formats in\n&feature_snapshot to apply only to repository configuration.\n\n> > It would be possible to make the gitweb site configuration\n> > backward-compatible too; here's one possible approach. On startup,\n> > gitweb would check whether $feature{'snapshot'}{'default'} is a\n> > three-element list that appears to be in the old format. If so, it\n> > would save the settings in $known_snapshot_formats{'default'} and then\n> > set $feature{'snapshot'}{'default'} = 'default' . This is a hack; is\n> > it justified by the compatibility benefit?\n>\n> If you implement 'gzip' and 'bzip2' as aliases, it could be as simple\n> as just taking last non-false element of array if the array has more\n> than one element.\n\nNo, because it must be possible for the site default to consist of\nmultiple formats. It would be possible to take all elements that are\nrecognized as snapshot formats and ignore the others, but I would like\nto be able to distinguish between \"formats\" that are part of a legacy\nspecification and completely bogus formats so that we can issue a\nwarning or error for the latter.\n\n> But I'm not sure if it is worth it.\n\nAt this point I am leaning against backward compatibility for the site\nconfiguration, and if I do implement it, I would just recognize the\nthree exact specifications that currently appear in feature_snapshot.\nRecognizing other legacy specifications is iffy in the first place,\nand handling them would require the hack I described.\n\nI will send the revised patch soon.\n\nMatt\n"},{"id":"47663","messageId":"1184699486.9831.7.camel@mattlaptop2","threadId":"8762","inReplyTo":"3bbc18d20707171103q262eaa8amb319ca9f835dbf67@mail.gmail.com","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-17T19:11:26Z","receivedAt":"2007-07-17T19:11:26Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and if more than one format is offered, the \"_snapshot_\"\n link becomes something like \"snapshot (_tar.bz2_ _zip_)\".\n\n- If only one format is offered, a tooltip on the \"_snapshot_\" link\n tells the user what it is.\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nAlert for gitweb site administrators: This patch changes the format of\n$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\nthree pieces of information about a single format to a list of one or\nmore formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\nUpdate your gitweb_config.perl appropriately. The preferred names for\ngitweb.snapshot in repository configuration have also changed from\n'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still\nrecognized for compatibility.\n\nSigned-off-by: Matt McCutchen \n---\n\nChanges since the previous revision of the patch:\n\n- Added display names.\n- Changed compressor command line to list form.\n- Added compatibility format aliases for repository configuration.\n- Tweaked filtering of unknown formats to apply only to repository\n configuration.\n- Reformatted format_snapshot_links and added/modified comments to make it much\n easier to understand.\n- When a single snapshot format is offered, added a tooltip showing the format\n to the \"snapshot\" link. This helps Junio's hypothetical end user without\n using additional screen real estate.\n\nI thought of another incompatibility: previously bookmarked snapshot\nURLs will no longer work because they lack the new \"sf\" parameter. I\ndon't care about this; do any of you?\n\nIs there anything else I need to do to the patch? If not, how soon is\nit likely to be committed? I'm guessing I missed the boat on git 1.5.3.\n\nMatt\n\n gitweb/gitweb.perl | 134 +++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 86 insertions(+), 48 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c8ba3a2..f17c983 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -104,6 +104,22 @@ our $mimetypes_file = undef;\n # could be even 'utf-8' for the old behavior)\n our $fallback_encoding = 'latin1';\n \n+# information about snapshot formats that gitweb is capable of serving\n+# name => [display name, mime type, filename suffix, --format for git-archive,\n+# [compressor command and arguments] | undef]\n+our %known_snapshot_formats = (\n+\t'tgz' => ['tar.gz' , 'application/x-gzip' , '.tar.gz' , 'tar', ['gzip' ]],\n+\t'tbz2' => ['tar.bz2', 'application/x-bzip2', '.tar.bz2', 'tar', ['bzip2']],\n+\t'zip' => ['zip', 'application/x-zip' , '.zip' , 'zip', undef ],\n+);\n+\n+# Aliases so we understand old gitweb.snapshot values in repository\n+# configuration.\n+our %known_snapshot_format_aliases = (\n+\t'gzip' => 'tgz' ,\n+\t'bzip2' => 'tbz2',\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -134,20 +150,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -246,28 +264,17 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /,/, $val);\n+\t\t@fmts = map $known_snapshot_format_aliases{$_} || $_, @fmts;\n+\t\t@fmts = grep exists $known_snapshot_formats{$_}, @fmts;\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn @fmts;\n }\n \n sub feature_grep {\n@@ -563,6 +570,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1257,6 +1265,39 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"_snapshot_\" or \"snapshot (_tbz2_ _zip_)\",\n+# linked. Pass the hash of the tree/commit to snapshot.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tmy $num_fmts = @snapshot_fmts;\n+\tif ($num_fmts > 1) {\n+\t\t# A parenthesized list of links bearing format names.\n+\t\treturn \"snapshot (\" . join(' ', map\n+\t\t\t$cgi->a({\n+\t\t\t\t-href => href(\n+\t\t\t\t\taction=>\"snapshot\",\n+\t\t\t\t\thash=>$hash,\n+\t\t\t\t\tsnapshot_format=>$_\n+\t\t\t\t)\n+\t\t\t}, $known_snapshot_formats{$_}[0]) # the display name\n+\t\t, @snapshot_fmts) . \")\";\n+\t} elsif ($num_fmts == 1) {\n+\t\t# A single \"snapshot\" link whose tooltip bears the format name.\n+\t\tmy ($fmt) = @snapshot_fmts;\n+\t\treturn $cgi->a({\n+\t\t\t\t-href => href(\n+\t\t\t\t\taction=>\"snapshot\",\n+\t\t\t\t\thash=>$hash,\n+\t\t\t\t\tsnapshot_format=>$fmt\n+\t\t\t\t),\n+\t\t\t\t-title => \"in format: $known_snapshot_formats{$fmt}[0]\" # the display name\n+\t\t\t}, \"snapshot\");\n+\t} else { # $num_fmts == 0\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3321,8 +3362,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3349,8 +3388,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4132,8 +4172,6 @@ sub git_blob {\n }\n \n sub git_tree {\n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tif (!defined $hash_base) {\n \t\t$hash_base = \"HEAD\";\n \t}\n@@ -4167,11 +4205,10 @@ sub git_tree {\n \t\t\t\t hash_base=>\"HEAD\", file_name=>$file_name)},\n \t\t\t\t \"HEAD\"),\n \t\t}\n-\t\tif ($have_snapshot) {\n+\t\tmy $snapshot_links = format_snapshot_links($hash);\n+\t\tif (defined $snapshot_links) {\n \t\t\t# FIXME: Should be available when we have no hash base as well.\n-\t\t\tpush @views_nav,\n-\t\t\t\t$cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)},\n-\t\t\t\t \"snapshot\");\n+\t\t\tpush @views_nav, $snapshot_links;\n \t\t}\n \t\tgit_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);\n@@ -4235,11 +4272,16 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n+\tmy ($dispname, $ctype, $suffix, $ga_format, $compressor_argv) =\n+\t\t@{$known_snapshot_formats{$format}};\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n@@ -4252,16 +4294,14 @@ sub git_snapshot {\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n+\t$filename .= \"-$hash$suffix\";\n+\t$cmd = \"$git archive --format=$ga_format --prefix=\\'$name\\'/ $hash\";\n+\tif (defined $compressor_argv) {\n+\t\t$cmd .= ' | ' . join ' ', @$compressor_argv;\n \t}\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => \"$ctype\",\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n@@ -4390,8 +4430,6 @@ sub git_commit {\n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $co{'id'});\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tgit_header_html(undef, $expires);\n \tgit_print_page_nav('commit', '',\n \t $hash, $co{'tree'}, $hash,\n@@ -4430,9 +4468,9 @@ sub git_commit {\n \t \"\" .\n \t $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$hash)},\n \t \"tree\");\n-\tif ($have_snapshot) {\n-\t\tprint \" | \" .\n-\t\t $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)}, \"snapshot\");\n+\tmy $snapshot_links = format_snapshot_links($hash);\n+\tif (defined $snapshot_links) {\n+\t\tprint \" | \" . $snapshot_links;\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-- \n1.5.3.rc2.6.gf09b\n"},{"id":"47782","messageId":"200707190140.05235.jnareb@gmail.com","threadId":"8762","inReplyTo":"1184699486.9831.7.camel@mattlaptop2","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-18T23:40:03Z","receivedAt":"2007-07-18T23:40:03Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 17 July 2007, Matt McCutchen napisał:\n> - Centralize knowledge about snapshot formats (mime types, extensions,\n> commands) in %known_snapshot_formats and improve how some of that\n> information is specified. In particular, zip files are no longer a\n> special case.\n> \n> - Add support for offering multiple snapshot formats to the user so\n> that he/she can download a snapshot in the format he/she prefers.\n> The site-wide or project configuration now gives a list of formats\n> to offer, and if more than one format is offered, the \"_snapshot_\"\n> link becomes something like \"snapshot (_tar.bz2_ _zip_)\".\n> \n> - If only one format is offered, a tooltip on the \"_snapshot_\" link\n> tells the user what it is.\n\nNice idea.\n\n> - Fix out-of-date \"tarball\" -> \"archive\" in comment.\n> \n> Alert for gitweb site administrators: This patch changes the format of\n> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\n> three pieces of information about a single format to a list of one or\n> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\n> Update your gitweb_config.perl appropriately. The preferred names for\n> gitweb.snapshot in repository configuration have also changed from\n> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still\n> recognized for compatibility.\n\nThis alert/warning should probably be put in RelNotes for when it would\nbe in git.git\n\n> Signed-off-by: Matt McCutchen \n> ---\n> \n> Changes since the previous revision of the patch:\n> \n> - Added display names.\n> - Changed compressor command line to list form.\n> - Added compatibility format aliases for repository configuration.\n> - Tweaked filtering of unknown formats to apply only to repository\n> configuration.\n> - Reformatted format_snapshot_links and added/modified comments to make it much\n> easier to understand.\n> - When a single snapshot format is offered, added a tooltip showing the format\n> to the \"snapshot\" link. This helps Junio's hypothetical end user without\n> using additional screen real estate.\n> \n> I thought of another incompatibility: previously bookmarked snapshot\n> URLs will no longer work because they lack the new \"sf\" parameter. I\n> don't care about this; do any of you?\n\nI think either having good error message, or using first format avaiable\nwould be good enough.\n\n> +# information about snapshot formats that gitweb is capable of serving\n> +# name => [display name, mime type, filename suffix, --format for git-archive,\n> +# [compressor command and arguments] | undef]\n> +our %known_snapshot_formats = (\n> +\t'tgz' => ['tar.gz' , 'application/x-gzip' , '.tar.gz' , 'tar', ['gzip' ]],\n> +\t'tbz2' => ['tar.bz2', 'application/x-bzip2', '.tar.bz2', 'tar', ['bzip2']],\n> +\t'zip' => ['zip', 'application/x-zip' , '.zip' , 'zip', undef ],\n> +);\n\nFirst I'm not sure if we want to do the way it had to be done when\nthose info was in the subfield of %feature hash, or to imitate %feature\nhash using instead:\n\n+our %known_snapshot_formats = (\n+\t'tgz' => {\n+\t\t'display' => 'tar.gz',\n+\t\t'mimetype' => 'application/x-gzip',\n+\t\t'suffix' => '.tar.gz',\n+\t\t'format' => 'tar',\n+\t\t'compressor' => ['gzip' ]},\n... \n\nwhich means that when using %known_snapshot_formats we don't have\nto remember for example which of the elements in array is mimetype,\nwhich display name, and which format to be passed to git-archive.\n\n\nSecond, I have thought that we might want to simply use the rest of\narray for the compressor and it's arguments instead of adding it as\nanonymous array reference (inner array). But this way we could in\nprincile add more pipelines... although I think it would be not useful.\nI'd rather have first option implemented, even if it does not allow for\nmultiple pipelines.\n\nThird, I think we don't need to say \"undef\" explicitely, I think.\n\"defined ('a')[1]\" returns the same as \"defined ('a', undef)[1]\".\n\n> +# Aliases so we understand old gitweb.snapshot values in repository\n> +# configuration.\n> +our %known_snapshot_format_aliases = (\n> +\t'gzip' => 'tgz' ,\n> +\t'bzip2' => 'tbz2',\n> +);\n\nGood idea, better than tring to fit it in %known_snapshot_formats.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"47791","messageId":"7vvech42nb.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"200707190140.05235.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-19T01:12:56Z","receivedAt":"2007-07-19T01:12:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> On Tue, 17 July 2007, Matt McCutchen napisał:\n> ...\n>> Alert for gitweb site administrators: This patch changes the format of\n>> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\n>> three pieces of information about a single format to a list of one or\n>> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\n>> Update your gitweb_config.perl appropriately. The preferred names for\n>> gitweb.snapshot in repository configuration have also changed from\n>> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still\n>> recognized for compatibility.\n>\n> This alert/warning should probably be put in RelNotes for when it would\n> be in git.git\n\nDoes anybody else worry about the backward imcompatibility, I\nwonder... List?\n\nI really hate to having to say something like that in the\nRelNotes. I do not think this is a good enough reason to break\nexisting configurations; I would not want to be defending that\nchange.\n\n>> I thought of another incompatibility: previously bookmarked snapshot\n>> URLs will no longer work because they lack the new \"sf\" parameter. I\n>> don't care about this; do any of you?\n>\n> I think either having good error message, or using first format avaiable\n> would be good enough.\n\nI doubt bookmarked snapshot URL would make sense to begin with,\nso this would be Ok.\n\nI am wondering if something like this patch (totally untested,\nmind you) to convert the old style %feature in configuration at\nthe site at runtime would be sufficient.\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f17c983..cdec4d0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -236,9 +236,39 @@ our %feature = (\n \t\t'default' => [0]},\n );\n \n+# Functions to convert values from older gitweb configuration\n+# into the current data format\n+sub gitweb_bc_feature_snapshot {\n+\tmy $def = $feature{'snapshot'}{'default'};\n+\t# Older definition was to have either undef (to disable), or\n+\t# a three-element array whose first element was content encoding\n+\t# without leading \"application/\".\n+\treturn if (ref $def ne 'ARRAY');\n+\tif (!defined $def->[0] && @$def == 1) {\n+\t\t# Disabled -- the new way to spell it is to have an empty\n+\t\t# arrayref.\n+\t\t$feature{'snapshot'}{'default'} = [];\n+\t\treturn;\n+\t}\n+\treturn if (@$def != 3);\n+\tfor ($def->[0]) {\n+\t\tif (/x-gzip/) {\n+\t\t\t$feature{'snapshot'}{'default'} = ['tgz'];\n+\t\t}\n+\t\tif (/x-bz2/) {\n+\t\t\t$feature{'snapshot'}{'default'} = ['tbz2'];\n+\t\t}\n+\t\tif (/x-zip/) {\n+\t\t\t$feature{'snapshot'}{'default'} = ['zip'];\n+\t\t}\n+\t}\n+}\n+\n sub gitweb_check_feature {\n \tmy ($name) = @_;\n \treturn unless exists $feature{$name};\n+\teval \"gitweb_bc_feature_$name()\";\n+\n \tmy ($sub, $override, @defaults) = (\n \t\t$feature{$name}{'sub'},\n \t\t$feature{$name}{'override'},\n"},{"id":"47802","messageId":"571532.59758.qm@web31813.mail.mud.yahoo.com","threadId":"8762","inReplyTo":"7vvech42nb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Luben Tuikov","fromEmail":"ltuikov@yahoo.com","sentAt":"2007-07-19T03:30:43Z","receivedAt":"2007-07-19T03:30:43Z","isPatch":true,"sender":{"key":"ltuikov@yahoo.com","avatar":null},"body":"--- Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > On Tue, 17 July 2007, Matt McCutchen napisał:\n> > ...\n> >> Alert for gitweb site administrators: This patch changes the format of\n> >> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\n> >> three pieces of information about a single format to a list of one or\n> >> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\n> >> Update your gitweb_config.perl appropriately. The preferred names for\n> >> gitweb.snapshot in repository configuration have also changed from\n> >> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still\n> >> recognized for compatibility.\n> >\n> > This alert/warning should probably be put in RelNotes for when it would\n> > be in git.git\n> \n> Does anybody else worry about the backward imcompatibility, I\n> wonder... List?\n\nI wouldn't mind an improvement in the snapshot area of gitweb.\nI wasn't really happy with the snapshot feature as it was originally\nimplemented, as it would generate a tar file with \".tar.bz2\"\nname extension, but the file was NOT bz2, and I had to always\nmanually rename, bz2, and rename back.\n\n> I really hate to having to say something like that in the\n> RelNotes. I do not think this is a good enough reason to break\n> existing configurations; I would not want to be defending that\n> change.\n> \n> >> I thought of another incompatibility: previously bookmarked snapshot\n> >> URLs will no longer work because they lack the new \"sf\" parameter. I\n> >> don't care about this; do any of you?\n> >\n> > I think either having good error message, or using first format avaiable\n> > would be good enough.\n> \n> I doubt bookmarked snapshot URL would make sense to begin with,\n> so this would be Ok.\n> \n> I am wondering if something like this patch (totally untested,\n> mind you) to convert the old style %feature in configuration at\n> the site at runtime would be sufficient.\n\n\"totally untested\" is a problem. Anything going into gitweb for\npublic consumption (master branch, next ok), should be completely\nand exhaustively tested.\n\n Luben\n\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index f17c983..cdec4d0 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -236,9 +236,39 @@ our %feature = (\n> \t\t'default' => [0]},\n> );\n> \n> +# Functions to convert values from older gitweb configuration\n> +# into the current data format\n> +sub gitweb_bc_feature_snapshot {\n> +\tmy $def = $feature{'snapshot'}{'default'};\n> +\t# Older definition was to have either undef (to disable), or\n> +\t# a three-element array whose first element was content encoding\n> +\t# without leading \"application/\".\n> +\treturn if (ref $def ne 'ARRAY');\n> +\tif (!defined $def->[0] && @$def == 1) {\n> +\t\t# Disabled -- the new way to spell it is to have an empty\n> +\t\t# arrayref.\n> +\t\t$feature{'snapshot'}{'default'} = [];\n> +\t\treturn;\n> +\t}\n> +\treturn if (@$def != 3);\n> +\tfor ($def->[0]) {\n> +\t\tif (/x-gzip/) {\n> +\t\t\t$feature{'snapshot'}{'default'} = ['tgz'];\n> +\t\t}\n> +\t\tif (/x-bz2/) {\n> +\t\t\t$feature{'snapshot'}{'default'} = ['tbz2'];\n> +\t\t}\n> +\t\tif (/x-zip/) {\n> +\t\t\t$feature{'snapshot'}{'default'} = ['zip'];\n> +\t\t}\n> +\t}\n> +}\n> +\n> sub gitweb_check_feature {\n> \tmy ($name) = @_;\n> \treturn unless exists $feature{$name};\n> +\teval \"gitweb_bc_feature_$name()\";\n> +\n> \tmy ($sub, $override, @defaults) = (\n> \t\t$feature{$name}{'sub'},\n> \t\t$feature{$name}{'override'},\n> \n> \n"},{"id":"47820","messageId":"200707190930.57019.jnareb@gmail.com","threadId":"8762","inReplyTo":"571532.59758.qm@web31813.mail.mud.yahoo.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-19T07:30:56Z","receivedAt":"2007-07-19T07:30:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Luben Tuikov wrote:\n\n> I wouldn't mind an improvement in the snapshot area of gitweb.\n> I wasn't really happy with the snapshot feature as it was originally\n> implemented, as it would generate a tar file with \".tar.bz2\"\n> name extension, but the file was NOT bz2, and I had to always\n> manually rename, bz2, and rename back.\n\nThis was a *bug*, but it is now corrected (in 9aa17573). Gitweb used \nContent-Encoding, which is meant for _transparent_ compression.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"47821","messageId":"513314.51284.qm@web31813.mail.mud.yahoo.com","threadId":"8762","inReplyTo":"200707190930.57019.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Luben Tuikov","fromEmail":"ltuikov@yahoo.com","sentAt":"2007-07-19T07:40:07Z","receivedAt":"2007-07-19T07:40:07Z","isPatch":true,"sender":{"key":"ltuikov@yahoo.com","avatar":null},"body":"--- Jakub Narebski wrote:\n> Luben Tuikov wrote:\n> \n> > I wouldn't mind an improvement in the snapshot area of gitweb.\n> > I wasn't really happy with the snapshot feature as it was originally\n> > implemented, as it would generate a tar file with \".tar.bz2\"\n> > name extension, but the file was NOT bz2, and I had to always\n> > manually rename, bz2, and rename back.\n> \n> This was a *bug*, but it is now corrected (in 9aa17573). Gitweb used \n> Content-Encoding, which is meant for _transparent_ compression.\n\nYeah, that's what I suspected, since there was nothing obviously\nwrong with the code.\n\nThanks for the fix.\n\n Luben\n"},{"id":"47859","messageId":"200707191105.19735.jnareb@gmail.com","threadId":"8762","inReplyTo":"7vvech42nb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-19T09:05:19Z","receivedAt":"2007-07-19T09:05:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 19 July 2007, Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>> On Tue, 17 July 2007, Matt McCutchen napisał:\n>> ...\n>>> Alert for gitweb site administrators: This patch changes the format of\n>>> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\n>>> three pieces of information about a single format to a list of one or\n>>> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\n>>> Update your gitweb_config.perl appropriately. The preferred names for\n>>> gitweb.snapshot in repository configuration have also changed from\n>>> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still\n>>> recognized for compatibility.\n>>\n>> This alert/warning should probably be put in RelNotes for when it would\n>> be in git.git\n> \n> Does anybody else worry about the backward imcompatibility, I\n> wonder... List?\n> \n> I really hate to having to say something like that in the\n> RelNotes. I do not think this is a good enough reason to break\n> existing configurations; I would not want to be defending that\n> change.\n[...]\n> I am wondering if something like this patch (totally untested,\n> mind you) to convert the old style %feature in configuration at\n> the site at runtime would be sufficient.\n\nWould it be sufficient to put above alert/warning in commit message,\nRelNotes and gitweb/INSTALL (or gitweb/README), and add rule to Makefile\nto convert old configuration, or at least check if GITWEB_CONFIG uses\nold snapshot configuration? This way if somebody is installing/upgrading\ngitweb by hand he/she would know what needs possibly to be changes, and\nif somebody uses \"make gitweb/gitweb.cgi\" he would get big fat warning,\nand info how to convert gitweb config.\n\nBy the way, I think it was a mistake to use different syntax in the\n%feature hash ([content-encoding, suffix, program]) than in repo config\noverride (name).\n\n\nBesides the proposed patch incurs performance penalty for all feature\nchecks, not only for snapshot. I think it could be solved by using\na hack of providing more aliases, so that 'gzip' (repo config) but\nalso 'x-gzip', 'gz' and 'gzip' (gitweb config) would be aliases to\n'tgz' snapshot, and we would perform \"uniq\" on the list of snapshot\nformats (assuming it is sorted). Or make 'x-gzip' and 'gz' aliases\ninto undef, so 'gzip' from old configuration would be aliased to the\nnew format name 'tgz'. What do you think about this?\n\nOoops, this has disadvantage of having to guess what could be put\nin the gitweb config regarding snapshot configuration, but I think we\ncould assume that only the values enumerated in the old feature_snapshot\nwould be used.\n\n\nAll said, I think it is a good change. I guess that gitweb admins would\nwant to provide both tgz/tar.gz archives for the Unix crowd, and zip\narchives for MS Windows users...\n\n\nP.S. I wonder why git-archive does not support tgz format. Git is linked\nto zlib, so...\n-- \nJakub Narebski\nPoland\n"},{"id":"47860","messageId":"200707191114.39553.jnareb@gmail.com","threadId":"8762","inReplyTo":"1184699486.9831.7.camel@mattlaptop2","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-19T09:14:39Z","receivedAt":"2007-07-19T09:14:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 17 July 2007, Matt McCutchen wrote:\n\n>  sub feature_snapshot {\n> -       my ($ctype, $suffix, $command) = @_;\n> +       my (@fmts) = @_;\n>  \n>         my ($val) = git_get_project_config('snapshot');\n>  \n> -       if ($val eq 'gzip') {\n> -               return ('x-gzip', 'gz', 'gzip');\n> -       } elsif ($val eq 'bzip2') {\n> -               return ('x-bzip2', 'bz2', 'bzip2');\n> -       } elsif ($val eq 'zip') {\n> -               return ('x-zip', 'zip', '');\n> -       } elsif ($val eq 'none') {\n> -               return ();\n> +       if ($val) {\n> +               @fmts = ($val eq 'none' ? () : split /,/, $val);\n> +               @fmts = map $known_snapshot_format_aliases{$_} || $_, @fmts;\n> +               @fmts = grep exists $known_snapshot_formats{$_}, @fmts;\n>         }\n>  \n> -       return ($ctype, $suffix, $command);\n> -}\n\nI would use more permissive (be forbidding in what you accept) regexp\nto split gitweb.snapshot value into list of snapshot formats, so one\ncould use \"tgz, zip\", or perhaps even \"tgz zip\", and not only \"tgz,zip\"\n(no whitespace possible). For example\n\n+ @fmts = ($val eq 'none' ? () : split(/\\s*,\\s*/, $val));\n\nor even\n\n+ @fmts = ($val eq 'none' ? () : split(/\\s*[,\\s]\\s*/, $val));\n\nto allow \"tgz zip\".\n\n\nYour regexp for \"tgz, zip\" would get 'tgz', ' zip' (with leading space)\nas snapshot formats to use.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"47921","messageId":"7vbqe71yvh.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"200707191105.19735.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-20T04:29:38Z","receivedAt":"2007-07-20T04:29:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> On Thu, 19 July 2007, Junio C Hamano wrote:\n> ...\n>> I really hate to having to say something like that in the\n>> RelNotes. I do not think this is a good enough reason to break\n>> existing configurations; I would not want to be defending that\n>> change.\n>\n> Would it be sufficient to put above alert/warning in commit message,\n> RelNotes and gitweb/INSTALL (or gitweb/README), and add rule to Makefile\n> to convert old configuration, or at least check if GITWEB_CONFIG uses\n> old snapshot configuration? This way if somebody is installing/upgrading\n> gitweb by hand he/she would know what needs possibly to be changes, and\n> if somebody uses \"make gitweb/gitweb.cgi\" he would get big fat warning,\n> and info how to convert gitweb config.\n>\n> By the way, I think it was a mistake to use different syntax in the\n> %feature hash ([content-encoding, suffix, program]) than in repo config\n> override (name).\n\nThat's true. I'd rather see this polished a bit more before\napplying.\n"},{"id":"48068","messageId":"200707220130.28516.jnareb@gmail.com","threadId":"8762","inReplyTo":"1184699486.9831.7.camel@mattlaptop2","subject":"[PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-21T23:30:27Z","receivedAt":"2007-07-21T23:30:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Matt McCutchen \nSubject: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats\n\n- Centralize knowledge about snapshot formats (mime types, extensions,\n commands) in %known_snapshot_formats and improve how some of that\n information is specified. In particular, zip files are no longer a\n special case.\n\n- Add support for offering multiple snapshot formats to the user so\n that he/she can download a snapshot in the format he/she prefers.\n The site-wide or project configuration now gives a list of formats\n to offer, and if more than one format is offered, the \"_snapshot_\"\n link becomes something like \"snapshot (_tar.bz2_ _zip_)\".\n\n- If only one format is offered, a tooltip on the \"_snapshot_\" link\n tells the user what it is.\n\n- Fix out-of-date \"tarball\" -> \"archive\" in comment.\n\nAlert for gitweb site administrators: This patch changes the format of\n$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of\nthree pieces of information about a single format to a list of one or\nmore formats you wish to offer from the set ('tgz', 'tbz2', 'zip').\nUpdate your gitweb_config.perl appropriately. There was taken care\nfor old-style gitweb configuration to work as it used to, but this\nbackward compatibility works only for the values which correspond to\ngitweb.snapshot values of 'gzip', 'bzip2' and 'zip', i.e.\n ['x-gzip', 'gz', 'gzip']\n ['x-bzip2', 'bz2', 'bzip2']\n ['x-zip', 'zip', '']\n\nThe preferred names for gitweb.snapshot in repository configuration\nhave also changed from 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but\nthe old names are still recognized for compatibility.\n\nSigned-off-by: Matt McCutchen \nSigned-off-by: Jakub Narebski \n---\nThis tries to address concerns that was raised in this thread.\nNamely:\n - hash is used instead of fixed order of mixed arguments array\n - it tries to be backward compatibile wrt. gitweb config\n\n gitweb/gitweb.perl | 166 ++++++++++++++++++++++++++++++++++++----------------\n 1 files changed, 116 insertions(+), 50 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6754e26..c4f8824 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -114,6 +114,49 @@ our $fallback_encoding = 'latin1';\n # - one might want to include '-B' option, e.g. '-B', '-M'\n our @diff_opts = ('-M'); # taken from git_commit\n \n+# information about snapshot formats that gitweb is capable of serving\n+our %known_snapshot_formats = (\n+\t# name => {\n+\t# \t'display' => display name,\n+\t# \t'type' => mime type,\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#\n+\t'tgz' => {\n+\t\t'display' => 'tar.gz',\n+\t\t'type' => 'application/x-gzip',\n+\t\t'suffix' => '.tar.gz',\n+\t\t'format' => 'tar',\n+\t\t'compressor' => ['gzip']},\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+\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+);\n+\n+# Aliases so we understand old gitweb.snapshot values in repository\n+# configuration.\n+our %known_snapshot_format_aliases = (\n+\t'gzip' => 'tgz',\n+\t'bzip2' => 'tbz2',\n+\n+\t# backward compatibility: legacy gitweb config support\n+\t'x-gzip' => undef, 'gz' => undef,\n+\t'x-bzip2' => undef, 'bz2' => undef,\n+\t'x-zip' => undef, '' => undef,\n+);\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -144,20 +187,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [0]},\n \n-\t# Enable the 'snapshot' link, providing a compressed tarball of any\n+\t# Enable the 'snapshot' link, providing a compressed archive of any\n \t# tree. This can potentially generate high traffic if you have large\n \t# project.\n \n+\t# Value is a list of formats defined in %known_snapshot_formats that\n+\t# you wish to offer.\n \t# To disable system wide have in $GITWEB_CONFIG\n-\t# $feature{'snapshot'}{'default'} = [undef];\n+\t# $feature{'snapshot'}{'default'} = [];\n \t# To have project specific config enable override in $GITWEB_CONFIG\n \t# $feature{'snapshot'}{'override'} = 1;\n-\t# and in project config gitweb.snapshot = none|gzip|bzip2|zip;\n+\t# and in project config, a comma-separated list of formats or \"none\"\n+\t# to disable. Example: gitweb.snapshot = tbz2,zip;\n \t'snapshot' => {\n \t\t'sub' => \\&feature_snapshot,\n \t\t'override' => 0,\n-\t\t# => [content-encoding, suffix, program]\n-\t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\t\t'default' => ['tgz']},\n \n \t# Enable text search, which will list the commits which match author,\n \t# committer or commit text to a given string. Enabled by default.\n@@ -256,28 +301,19 @@ sub feature_blame {\n }\n \n sub feature_snapshot {\n-\tmy ($ctype, $suffix, $command) = @_;\n+\tmy (@fmts) = @_;\n \n \tmy ($val) = git_get_project_config('snapshot');\n \n-\tif ($val eq 'gzip') {\n-\t\treturn ('x-gzip', 'gz', 'gzip');\n-\t} elsif ($val eq 'bzip2') {\n-\t\treturn ('x-bzip2', 'bz2', 'bzip2');\n-\t} elsif ($val eq 'zip') {\n-\t\treturn ('x-zip', 'zip', '');\n-\t} elsif ($val eq 'none') {\n-\t\treturn ();\n+\tif ($val) {\n+\t\t@fmts = ($val eq 'none' ? () : split /\\s*[,\\s]\\s*/, $val);\n+\t\t@fmts = grep { defined } map {\n+\t\t\texists $known_snapshot_format_aliases{$_} ?\n+\t\t\t\t$known_snapshot_format_aliases{$_} : $_ } @fmts;\n+\t\t@fmts = grep(exists $known_snapshot_formats{$_}, @fmts);\n \t}\n \n-\treturn ($ctype, $suffix, $command);\n-}\n-\n-sub gitweb_have_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\n-\treturn $have_snapshot;\n+\treturn @fmts;\n }\n \n sub feature_grep {\n@@ -563,6 +599,7 @@ sub href(%) {\n \t\torder => \"o\",\n \t\tsearchtext => \"s\",\n \t\tsearchtype => \"st\",\n+\t\tsnapshot_format => \"sf\",\n \t);\n \tmy %mapping = @mapping;\n \n@@ -1257,6 +1294,39 @@ sub format_diff_line {\n \treturn \"
\" . esc_html($line, -nbsp=>1) . \"
\\n\";\n }\n \n+# Generates undef or something like \"_snapshot_\" or \"snapshot (_tbz2_ _zip_)\",\n+# linked. Pass the hash of the tree/commit to snapshot.\n+sub format_snapshot_links {\n+\tmy ($hash) = @_;\n+\tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\tmy $num_fmts = @snapshot_fmts;\n+\tif ($num_fmts > 1) {\n+\t\t# A parenthesized list of links bearing format names.\n+\t\treturn \"snapshot (\" . join(' ', map\n+\t\t\t$cgi->a({\n+\t\t\t\t-href => href(\n+\t\t\t\t\taction=>\"snapshot\",\n+\t\t\t\t\thash=>$hash,\n+\t\t\t\t\tsnapshot_format=>$_\n+\t\t\t\t)\n+\t\t\t}, $known_snapshot_formats{$_}{'display'})\n+\t\t, @snapshot_fmts) . \")\";\n+\t} elsif ($num_fmts == 1) {\n+\t\t# A single \"snapshot\" link whose tooltip bears the format name.\n+\t\tmy ($fmt) = @snapshot_fmts;\n+\t\treturn $cgi->a({\n+\t\t\t\t-href => href(\n+\t\t\t\t\taction=>\"snapshot\",\n+\t\t\t\t\thash=>$hash,\n+\t\t\t\t\tsnapshot_format=>$fmt\n+\t\t\t\t),\n+\t\t\t\t-title => \"in format: $known_snapshot_formats{$fmt}{'display'}\"\n+\t\t\t}, \"snapshot\");\n+\t} else { # $num_fmts == 0\n+\t\treturn undef;\n+\t}\n+}\n+\n ## ----------------------------------------------------------------------\n ## git utility subroutines, invoking git commands\n \n@@ -3321,8 +3391,6 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \t$from = 0 unless defined $from;\n \t$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);\n \n@@ -3349,8 +3417,9 @@ sub git_shortlog_body {\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif ($have_snapshot) {\n-\t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n+\t\tmy $snapshot_links = format_snapshot_links($commit);\n+\t\tif (defined $snapshot_links) {\n+\t\t\tprint \" | \" . $snapshot_links;\n \t\t}\n \t\tprint \"\\n\" .\n \t\t \"\\n\";\n@@ -4132,8 +4201,6 @@ sub git_blob {\n }\n \n sub git_tree {\n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tif (!defined $hash_base) {\n \t\t$hash_base = \"HEAD\";\n \t}\n@@ -4167,11 +4234,10 @@ sub git_tree {\n \t\t\t\t hash_base=>\"HEAD\", file_name=>$file_name)},\n \t\t\t\t \"HEAD\"),\n \t\t}\n-\t\tif ($have_snapshot) {\n+\t\tmy $snapshot_links = format_snapshot_links($hash);\n+\t\tif (defined $snapshot_links) {\n \t\t\t# FIXME: Should be available when we have no hash base as well.\n-\t\t\tpush @views_nav,\n-\t\t\t\t$cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)},\n-\t\t\t\t \"snapshot\");\n+\t\t\tpush @views_nav, $snapshot_links;\n \t\t}\n \t\tgit_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);\n@@ -4235,33 +4301,36 @@ sub git_tree {\n }\n \n sub git_snapshot {\n-\tmy ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');\n-\tmy $have_snapshot = (defined $ctype && defined $suffix);\n-\tif (!$have_snapshot) {\n-\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\n+\tmy $format = $cgi->param('sf');\n+\tunless ($format =~ m/[a-z0-9]+/\n+\t\t&& exists($known_snapshot_formats{$format})\n+\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n \n \tif (!defined $hash) {\n \t\t$hash = git_get_head_hash($project);\n \t}\n \n-\tmy $git = git_cmd_str();\n+\tmy $git_command = git_cmd_str();\n \tmy $name = $project;\n \t$name =~ s,([^/])/*\\.git$,$1,;\n \t$name = basename($name);\n \tmy $filename = to_utf8($name);\n \t$name =~ s/\\047/\\047\\\\\\047\\047/g;\n \tmy $cmd;\n-\tif ($suffix eq 'zip') {\n-\t\t$filename .= \"-$hash.$suffix\";\n-\t\t$cmd = \"$git archive --format=zip --prefix=\\'$name\\'/ $hash\";\n-\t} else {\n-\t\t$filename .= \"-$hash.tar.$suffix\";\n-\t\t$cmd = \"$git archive --format=tar --prefix=\\'$name\\'/ $hash | $command\";\n+\t$filename .= \"-$hash$known_snapshot_formats{$format}{'suffix'}\";\n+\t$cmd = \"$git_command archive \" .\n+\t\t\"--format=$known_snapshot_formats{$format}{'format'}\" .\n+\t\t\"--prefix=\\'$name\\'/ $hash\";\n+\tif (exists $known_snapshot_formats{$format}{'compressor'}) {\n+\t\t$cmd .= ' | ' . join ' ', @{$known_snapshot_formats{$format}{'compressor'}};\n \t}\n \n \tprint $cgi->header(\n-\t\t-type => \"application/$ctype\",\n+\t\t-type => $known_snapshot_formats{$format}{'type'},\n \t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n \t\t-status => '200 OK');\n \n@@ -4271,7 +4340,6 @@ sub git_snapshot {\n \tprint <$fd>;\n \tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n \tclose $fd;\n-\n }\n \n sub git_log {\n@@ -4390,8 +4458,6 @@ sub git_commit {\n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $co{'id'});\n \n-\tmy $have_snapshot = gitweb_have_snapshot();\n-\n \tgit_header_html(undef, $expires);\n \tgit_print_page_nav('commit', '',\n \t $hash, $co{'tree'}, $hash,\n@@ -4430,9 +4496,9 @@ sub git_commit {\n \t \"\" .\n \t $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$hash)},\n \t \"tree\");\n-\tif ($have_snapshot) {\n-\t\tprint \" | \" .\n-\t\t $cgi->a({-href => href(action=>\"snapshot\", hash=>$hash)}, \"snapshot\");\n+\tmy $snapshot_links = format_snapshot_links($hash);\n+\tif (defined $snapshot_links) {\n+\t\tprint \" | \" . $snapshot_links;\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-- \n1.5.2.4\n"},{"id":"48090","messageId":"7vd4ylt3eh.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"200707220130.28516.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-22T05:26:30Z","receivedAt":"2007-07-22T05:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Will apply.\n"},{"id":"48145","messageId":"3bbc18d20707220805hd95c4ccsc48f140888403391@mail.gmail.com","threadId":"8762","inReplyTo":"7vd4ylt3eh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-22T15:05:51Z","receivedAt":"2007-07-22T15:05:51Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On 7/22/07, Junio C Hamano wrote:\n> Thanks. Will apply.\n\nJakub, thanks for picking this up. I was running out of energy to\npush what began as an offer of a possibly useful local customization\nany further toward adoption.\n\nThat said: the backward compatibility code for gitweb _site_\nconfiguration is broken because it is inside an if statement that only\nruns for gitweb _repository_ configuration. I tested it with a\ngitweb_config.perl with\n\n$feature{'snapshot'}{'default'} = ['x-gzip', 'gz', 'gzip'];\n$feature{'snapshot'}{'override'} = 0;\n\nand gitweb generated \"snapshot ( )\" (no visible link). Inspection of\nthe source revealed links to \"sf=x-gzip\", \"sf=gz\", and \"sf=gzip\" with\nempty strings for the link text. The expected behavior is\n\"_snapshot_\", linking to \"sf=tgz\", with a tooltip indicating \"tar.gz\"\nformat.\n\nMoving the code out of the `if' wouldn't solve the problem by itself\nbecause feature_snapshot is only reached if override is enabled, but\nthe site configuration compatibility needs to work whether or not\noverride is enabled. That's why Junio was considering adding a\nseparate function gitweb_bc_feature_snapshot. I think a nicer\nalternative would be to change gitweb_check_feature to call the sub\n(if any) whether or not override is enabled and let the sub check\n$feature{'foo'}{'override'} itself to decide whether to look for an\noverride. Then each feature_* sub is the central point for both\noverride processing and backward compatibility for that feature.\n\nMatt\n"},{"id":"48216","messageId":"200707222341.21538.jnareb@gmail.com","threadId":"8762","inReplyTo":"3bbc18d20707220805hd95c4ccsc48f140888403391@mail.gmail.com","subject":"[PATCH] gitweb: Fix support for legacy gitweb config for snapshots","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-22T21:41:20Z","receivedAt":"2007-07-22T21:41:20Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Earlier commit which cleaned up snapshot support and introduced\nsupport for multiple snapshot formats changed the format of\n$feature{'snapshot'}{'default'} (gitweb configuration) and\ngitweb.snapshot configuration variable (repository configuration).\nIt supported old gitweb.snapshot values of 'gzip', 'bzip2' and 'zip'\nand tried to support, but failed to do that, old values of\n$feature{'snapshot'}{'default'}; at least those corresponding to\nold gitweb.snapshot values of 'gzip', 'bzip2' and 'zip', i.e.\n ['x-gzip', 'gz', 'gzip']\n ['x-bzip2', 'bz2', 'bzip2']\n ['x-zip', 'zip', '']\n\nThis commit moves legacy configuration support out of feature_snapshot\nsubroutine to separate filter_snapshot_fmts subroutine. The\nfilter_snapshot_fmts is used on result on result of\ngitweb_check_feature('snapshot'). This way feature_snapshot deals\n_only_ with repository config.\n\nAs a byproduct you can now use 'gzip' and 'bzip2' as aliases to 'tgz'\nand 'tbz2' also in $feature{'snapshot'}{'default'}, not only in\ngitweb.snapshot.\n\n\nWhile at it do some whitespace cleanup: use tabs for indent, but\nspaces for align.\n\nNoticed-by: Matt McCutchen \nSigned-off-by: Jakub Narebski \n---\nOn Sun, 22 July 2007, Matt McCutchen wrote:\n\n> That said: the backward compatibility code for gitweb _site_\n> configuration is broken because it is inside an if statement that only\n> runs for gitweb _repository_ configuration.\n\nSorry for sending not fully tested code. This should fix that.\nThis commit _is_ rudimentally tested.\n\n gitweb/gitweb.perl | 27 ++++++++++++++++++++-------\n 1 files changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c4f8824..fdfce31 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -307,10 +307,6 @@ sub feature_snapshot {\n \n \tif ($val) {\n \t\t@fmts = ($val eq 'none' ? () : split /\\s*[,\\s]\\s*/, $val);\n-\t\t@fmts = grep { defined } map {\n-\t\t\texists $known_snapshot_format_aliases{$_} ?\n-\t\t\t\t$known_snapshot_format_aliases{$_} : $_ } @fmts;\n-\t\t@fmts = grep(exists $known_snapshot_formats{$_}, @fmts);\n \t}\n \n \treturn @fmts;\n@@ -356,6 +352,18 @@ sub check_export_ok {\n \t\t(!$export_ok || -e \"$dir/$export_ok\"));\n }\n \n+# process alternate names for backward compatibility\n+# filter out unsupported (unknown) snapshot formats\n+sub filter_snapshot_fmts {\n+\tmy @fmts = @_;\n+\n+\t@fmts = map {\n+\t\texists $known_snapshot_format_aliases{$_} ?\n+\t\t $known_snapshot_format_aliases{$_} : $_} @fmts;\n+\t@fmts = grep(exists $known_snapshot_formats{$_}, @fmts);\n+\n+}\n+\n our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\n \n@@ -1299,9 +1307,11 @@ sub format_diff_line {\n sub format_snapshot_links {\n \tmy ($hash) = @_;\n \tmy @snapshot_fmts = gitweb_check_feature('snapshot');\n+\t@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);\n \tmy $num_fmts = @snapshot_fmts;\n \tif ($num_fmts > 1) {\n \t\t# A parenthesized list of links bearing format names.\n+\t\t# e.g. \"snapshot (_tar.gz_ _zip_)\"\n \t\treturn \"snapshot (\" . join(' ', map\n \t\t\t$cgi->a({\n \t\t\t\t-href => href(\n@@ -1313,8 +1323,10 @@ sub format_snapshot_links {\n \t\t, @snapshot_fmts) . \")\";\n \t} elsif ($num_fmts == 1) {\n \t\t# A single \"snapshot\" link whose tooltip bears the format name.\n+\t\t# i.e. \"_snapshot_\"\n \t\tmy ($fmt) = @snapshot_fmts;\n-\t\treturn $cgi->a({\n+\t\treturn\n+\t\t\t$cgi->a({\n \t\t\t\t-href => href(\n \t\t\t\t\taction=>\"snapshot\",\n \t\t\t\t\thash=>$hash,\n@@ -4302,11 +4314,12 @@ sub git_tree {\n \n sub git_snapshot {\n \tmy @supported_fmts = gitweb_check_feature('snapshot');\n+\t@supported_fmts = filter_snapshot_fmts(@supported_fmts);\n \n \tmy $format = $cgi->param('sf');\n \tunless ($format =~ m/[a-z0-9]+/\n-\t\t&& exists($known_snapshot_formats{$format})\n-\t\t&& grep($_ eq $format, @supported_fmts)) {\n+\t && exists($known_snapshot_formats{$format})\n+\t && grep($_ eq $format, @supported_fmts)) {\n \t\tdie_error(undef, \"Unsupported snapshot format\");\n \t}\n \n-- \n1.5.2.4\n"},{"id":"48237","messageId":"3bbc18d20707221610q757536eet213a6d08b810b280@mail.gmail.com","threadId":"8762","inReplyTo":"200707222341.21538.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Fix support for legacy gitweb config for snapshots","fromName":"Matt McCutchen","fromEmail":"hashproduct@gmail.com","sentAt":"2007-07-22T23:10:43Z","receivedAt":"2007-07-22T23:10:43Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"On 7/22/07, Jakub Narebski wrote:\n> This commit moves legacy configuration support out of feature_snapshot\n> subroutine to separate filter_snapshot_fmts subroutine. The\n> filter_snapshot_fmts is used on result on result of\n\nThere's a typo: \"on result\" appears twice.\n\n> On Sun, 22 July 2007, Matt McCutchen wrote:\n>\n> > That said: the backward compatibility code for gitweb _site_\n> > configuration is broken because it is inside an if statement that only\n> > runs for gitweb _repository_ configuration.\n>\n> Sorry for sending not fully tested code. This should fix that.\n> This commit _is_ rudimentally tested.\n\nI tested it too and it worked. I like the approach of using\nfilter_snapshot_fmts.\n\nMatt\n"},{"id":"48239","messageId":"7vwswsovtu.fsf@assigned-by-dhcp.cox.net","threadId":"8762","inReplyTo":"3bbc18d20707221610q757536eet213a6d08b810b280@mail.gmail.com","subject":"Re: [PATCH] gitweb: Fix support for legacy gitweb config for snapshots","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-22T23:35:57Z","receivedAt":"2007-07-22T23:35:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, both.\n"},{"id":"48595","messageId":"200707252039.44312.jnareb@gmail.com","threadId":"8762","inReplyTo":"513314.51284.qm@web31813.mail.mud.yahoo.com","subject":"[RFC/PATCH] gitweb: Enable transparent compression form HTTP output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-25T18:39:43Z","receivedAt":"2007-07-25T18:39:43Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Check if PerlIO::gzip is available, and if it is make it possible to\nenable (via 'compression' %feature) transparent compression of HTML\noutput. Error messages and any non-HTML output are excluded from\ntransparent compression.\n\nSigned-off-by: Jakub Narebski \n---\nOn Thu, 19 July 2007, Luben Tuikov wrote:\n> --- Jakub Narebski wrote:\n> > Luben Tuikov wrote:\n> > \n> > > I wouldn't mind an improvement in the snapshot area of gitweb.\n> > > I wasn't really happy with the snapshot feature as it was originally\n> > > implemented, as it would generate a tar file with \".tar.bz2\"\n> > > name extension, but the file was NOT bz2, and I had to always\n> > > manually rename, bz2, and rename back.\n> > \n> > This was a *bug*, but it is now corrected (in 9aa17573). Gitweb used \n> > Content-Encoding, which is meant for _transparent_ compression.\n> \n> Yeah, that's what I suspected, since there was nothing obviously\n> wrong with the code.\n\nAnd _this_ patch adds support for true, intentional transparent\ncompression.\n\n gitweb/gitweb.perl | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 48 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0acd0ca..d48a193 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -20,8 +20,12 @@ binmode STDOUT, ':utf8';\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n+\n+\teval { require PerlIO::gzip; }; # needed for transparent compression\n }\n \n+our $enable_transparent_compression = !! $PerlIO::gzip::VERSION;\n+\n our $cgi = new CGI;\n our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n@@ -238,6 +242,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [1]},\n \n+\t# Enable transparent compression, for now only for HTML output;\n+\t# this reduces network bandwidth at the cost of CPU usage.\n+\t# You need to have PerlIO::gzip for that, and browser has to accept\n+\t# (via Accept-Encoding: HTTP request header) 'gzip' encoding.\n+\t# Transparent compression is not used for error messages.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'compression'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'compression'}{'override'} = 1;\n+\t# and in project config gitweb.compression = 0|1;\n+\t'compression' => {\n+\t\t'sub' => \\&feature_compression,\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n+\n \t# Make gitweb use an alternative format of the URLs which can be\n \t# more readable and natural-looking: project name is embedded\n \t# directly in the path and the query string contains other\n@@ -336,6 +356,18 @@ sub feature_pickaxe {\n \treturn ($_[0]);\n }\n \n+sub feature_compression {\n+\tmy ($val) = git_get_project_config('compression', '--bool');\n+\n+\tif ($val eq 'true') {\n+\t\treturn (1);\n+\t} elsif ($val eq 'false') {\n+\t\treturn (0);\n+\t}\n+\n+\treturn ($_[0]);\n+}\n+\n # checking HEAD file with -e is fragile if the repository was\n # initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed\n # and then pruned.\n@@ -2238,9 +2270,24 @@ sub git_header_html {\n \t} else {\n \t\t$content_type = 'text/html';\n \t}\n+\t# transparent compression has to be supported, enabled, and accepted\n+\t# explicitely by UA; note that qvalue of 0 means \"not acceptable.\"\n+\tmy %content_encoding = ();\n+\tif ($enable_transparent_compression &&\n+\t gitweb_check_feature('compression') &&\n+\t defined $cgi->http('HTTP_ACCEPT_ENCODING') &&\n+\t $cgi->http('HTTP_ACCEPT_ENCODING') =~ m/(^|,|;|\\s)gzip(,|;|\\s|$)/ &&\n+\t $cgi->http('HTTP_ACCEPT_ENCODING') !~ m/(^|,|;|\\s)gzip\\s*;q=0(,|\\s|$)/) {\n+\t\t%content_encoding = (-content_encoding => 'gzip');\n+\t}\n \tprint $cgi->header(-type=>$content_type, -charset => 'utf-8',\n+\t %content_encoding,\n \t -status=> $status, -expires => $expires);\n \tmy $mod_perl_version = $ENV{'MOD_PERL'} ? \" $ENV{'MOD_PERL'}\" : '';\n+\tif (%content_encoding) {\n+\t\t# implies $enable_transparent_compression\n+\t\tbinmode STDOUT, ':gzip';\n+\t}\n \tprint <\n \n@@ -2375,6 +2422,7 @@ sub die_error {\n \tmy $status = shift || \"403 Forbidden\";\n \tmy $error = shift || \"Malformed query, file missing or permission denied\";\n \n+\t$enable_transparent_compression = 0;\n \tgit_header_html($status);\n \tprint <\n-- \n1.5.2.4\n"},{"id":"51518","messageId":"20070825180350.GA1219@pasky.or.cz","threadId":"8762","inReplyTo":"200707252039.44312.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Enable transparent compression form HTTP output","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-25T18:03:50Z","receivedAt":"2007-08-25T18:03:50Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Jul 25, 2007 at 08:39:43PM CEST, Jakub Narebski wrote:\n> Check if PerlIO::gzip is available, and if it is make it possible to\n\nIt doesn't really check if the require succeeded. Either the description\nor (preferrably, but not a showstopper, IMO) the code should be\nadjusted.\n\n> enable (via 'compression' %feature) transparent compression of HTML\n> output. Error messages and any non-HTML output are excluded from\n> transparent compression.\n> \n> Signed-off-by: Jakub Narebski \n\nAcked-by: Petr Baudis \n\nI'd put it on repo.or.cz... too bad that there I value CPU much more\nthan the bandwidth. ;-)\n\nWhy did you exclude non-HTML output from transparent compression? Me and\nI guess other people too sometimes download rather large chunks of raw\ndata over gitweb.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nEver try. Ever fail. No matter. // Try again. Fail again. Fail better.\n\t\t-- Samuel Beckett\n"},{"id":"51524","messageId":"200708260009.30092.jnareb@gmail.com","threadId":"8762","inReplyTo":"20070825180350.GA1219@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Enable transparent compression form HTTP output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-08-25T22:09:29Z","receivedAt":"2007-08-25T22:09:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, Aug 25, 2007, Petr Baudis wrote:\n> On Wed, Jul 25, 2007 at 08:39:43PM CEST, Jakub Narebski wrote:\n\n>> Check if PerlIO::gzip is available, and if it is make it possible to\n> \n> It doesn't really check if the require succeeded. Either the description\n> or (preferrably, but not a showstopper, IMO) the code should be\n> adjusted.\n\nIt does not check if require succeeded (I could do that this way),\nbut instead checks if $PerlIO::gzip::VERSION is defined (if it is true).\n\nour $enable_transparent_compression = !! $PerlIO::gzip::VERSION;\n \n>> enable (via 'compression' %feature) transparent compression of HTML\n>> output. Error messages and any non-HTML output are excluded from\n>> transparent compression.\n>> \n>> Signed-off-by: Jakub Narebski \n> \n> Acked-by: Petr Baudis \n\nBy the way, this was more \"proof of concept\" than solution of an itch.\n \n> I'd put it on repo.or.cz... too bad that there I value CPU much more\n> than the bandwidth. ;-)\n> \n> Why did you exclude non-HTML output from transparent compression? Me and\n> I guess other people too sometimes download rather large chunks of raw\n> data over gitweb.\n\nBecause it was easiest. We have single point of entry for HTML output\n(the git_header_html subroutine), but we don't have anything similar for\nnon-HTML output. And we most certainly wouldn't want to enable transparent\ncompression for snapshots and 'blob_plain' view for compressed files,\nincluding png, gif, jpeg, zip, mp3, ogg,...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"51525","messageId":"20070825221445.GD1219@pasky.or.cz","threadId":"8762","inReplyTo":"200708260009.30092.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Enable transparent compression form HTTP output","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-08-25T22:14:45Z","receivedAt":"2007-08-25T22:14:45Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Sun, Aug 26, 2007 at 12:09:29AM CEST, Jakub Narebski wrote:\n> On Sat, Aug 25, 2007, Petr Baudis wrote:\n> > On Wed, Jul 25, 2007 at 08:39:43PM CEST, Jakub Narebski wrote:\n> \n> >> Check if PerlIO::gzip is available, and if it is make it possible to\n> > \n> > It doesn't really check if the require succeeded. Either the description\n> > or (preferrably, but not a showstopper, IMO) the code should be\n> > adjusted.\n> \n> It does not check if require succeeded (I could do that this way),\n> but instead checks if $PerlIO::gzip::VERSION is defined (if it is true).\n> \n> our $enable_transparent_compression = !! $PerlIO::gzip::VERSION;\n\nWhoops, I completely missed this chunk.\n\n Bareword \"PerlIO::gzip::VERSION\" not allowed while \"strict subs\" in use at /home/pasky/WWW/repo/gitweb.cgi line 26.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nEver try. Ever fail. No matter. // Try again. Fail again. Fail better.\n\t\t-- Samuel Beckett\n"},{"id":"51684","messageId":"200708271301.06451.jnareb@gmail.com","threadId":"8762","inReplyTo":"20070825221445.GD1219@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Enable transparent compression form HTTP output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-08-27T11:01:05Z","receivedAt":"2007-08-27T11:01:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sunday, 26 August 2007, Petr \"Pasky\" Baudis wrote:\n> On Sun, Aug 26, 2007 at 12:09:29AM CEST, Jakub Narebski wrote:\n>> On Sat, Aug 25, 2007, Petr Baudis wrote:\n>>> On Wed, Jul 25, 2007 at 08:39:43PM CEST, Jakub Narebski wrote:\n>>> \n>>>> Check if PerlIO::gzip is available, and if it is make it possible to\n>>> \n>>> It doesn't really check if the require succeeded. Either the description\n>>> or (preferrably, but not a showstopper, IMO) the code should be\n>>> adjusted.\n>> \n>> It does not check if require succeeded (I could do that this way),\n>> but instead checks if $PerlIO::gzip::VERSION is defined (if it is true).\n\nSee below for alternate solution.\n\n>> our $enable_transparent_compression = !! $PerlIO::gzip::VERSION;\n> \n> Whoops, I completely missed this chunk.\n> \n> Bareword \"PerlIO::gzip::VERSION\" not allowed while \"strict subs\" in use at /home/pasky/WWW/repo/gitweb.cgi line 26.\n\nDid you perchance forgot '$' in \"$PerlIO::gzip::VERSION\"?\n\nBut I agree that using\n\n\tBEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n\n\t eval { require PerlIO::gzip; }; # needed for transparent compression\n\t\tour $enable_transparent_compression = ! $@;\n\t}\n \n\ninstead of\n\n\tBEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n\n\t eval { require PerlIO::gzip; }; # needed for transparent compression\n\t}\n \n\tour $enable_transparent_compression = !! $PerlIO::gzip::VERSION;\n \nis more sensible. I have tried to check the above code for the case when\nPerlIO::gzip is not available by using non-existent module \"PerlIO::gzp\"\nin eval, and non-existent variable \"$PerlIO::gip::VERSION\" in the\ndefinition of $enable_transparent_compression variable, and Perl doesn't\ngive any errors nor warnings while running gitweb.\n\nBut what it is a bit strange, when I have chosen different name for\na variable to test, \"$PerlIO::gzip::VERSON\" (existing but not loaded\nmodule, non-existent name), I have got the following strange warning:\n\n gitweb.perl: Name \"PerlIO::gzip::VERSON\" used only once: possible typo\n at /home/jnareb/git/t/trash/../../gitweb/gitweb.perl line 27.\n\nStrange...\n\n-- \nJakub Narebski\nPoland\n"}]}