gitweb: Smarter snapshot names

Teach gitweb how to produce nicer snapshot names by only using the
short hash id.  If clients make requests using a tree-ish that is not
a partial or full SHA-1 hash, then the short hash will also be appended
to whatever they asked for.  If clients request snapshot of a tag
(which means that $hash ('h') parameter has 'refs/tags/' prefix),
use only tag name.

Update tests cases in t9502-gitweb-standalone-parse-output.

Gitweb uses the following format for snapshot filenames:
  <sanitized project name>-<version info>.<snapshot suffix>
where <sanitized project name> is project name with '.git' or '/.git'
suffix stripped, unless '.git' is the whole project name.  For
snapshot prefix it uses:
  <sanitized project name>-<version info>/
as compared to <sanitized project name>/ before (without version info).

Current rules for <version info>:
* if 'h' / $hash parameter is SHA-1 or shortened SHA-1, use SHA-1
  shortened to to 7 characters
* otherwise if 'h' / $hash parameter is tag name (it begins with
  'refs/tags/' prefix, use tag name (with 'refs/tags/' stripped
* otherwise if 'h' / $hash parameter starts with 'refs/heads/' prefix,
  strip this prefix, convert '/' into '.', and append shortened SHA-1
  after '-', i.e. use <sanitized hash>-<shortened sha1>

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d4a2ae6004b38991e03d35113628c1a0304aa32..d8dfd950a445756eb50d8905f3ba63100f20544c 100755 (executable)
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,16 +1983,27 @@ sub quote_command {
 
 # get HEAD ref of given project as hash
 sub git_get_head_hash {
-       my $project = shift;
+       return git_get_full_hash(shift, 'HEAD');
+}
+
+sub git_get_full_hash {
+       return git_get_hash(@_);
+}
+
+sub git_get_short_hash {
+       return git_get_hash(@_, '--short=7');
+}
+
+sub git_get_hash {
+       my ($project, $hash, @options) = @_;
        my $o_git_dir = $git_dir;
        my $retval = undef;
        $git_dir = "$projectroot/$project";
-       if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
-               my $head = <$fd>;
+       if (open my $fd, '-|', git_cmd(), 'rev-parse',
+           '--verify', '-q', @options, $hash) {
+               $retval = <$fd>;
+               chomp $retval if defined $retval;
                close $fd;
-               if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
-                       $retval = $1;
-               }
        }
        if (defined $o_git_dir) {
                $git_dir = $o_git_dir;
@@ -5179,6 +5190,43 @@ sub git_tree {
        git_footer_html();
 }
 
+sub snapshot_name {
+       my ($project, $hash) = @_;
+
+       # path/to/project.git  -> project
+       # path/to/project/.git -> project
+       my $name = to_utf8($project);
+       $name =~ s,([^/])/*\.git$,$1,;
+       $name = basename($name);
+       # sanitize name
+       $name =~ s/[[:cntrl:]]/?/g;
+
+       my $ver = $hash;
+       if ($hash =~ /^[0-9a-fA-F]+$/) {
+               # shorten SHA-1 hash
+               my $full_hash = git_get_full_hash($project, $hash);
+               if ($full_hash =~ /^$hash/ && length($hash) > 7) {
+                       $ver = git_get_short_hash($project, $hash);
+               }
+       } elsif ($hash =~ m!^refs/tags/(.*)$!) {
+               # tags don't need shortened SHA-1 hash
+               $ver = $1;
+       } else {
+               # branches and other need shortened SHA-1 hash
+               if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
+                       $ver = $1;
+               }
+               $ver .= '-' . git_get_short_hash($project, $hash);
+       }
+       # in case of hierarchical branch names
+       $ver =~ s!/!.!g;
+
+       # name = project-version_string
+       $name = "$name-$ver";
+
+       return wantarray ? ($name, $name) : $name;
+}
+
 sub git_snapshot {
        my $format = $input_params{'snapshot_format'};
        if (!@snapshot_fmts) {
@@ -5203,24 +5251,20 @@ sub git_snapshot {
                die_error(400, 'Object is not a tree-ish');
        }
 
-       my $name = $project;
-       $name =~ s,([^/])/*\.git$,$1,;
-       $name = basename($name);
-       my $filename = to_utf8($name);
-       $name =~ s/\047/\047\\\047\047/g;
-       my $cmd;
-       $filename .= "-$hash$known_snapshot_formats{$format}{'suffix'}";
-       $cmd = quote_command(
+       my ($name, $prefix) = snapshot_name($project, $hash);
+       my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
+       my $cmd = quote_command(
                git_cmd(), 'archive',
                "--format=$known_snapshot_formats{$format}{'format'}",
-               "--prefix=$name/", $hash);
+               "--prefix=$prefix/", $hash);
        if (exists $known_snapshot_formats{$format}{'compressor'}) {
                $cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
        }
 
+       $filename =~ s/(["\\])/\\$1/g;
        print $cgi->header(
                -type => $known_snapshot_formats{$format}{'type'},
-               -content_disposition => 'inline; filename="' . "$filename" . '"',
+               -content_disposition => 'inline; filename="' . $filename . '"',
                -status => '200 OK');
 
        open my $fd, "-|", $cmd

diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 741187b9e42558153e341a31c3c84c022ad3b8ae..dd8389000187b11fe126f6ca76f578324075bd6f 100755 (executable)
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -56,29 +56,57 @@ test_debug '
 
 test_expect_success 'snapshot: full sha1' '
        gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
-       check_snapshot ".git-$FULL_ID" ".git"
+       check_snapshot ".git-$SHORT_ID"
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
 test_expect_success 'snapshot: shortened sha1' '
        gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
-       check_snapshot ".git-$SHORT_ID" ".git"
+       check_snapshot ".git-$SHORT_ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: almost full sha1' '
+       ID=$(git rev-parse --short=30 HEAD) &&
+       gitweb_run "p=.git;a=snapshot;h=$ID;sf=tar" &&
+       check_snapshot ".git-$SHORT_ID"
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
 test_expect_success 'snapshot: HEAD' '
        gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
-       check_snapshot ".git-HEAD" ".git"
+       check_snapshot ".git-HEAD-$SHORT_ID"
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
 test_expect_success 'snapshot: short branch name (master)' '
        gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
-       check_snapshot ".git-master" ".git"
+       ID=$(git rev-parse --verify --short=7 master) &&
+       check_snapshot ".git-master-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: short tag name (first)' '
+       gitweb_run "p=.git;a=snapshot;h=first;sf=tar" &&
+       ID=$(git rev-parse --verify --short=7 first) &&
+       check_snapshot ".git-first-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: full branch name (refs/heads/master)' '
+       gitweb_run "p=.git;a=snapshot;h=refs/heads/master;sf=tar" &&
+       ID=$(git rev-parse --verify --short=7 master) &&
+       check_snapshot ".git-master-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: full tag name (refs/tags/first)' '
+       gitweb_run "p=.git;a=snapshot;h=refs/tags/first;sf=tar" &&
+       check_snapshot ".git-first"
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
-test_expect_failure 'snapshot: hierarchical branch name (xx/test)' '
+test_expect_success 'snapshot: hierarchical branch name (xx/test)' '
        gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
        ! grep "filename=.*/" gitweb.headers
 '