Re: [PATCH v5 3/3] gitweb: add If-Modified-Since handling to git_snapshot().
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Mar 27, 2012, 22:31 UTC
- Message-ID
- <201203280031.38011.jnareb@gmail.com>
- In-Reply-To
- <20120326191418.GB9041@odin.tremily.us>
On Mon, 26 Mar 2012, W. Trevor King wrote:
> Because snapshots can be large, you can save some bandwidth by > supporting caching via If-Modified-Since. This patch adds support for > the i-m-s request to git_snapshot() if the requested hash is a commit.
"if the requested hash is a commit" means here "if we request snapshot of a commit", isn't it?
> Requests for snapshots of tree-ishes, which lack well defined > timestamps, are still handled as they were before.
s/tree-ishes/trees/. Commit is tree-ish but not a tree; it is tree that lacks timestamp that commit has.
Show 19 quoted lines
>
> Signed-off-by: W Trevor King <wking@drexel.edu>
> ---
> gitweb/gitweb.perl | 21 +++++++++++++++---
> t/t9501-gitweb-standalone-http-status.sh | 33 ++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 229f3da..be9ad5d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -7051,6 +7051,10 @@ sub git_snapshot {
>
> my ($name, $prefix) = snapshot_name($project, $hash);
> my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
> +
> + my %co = parse_commit($hash);
> + exit_if_unmodified_since($co{'committer_epoch'}) if %co;
> +O.K. Nice.
Show 24 quoted lines
> my $cmd = quote_command(
> git_cmd(), 'archive',
> "--format=$known_snapshot_formats{$format}{'format'}",
> @@ -7060,10 +7064,19 @@ sub git_snapshot {
> }
>
> $filename =~ s/(["\\])/\\$1/g;
> - print $cgi->header(
> - -type => $known_snapshot_formats{$format}{'type'},
> - -content_disposition => 'inline; filename="' . $filename . '"',
> - -status => '200 OK');
> + if (%co) {
> + my %latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
> + print $cgi->header(
> + -type => $known_snapshot_formats{$format}{'type'},
> + -content_disposition => 'inline; filename="' . $filename . '"',
> + -last_modified => $latest_date{'rfc2822'},
> + -status => '200 OK');
> + } else {
> + print $cgi->header(
> + -type => $known_snapshot_formats{$format}{'type'},
> + -content_disposition => 'inline; filename="' . $filename . '"',
> + -status => '200 OK');
> + }This can be written shorter, and with less code repetition using ?: conditional operator:
Show 5 quoted lines
> + print $cgi->header(
> + -type => $known_snapshot_formats{$format}{'type'},
> + -content_disposition => 'inline; filename="' . $filename . '"',
> + (%co ? -last_modified => $latest_date{'rfc2822'} : ()),
> + -status => '200 OK');...or something like that.
Show 7 quoted lines
> > open my $fd, "-|", $cmd > or die_error(500, "Execute git-archive failed"); > diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh > index 0e49f29..38e90bd 100755 > --- a/t/t9501-gitweb-standalone-http-status.sh > +++ b/t/t9501-gitweb-standalone-http-status.sh
Same comments as for 2/3.
-- Jakub Narebski Poland