{"thread":{"id":"21517","subject":"[PATCHv2 0/3] gitweb: Smarter snapshot names","startedAt":"2009-11-07T15:13:26Z","lastAt":"2009-11-07T15:13:29Z","messageCount":4,"participants":["Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"127044","messageId":"1257606809-23287-1-git-send-email-jnareb@gmail.com","threadId":"21517","inReplyTo":null,"subject":"[PATCHv2 0/3] gitweb: Smarter snapshot names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-11-07T15:13:26Z","receivedAt":"2009-11-07T15:13:26Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is the replacement for 'mr/gitweb-snapshot' branch (aba1076),\ncurrently in 'pu'.\n\nCompared to previous series the t9502-gitweb-standalone-parse-output\ntest is created in earlier commit, to better show changes in snapshot\nbehaviour in \"gitweb: Smarter snapshot names\".\n\nThe first commit in series is identical to the one in previous version\nof this series.  Changes in last commit in series are only about\ntests.\n\nShortlog:\n~~~~~~~~~\nJakub Narebski (2):\n  t/gitweb-lib.sh: Split gitweb output into headers and body\n  gitweb: Document current snapshot rules via new tests\n\nMark Rada (1):\n  gitweb: Smarter snapshot names\n\nTable of contents:\n~~~~~~~~~~~~~~~~~~\n [PATCH 1/3] t/gitweb-lib.sh: Split gitweb output into headers and body\n [PATCH 2/3] gitweb: Document current snapshot rules via new tests\n [PATCH 3/3] gitweb: Smarter snapshot names\n\nDiffstat:\n~~~~~~~~~\n gitweb/gitweb.perl                        |   76 +++++++++++++++----\n t/gitweb-lib.sh                           |    6 +-\n t/t9502-gitweb-standalone-parse-output.sh |  115 +++++++++++++++++++++++++++++\n 3 files changed, 180 insertions(+), 17 deletions(-)\n create mode 100755 t/t9502-gitweb-standalone-parse-output.sh\n\n-- \nJakub Narebski\nPoland\n"},{"id":"127046","messageId":"1257606809-23287-2-git-send-email-jnareb@gmail.com","threadId":"21517","inReplyTo":"1257606809-23287-1-git-send-email-jnareb@gmail.com","subject":"[PATCH 1/3] t/gitweb-lib.sh: Split gitweb output into headers and body","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-11-07T15:13:27Z","receivedAt":"2009-11-07T15:13:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Save HTTP headers into gitweb.headers, and the body of message into\ngitweb.body in gitweb_run()\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis version is identical to the one in previous version of this\nseries.\n\nI hope that this use of sed is portable enough.\n\nPlease take into account that HTTP headers part contains at least one\nline: HTTP status.\n\n t/gitweb-lib.sh |    6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex 8452532..32b841d\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -52,10 +52,14 @@ gitweb_run () {\n \trm -f gitweb.log &&\n \tperl -- \"$SCRIPT_NAME\" \\\n \t\t>gitweb.output 2>gitweb.log &&\n+\tsed -e   '/^\\r$/q' <gitweb.output >gitweb.headers &&\n+\tsed -e '1,/^\\r$/d' <gitweb.output >gitweb.body    &&\n \tif grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi\n \n \t# gitweb.log is left for debugging\n-\t# gitweb.output is used to parse http output\n+\t# gitweb.output is used to parse HTTP output\n+\t# gitweb.headers contains only HTTP headers\n+\t# gitweb.body contains body of message, without headers\n }\n \n . ./test-lib.sh\n-- \n1.6.5\n"},{"id":"127047","messageId":"1257606809-23287-3-git-send-email-jnareb@gmail.com","threadId":"21517","inReplyTo":"1257606809-23287-1-git-send-email-jnareb@gmail.com","subject":"[PATCH 2/3] gitweb: Document current snapshot rules via new tests","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-11-07T15:13:28Z","receivedAt":"2009-11-07T15:13:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add t9502-gitweb-standalone-parse-output test script, which runs\ngitweb as a CGI script from the commandline and checks that it\nproduces the correct output.\n\nCurrently this test script contains only tests of snapshot naming\n(proposed name of snapshot file) and snapshot prefix (prefix of files\nin the archive / snapshot).  It defines and uses 'tar' snapshot\nformat, without compression, for easy checking of snapshot prefix.\nTesting is done using check_snapshot function.\n\nGitweb uses the following format for snapshot filenames:\n  <sanitized project name>-<hash parameter><snapshot suffix>\nwhere <sanitized project name> is project name with '.git' or '/.git'\nsuffix stripped, unless '.git' is the whole project name.  For\nsnapshot prefix it uses simply:\n  <sanitized project name>/\n\nDisadvantages of current snapshot rules:\n* There exists convention that <basename>.<suffix> archive unpacks to\n  <basename>/ directory (<basename>/ is prefix of archive).  Gitweb\n  does not respect it\n* Snapshot links generated by gitweb use full SHA-1 id as a value of\n  'h' / $hash parameter.  With current rules it leads to long file\n  names like e.g. repo-1005c80cc11c531d327b12195027cbbb4ff9e3cb.tgz\n* For handcrafted URLs, where 'h' / $hash parameter is a symbolic\n  'volatile' revision name such as \"HEAD\" or \"next\" snapshot name\n  doesn't tell us what exact version it was created from\n* Proposed filename in Content-Disposition header should not contain\n  any directory path information, which means that it should not\n  contain '/' (see RFC2183)... which means that snapshot naming is\n  broken for $hash being e.g. hirearchical branch name such as\n  'xx/test'\n\nThis would be improved in next commit.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis is a new commit in this series.  It wasn't present in previous\nversion of \"smarter snapshot names\" series.\n\n t/t9502-gitweb-standalone-parse-output.sh |   87 +++++++++++++++++++++++++++++\n 1 files changed, 87 insertions(+), 0 deletions(-)\n create mode 100755 t/t9502-gitweb-standalone-parse-output.sh\n\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nnew file mode 100755\nindex 0000000..741187b\n--- /dev/null\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -0,0 +1,87 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2009 Mark Rada\n+#\n+\n+test_description='gitweb as standalone script (parsing script output).\n+\n+This test runs gitweb (git web interface) as a CGI script from the\n+commandline, and checks that it produces the correct output, either\n+in the HTTP header or the actual script output.'\n+\n+\n+. ./gitweb-lib.sh\n+\n+# ----------------------------------------------------------------------\n+# snapshot file name and prefix\n+\n+cat >>gitweb_config.perl <<\\EOF\n+\n+$known_snapshot_formats{'tar'} = {\n+\t'display' => 'tar',\n+\t'type' => 'application/x-tar',\n+\t'suffix' => '.tar',\n+\t'format' => 'tar',\n+};\n+\n+$feature{'snapshot'}{'default'} = ['tar'];\n+EOF\n+\n+# Call check_snapshot with the arguments \"<basename> [<prefix>]\"\n+#\n+# This will check that gitweb HTTP header contains proposed filename\n+# as <basename> with '.tar' suffix added, and that generated tarfile\n+# (gitweb message body) has <prefix> as prefix for al files in tarfile\n+#\n+# <prefix> default to <basename>\n+check_snapshot () {\n+\tbasename=$1\n+\tprefix=${2:-\"$1\"}\n+\techo \"basename=$basename\"\n+\tgrep \"filename=.*$basename.tar\" gitweb.headers >/dev/null 2>&1 &&\n+\t\"$TAR\" tf gitweb.body >file_list &&\n+\t! grep -v \"^$prefix/\" file_list\n+}\n+\n+test_expect_success setup '\n+\ttest_commit first foo &&\n+\tgit branch xx/test &&\n+\tFULL_ID=$(git rev-parse --verify HEAD) &&\n+\tSHORT_ID=$(git rev-parse --verify --short=7 HEAD)\n+'\n+test_debug '\n+\techo \"FULL_ID  = $FULL_ID\"\n+\techo \"SHORT_ID = $SHORT_ID\"\n+'\n+\n+test_expect_success 'snapshot: full sha1' '\n+\tgitweb_run \"p=.git;a=snapshot;h=$FULL_ID;sf=tar\" &&\n+\tcheck_snapshot \".git-$FULL_ID\" \".git\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: shortened sha1' '\n+\tgitweb_run \"p=.git;a=snapshot;h=$SHORT_ID;sf=tar\" &&\n+\tcheck_snapshot \".git-$SHORT_ID\" \".git\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: HEAD' '\n+\tgitweb_run \"p=.git;a=snapshot;h=HEAD;sf=tar\" &&\n+\tcheck_snapshot \".git-HEAD\" \".git\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: short branch name (master)' '\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tar\" &&\n+\tcheck_snapshot \".git-master\" \".git\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_failure 'snapshot: hierarchical branch name (xx/test)' '\n+\tgitweb_run \"p=.git;a=snapshot;h=xx/test;sf=tar\" &&\n+\t! grep \"filename=.*/\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_done\n-- \n1.6.5\n"},{"id":"127045","messageId":"1257606809-23287-4-git-send-email-jnareb@gmail.com","threadId":"21517","inReplyTo":"1257606809-23287-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv7 3/3] gitweb: Smarter snapshot names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-11-07T15:13:29Z","receivedAt":"2009-11-07T15:13:29Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Mark Rada <marada@uwaterloo.ca>\n\nTeach gitweb how to produce nicer snapshot names by only using the\nshort hash id.  If clients make requests using a tree-ish that is not\na partial or full SHA-1 hash, then the short hash will also be appended\nto whatever they asked for.  If clients request snapshot of a tag\n(which means that $hash ('h') parameter has 'refs/tags/' prefix),\nuse only tag name.\n\nUpdate tests cases in t9502-gitweb-standalone-parse-output.\n\nGitweb uses the following format for snapshot filenames:\n  <sanitized project name>-<version info>.<snapshot suffix>\nwhere <sanitized project name> is project name with '.git' or '/.git'\nsuffix stripped, unless '.git' is the whole project name.  For\nsnapshot prefix it uses:\n  <sanitized project name>-<version info>/\nas compared to <sanitized project name>/ before (without version info).\n\nCurrent rules for <version info>:\n* if 'h' / $hash parameter is SHA-1 or shortened SHA-1, use SHA-1\n  shortened to to 7 characters\n* otherwise if 'h' / $hash parameter is tag name (it begins with\n  'refs/tags/' prefix, use tag name (with 'refs/tags/' stripped\n* otherwise if 'h' / $hash parameter starts with 'refs/heads/' prefix,\n  strip this prefix, convert '/' into '.', and append shortened SHA-1\n  after '-', i.e. use <sanitized hash>-<shortened sha1>\n\nSigned-off-by: Mark Rada <marada@uwaterloo.ca>\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nChanges since v6:\n- use check_snapshot function to test snapshots; therefore separate\n  test checking archive prefix is not needed any more\n- fixed checking that hierarchical branch names work correctly\n\n gitweb/gitweb.perl                        |   76 +++++++++++++++++++++++------\n t/t9502-gitweb-standalone-parse-output.sh |   38 ++++++++++++--\n 2 files changed, 93 insertions(+), 21 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8d4a2ae..d8dfd95 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1983,16 +1983,27 @@ sub quote_command {\n \n # get HEAD ref of given project as hash\n sub git_get_head_hash {\n-\tmy $project = shift;\n+\treturn git_get_full_hash(shift, 'HEAD');\n+}\n+\n+sub git_get_full_hash {\n+\treturn git_get_hash(@_);\n+}\n+\n+sub git_get_short_hash {\n+\treturn git_get_hash(@_, '--short=7');\n+}\n+\n+sub git_get_hash {\n+\tmy ($project, $hash, @options) = @_;\n \tmy $o_git_dir = $git_dir;\n \tmy $retval = undef;\n \t$git_dir = \"$projectroot/$project\";\n-\tif (open my $fd, \"-|\", git_cmd(), \"rev-parse\", \"--verify\", \"HEAD\") {\n-\t\tmy $head = <$fd>;\n+\tif (open my $fd, '-|', git_cmd(), 'rev-parse',\n+\t    '--verify', '-q', @options, $hash) {\n+\t\t$retval = <$fd>;\n+\t\tchomp $retval if defined $retval;\n \t\tclose $fd;\n-\t\tif (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {\n-\t\t\t$retval = $1;\n-\t\t}\n \t}\n \tif (defined $o_git_dir) {\n \t\t$git_dir = $o_git_dir;\n@@ -5179,6 +5190,43 @@ sub git_tree {\n \tgit_footer_html();\n }\n \n+sub snapshot_name {\n+\tmy ($project, $hash) = @_;\n+\n+\t# path/to/project.git  -> project\n+\t# path/to/project/.git -> project\n+\tmy $name = to_utf8($project);\n+\t$name =~ s,([^/])/*\\.git$,$1,;\n+\t$name = basename($name);\n+\t# sanitize name\n+\t$name =~ s/[[:cntrl:]]/?/g;\n+\n+\tmy $ver = $hash;\n+\tif ($hash =~ /^[0-9a-fA-F]+$/) {\n+\t\t# shorten SHA-1 hash\n+\t\tmy $full_hash = git_get_full_hash($project, $hash);\n+\t\tif ($full_hash =~ /^$hash/ && length($hash) > 7) {\n+\t\t\t$ver = git_get_short_hash($project, $hash);\n+\t\t}\n+\t} elsif ($hash =~ m!^refs/tags/(.*)$!) {\n+\t\t# tags don't need shortened SHA-1 hash\n+\t\t$ver = $1;\n+\t} else {\n+\t\t# branches and other need shortened SHA-1 hash\n+\t\tif ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {\n+\t\t\t$ver = $1;\n+\t\t}\n+\t\t$ver .= '-' . git_get_short_hash($project, $hash);\n+\t}\n+\t# in case of hierarchical branch names\n+\t$ver =~ s!/!.!g;\n+\n+\t# name = project-version_string\n+\t$name = \"$name-$ver\";\n+\n+\treturn wantarray ? ($name, $name) : $name;\n+}\n+\n sub git_snapshot {\n \tmy $format = $input_params{'snapshot_format'};\n \tif (!@snapshot_fmts) {\n@@ -5203,24 +5251,20 @@ sub git_snapshot {\n \t\tdie_error(400, 'Object is not a tree-ish');\n \t}\n \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-\t$filename .= \"-$hash$known_snapshot_formats{$format}{'suffix'}\";\n-\t$cmd = quote_command(\n+\tmy ($name, $prefix) = snapshot_name($project, $hash);\n+\tmy $filename = \"$name$known_snapshot_formats{$format}{'suffix'}\";\n+\tmy $cmd = quote_command(\n \t\tgit_cmd(), 'archive',\n \t\t\"--format=$known_snapshot_formats{$format}{'format'}\",\n-\t\t\"--prefix=$name/\", $hash);\n+\t\t\"--prefix=$prefix/\", $hash);\n \tif (exists $known_snapshot_formats{$format}{'compressor'}) {\n \t\t$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});\n \t}\n \n+\t$filename =~ s/([\"\\\\])/\\\\$1/g;\n \tprint $cgi->header(\n \t\t-type => $known_snapshot_formats{$format}{'type'},\n-\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"',\n+\t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n \t\t-status => '200 OK');\n \n \topen my $fd, \"-|\", $cmd\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex 741187b..dd83890 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -56,29 +56,57 @@ test_debug '\n \n test_expect_success 'snapshot: full sha1' '\n \tgitweb_run \"p=.git;a=snapshot;h=$FULL_ID;sf=tar\" &&\n-\tcheck_snapshot \".git-$FULL_ID\" \".git\"\n+\tcheck_snapshot \".git-$SHORT_ID\"\n '\n test_debug 'cat gitweb.headers && cat file_list'\n \n test_expect_success 'snapshot: shortened sha1' '\n \tgitweb_run \"p=.git;a=snapshot;h=$SHORT_ID;sf=tar\" &&\n-\tcheck_snapshot \".git-$SHORT_ID\" \".git\"\n+\tcheck_snapshot \".git-$SHORT_ID\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: almost full sha1' '\n+\tID=$(git rev-parse --short=30 HEAD) &&\n+\tgitweb_run \"p=.git;a=snapshot;h=$ID;sf=tar\" &&\n+\tcheck_snapshot \".git-$SHORT_ID\"\n '\n test_debug 'cat gitweb.headers && cat file_list'\n \n test_expect_success 'snapshot: HEAD' '\n \tgitweb_run \"p=.git;a=snapshot;h=HEAD;sf=tar\" &&\n-\tcheck_snapshot \".git-HEAD\" \".git\"\n+\tcheck_snapshot \".git-HEAD-$SHORT_ID\"\n '\n test_debug 'cat gitweb.headers && cat file_list'\n \n test_expect_success 'snapshot: short branch name (master)' '\n \tgitweb_run \"p=.git;a=snapshot;h=master;sf=tar\" &&\n-\tcheck_snapshot \".git-master\" \".git\"\n+\tID=$(git rev-parse --verify --short=7 master) &&\n+\tcheck_snapshot \".git-master-$ID\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: short tag name (first)' '\n+\tgitweb_run \"p=.git;a=snapshot;h=first;sf=tar\" &&\n+\tID=$(git rev-parse --verify --short=7 first) &&\n+\tcheck_snapshot \".git-first-$ID\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: full branch name (refs/heads/master)' '\n+\tgitweb_run \"p=.git;a=snapshot;h=refs/heads/master;sf=tar\" &&\n+\tID=$(git rev-parse --verify --short=7 master) &&\n+\tcheck_snapshot \".git-master-$ID\"\n+'\n+test_debug 'cat gitweb.headers && cat file_list'\n+\n+test_expect_success 'snapshot: full tag name (refs/tags/first)' '\n+\tgitweb_run \"p=.git;a=snapshot;h=refs/tags/first;sf=tar\" &&\n+\tcheck_snapshot \".git-first\"\n '\n test_debug 'cat gitweb.headers && cat file_list'\n \n-test_expect_failure 'snapshot: hierarchical branch name (xx/test)' '\n+test_expect_success 'snapshot: hierarchical branch name (xx/test)' '\n \tgitweb_run \"p=.git;a=snapshot;h=xx/test;sf=tar\" &&\n \t! grep \"filename=.*/\" gitweb.headers\n '\n-- \n1.6.5\n"}]}