{"thread":{"id":"30083","subject":"[PATCH v7 0/3] gitweb: refactor If-Modified-Since handling","startedAt":"2012-03-28T17:44:15Z","lastAt":"2012-03-30T16:11:55Z","messageCount":18,"participants":["W. Trevor King","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":7,"patchTotal":3},"messages":[{"id":"187956","messageId":"cover.1332956550.git.wking@drexel.edu","threadId":"30083","inReplyTo":"20120328164513.GA4389@odin.tremily.us","subject":"[PATCH v7 0/3] gitweb: refactor If-Modified-Since handling","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T17:44:15Z","receivedAt":"2012-03-28T17:44:15Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"Changed since v6:\n* Don't set a Last-Modified header when it has no value.\n\n\np.s. the `From` issues with v6 were due to a missing\n\n  FromLineOverride=YES\n\nin my sSMTP config ;).\n\n\nW. Trevor King (3):\n  gitweb: add `status` headers to git_feed() responses.\n  gitweb: refactor If-Modified-Since handling\n  gitweb: add If-Modified-Since handling to git_snapshot().\n\n gitweb/gitweb.perl                       |   65 ++++++++++++++++++------------\n t/t9501-gitweb-standalone-http-status.sh |   60 +++++++++++++++++++++++++++-\n 2 files changed, 98 insertions(+), 27 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"187969","messageId":"4958e06536924de10105bb453d88e8f4c28bb8c1.1332956550.git.wking@drexel.edu","threadId":"30083","inReplyTo":"20120328164513.GA4389@odin.tremily.us","subject":"[PATCH v7 1/3] gitweb: add `status` headers to git_feed() responses.","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T17:44:16Z","receivedAt":"2012-03-28T17:44:16Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"The git_feed() method was not setting a `Status` header unless it was\nresponding to an If-Modified-Since request with `304 Not Modified`.\nNow, when it is serving successful responses, it sets status to `200\nOK`.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a8b5fad..041da17 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7841,11 +7841,13 @@ sub git_feed {\n \t\tprint $cgi->header(\n \t\t\t-type => $content_type,\n \t\t\t-charset => 'utf-8',\n-\t\t\t-last_modified => $latest_date{'rfc2822'});\n+\t\t\t-last_modified => $latest_date{'rfc2822'},\n+\t\t\t-status => '200 OK');\n \t} else {\n \t\tprint $cgi->header(\n \t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8');\n+\t\t\t-charset => 'utf-8',\n+\t\t\t-status => '200 OK');\n \t}\n \n \t# Optimization: skip generating the body if client asks only\n-- \n1.7.3.4\n"},{"id":"187968","messageId":"d2b8995e12466f1518a50418c3e296d8d8983096.1332956550.git.wking@drexel.edu","threadId":"30083","inReplyTo":"20120328164513.GA4389@odin.tremily.us","subject":"[PATCH v7 2/3] gitweb: refactor If-Modified-Since handling","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T17:44:17Z","receivedAt":"2012-03-28T17:44:17Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"The current gitweb only generates Last-Modified and handles\nIf-Modified-Since headers for the git_feed action.  This patch breaks\nthe Last-Modified and If-Modified-Since handling code out from\ngit_feed into a new function exit_if_unmodified_since.  This makes the\ncode easy to reuse for other actions.\n\nOnly gitweb actions which can easily calculate a modification time\nshould use exit_if_unmodified_since, as the goal is to balance local\nprocessing time vs. upload bandwidth.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl                       |   57 +++++++++++++++--------------\n t/t9501-gitweb-standalone-http-status.sh |   27 +++++++++++++-\n 2 files changed, 55 insertions(+), 29 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 041da17..6d3f9c0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7003,6 +7003,28 @@ sub snapshot_name {\n \treturn wantarray ? ($name, $name) : $name;\n }\n \n+sub exit_if_unmodified_since {\n+\tmy ($latest_epoch) = @_;\n+\tour $cgi;\n+\n+\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n+\tif (defined $if_modified) {\n+\t\tmy $since;\n+\t\tif (eval { require HTTP::Date; 1; }) {\n+\t\t\t$since = HTTP::Date::str2time($if_modified);\n+\t\t} elsif (eval { require Time::ParseDate; 1; }) {\n+\t\t\t$since = Time::ParseDate::parsedate($if_modified, GMT => 1);\n+\t\t}\n+\t\tif (defined $since && $latest_epoch <= $since) {\n+\t\t\tmy %latest_date = parse_date($latest_epoch);\n+\t\t\tprint $cgi->header(\n+\t\t\t\t-last_modified => $latest_date{'rfc2822'},\n+\t\t\t\t-status => '304 Not Modified');\n+\t\t\tgoto DONE_GITWEB;\n+\t\t}\n+\t}\n+}\n+\n sub git_snapshot {\n \tmy $format = $input_params{'snapshot_format'};\n \tif (!@snapshot_fmts) {\n@@ -7820,35 +7842,14 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n-\t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n-\t\tif (defined $if_modified) {\n-\t\t\tmy $since;\n-\t\t\tif (eval { require HTTP::Date; 1; }) {\n-\t\t\t\t$since = HTTP::Date::str2time($if_modified);\n-\t\t\t} elsif (eval { require Time::ParseDate; 1; }) {\n-\t\t\t\t$since = Time::ParseDate::parsedate($if_modified, GMT => 1);\n-\t\t\t}\n-\t\t\tif (defined $since && $latest_epoch <= $since) {\n-\t\t\t\tprint $cgi->header(\n-\t\t\t\t\t-type => $content_type,\n-\t\t\t\t\t-charset => 'utf-8',\n-\t\t\t\t\t-last_modified => $latest_date{'rfc2822'},\n-\t\t\t\t\t-status => '304 Not Modified');\n-\t\t\t\treturn;\n-\t\t\t}\n-\t\t}\n-\t\tprint $cgi->header(\n-\t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8',\n-\t\t\t-last_modified => $latest_date{'rfc2822'},\n-\t\t\t-status => '200 OK');\n-\t} else {\n-\t\tprint $cgi->header(\n-\t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8',\n-\t\t\t-status => '200 OK');\n+\t\texit_if_unmodified_since($latest_epoch);\n+\t\t%latest_date = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n \t}\n+\tprint $cgi->header(\n+\t\t-type => $content_type,\n+\t\t-charset => 'utf-8',\n+\t\t%latest_date ? (-last_modified => $latest_date{'rfc2822'}) : (),\n+\t\t-status => '200 OK');\n \n \t# Optimization: skip generating the body if client asks only\n \t# for Last-Modified date.\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 31076ed..afa6bd4 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -92,7 +92,7 @@ test_debug 'cat gitweb.output'\n test_expect_success 'snapshots: bad tree-ish id (tagged object)' '\n \techo object > tag-object &&\n \tgit add tag-object &&\n-\tgit commit -m \"Object to be tagged\" &&\n+\ttest_tick && git commit -m \"Object to be tagged\" &&\n \tgit tag tagged-object `git hash-object tag-object` &&\n \tgitweb_run \"p=.git;a=snapshot;h=tagged-object;sf=tgz\" &&\n \tgrep \"400 - Object is not a tree-ish\" gitweb.output\n@@ -112,6 +112,31 @@ test_expect_success 'snapshots: bad object id' '\n '\n test_debug 'cat gitweb.output'\n \n+# ----------------------------------------------------------------------\n+# modification times (Last-Modified and If-Modified-Since)\n+\n+test_expect_success 'modification: feed last-modified' '\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 200 OK\" gitweb.output &&\n+\tgrep \"Last-modified: Thu, 7 Apr 2005 22:14:13 +0000\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: feed if-modified-since (modified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 200 OK\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: feed if-modified-since (unmodified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Thu, 7 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 304 Not Modified\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n \n # ----------------------------------------------------------------------\n # load checking\n-- \n1.7.3.4\n"},{"id":"187957","messageId":"81004880a876bb9b9f607129956363d3167bff72.1332956550.git.wking@drexel.edu","threadId":"30083","inReplyTo":"20120328164513.GA4389@odin.tremily.us","subject":"[PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T17:44:18Z","receivedAt":"2012-03-28T17:44:18Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"Because snapshots can be large, you can save some bandwidth by\nsupporting caching via If-Modified-Since.  This patch adds support for\nthe i-m-s request to git_snapshot() if the request is a commit.\nRequests for snapshots of trees, which lack well defined timestamps,\nare still handled as they were before.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl                       |   10 +++++++++\n t/t9501-gitweb-standalone-http-status.sh |   33 ++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6d3f9c0..b649f9e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7051,6 +7051,10 @@ sub git_snapshot {\n \n \tmy ($name, $prefix) = snapshot_name($project, $hash);\n \tmy $filename = \"$name$known_snapshot_formats{$format}{'suffix'}\";\n+\n+\tmy %co = parse_commit($hash);\n+\texit_if_unmodified_since($co{'committer_epoch'}) if %co;\n+\n \tmy $cmd = quote_command(\n \t\tgit_cmd(), 'archive',\n \t\t\"--format=$known_snapshot_formats{$format}{'format'}\",\n@@ -7060,9 +7064,15 @@ sub git_snapshot {\n \t}\n \n \t$filename =~ s/([\"\\\\])/\\\\$1/g;\n+\tmy %latest_date;\n+\tif (%co) {\n+\t\t%latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});\n+\t}\n+\n \tprint $cgi->header(\n \t\t-type => $known_snapshot_formats{$format}{'type'},\n \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n+\t\t-last_modified => (%co ? $latest_date{'rfc2822'} : ()),\n \t\t-status => '200 OK');\n \n \topen my $fd, \"-|\", $cmd\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex afa6bd4..1487820 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -138,6 +138,39 @@ test_expect_success 'modification: feed if-modified-since (unmodified)' '\n '\n test_debug 'cat gitweb.headers'\n \n+test_expect_success 'modification: snapshot last-modified' '\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.output &&\n+\tgrep \"Last-modified: Thu, 7 Apr 2005 22:14:13 +0000\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: snapshot if-modified-since (modified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: snapshot if-modified-since (unmodified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Thu, 7 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 304 Not Modified\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: tree snapshot' '\n+\tID=`git rev-parse --verify HEAD^{tree}` &&\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=$ID;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.output &&\n+\t! grep \"Last-Modified\" gitweb.output\n+'\n+test_debug 'cat gitweb.headers'\n+\n # ----------------------------------------------------------------------\n # load checking\n \n-- \n1.7.3.4\n"},{"id":"187962","messageId":"201203282011.32148.jnareb@gmail.com","threadId":"30083","inReplyTo":"81004880a876bb9b9f607129956363d3167bff72.1332956550.git.wking@drexel.edu","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-28T18:11:31Z","receivedAt":"2012-03-28T18:11:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W. Trevor King wrote:\n\n> @@ -7060,9 +7064,15 @@ sub git_snapshot {\n>  \t}\n>  \n>  \t$filename =~ s/([\"\\\\])/\\\\$1/g;\n> +\tmy %latest_date;\n> +\tif (%co) {\n> +\t\t%latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});\n> +\t}\n> +\n>  \tprint $cgi->header(\n>  \t\t-type => $known_snapshot_formats{$format}{'type'},\n>  \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n> +\t\t-last_modified => (%co ? $latest_date{'rfc2822'} : ()),\n>  \t\t-status => '200 OK');\n\nI'm sorry to be bearer of bad news, but this is still incorrect.\nIt should be:\n\n   \tprint $cgi->header(\n   \t\t-type => $known_snapshot_formats{$format}{'type'},\n   \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n  +\t\t%co ? (-last_modified => $latest_date{'rfc2822'}) : (),\n   \t\t-status => '200 OK');\n\n(The \"fat comma\" => operator has relatively low priority, lower than\nternary conditional operator ?:)\n  \n> +test_expect_success 'modification: tree snapshot' '\n> +\tID=`git rev-parse --verify HEAD^{tree}` &&\n> +\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n> +\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n> +\tgitweb_run \"p=.git;a=snapshot;h=$ID;sf=tgz\" &&\n> +\tgrep \"Status: 200 OK\" gitweb.output &&\n> +\t! grep \"Last-Modified\" gitweb.output\n> +'\n> +test_debug 'cat gitweb.headers'\n\nAnd it was not caught by test because CGI.pm can output the last modified\nheader as \"Last-modified\" (RFC 2616, sec 4.2 states \"Field names are\ncase-insensitive\"), so the last check should be\n\n  +\t! grep -i \"Last-Modified\" gitweb.output\n\nHmmm... why we use gitweb.output and not gitweb.headers?  Is it consistency\nwith earlier tests?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"187966","messageId":"20120328183712.GA5992@odin.tremily.us","threadId":"30083","inReplyTo":"201203282011.32148.jnareb@gmail.com","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T18:37:12Z","receivedAt":"2012-03-28T18:37:12Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"On Wed, Mar 28, 2012 at 07:11:31PM +0100, Jakub Narebski wrote:\n> W. Trevor King wrote:\n> >  \tprint $cgi->header(\n> >  \t\t-type => $known_snapshot_formats{$format}{'type'},\n> >  \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n> > +\t\t-last_modified => (%co ? $latest_date{'rfc2822'} : ()),\n> >  \t\t-status => '200 OK');\n> \n> I'm sorry to be bearer of bad news, but this is still incorrect.\n> It should be:\n> \n>    \tprint $cgi->header(\n>    \t\t-type => $known_snapshot_formats{$format}{'type'},\n>    \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n>   +\t\t%co ? (-last_modified => $latest_date{'rfc2822'}) : (),\n>    \t\t-status => '200 OK');\n\nGrr.  Thanks.  I'm getting lots of rebase practice on this patch set,\nbut I'm still missing things…\n\n> And it was not caught by test because CGI.pm can output the last modified\n> header as \"Last-modified\" (RFC 2616, sec 4.2 states \"Field names are\n> case-insensitive\"), so the last check should be\n> \n>   +\t! grep -i \"Last-Modified\" gitweb.output\n> \n> Hmmm... why we use gitweb.output and not gitweb.headers?  Is it consistency\n> with earlier tests?\n\nYes, but I can switch to `gitweb.headers` if you'd like.  Should I\nadjust all the header tests in t9501 to use `gitweb.headers` and `grep\n-i`?  It should probably be a separate patch for the tests that\nexisted before my i-m-s additions.\n\n-- \nThis email may be signed or encrypted with GnuPG (http://www.gnupg.org).\nFor more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\n"},{"id":"187971","messageId":"7vr4wcv8v4.fsf@alter.siamese.dyndns.org","threadId":"30083","inReplyTo":"20120328183712.GA5992@odin.tremily.us","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-28T19:11:27Z","receivedAt":"2012-03-28T19:11:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"W. Trevor King\" <wking@drexel.edu> writes:\n\n> Grr.  Thanks.  I'm getting lots of rebase practice on this patch set,\n> but I'm still missing things…\n\nHow do you \"rebase\"?\n\nIt often is the easiest to check out the tip of the previous iteration,\nfix all issues that were brought up in the working tree, eyeball the\noutput from \"git diff HEAD\" to make sure you addressed all the comments,o\nand then make separate commits, using \"add -p\" to sift the fix-ups\naccording to which commit in the previous round they need to update.\n\nAnd then finally you run \"rebase -i\" to squash these fix-ups in.\n"},{"id":"187975","messageId":"20120328192750.GA6909@odin.tremily.us","threadId":"30083","inReplyTo":"7vr4wcv8v4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-28T19:27:51Z","receivedAt":"2012-03-28T19:27:51Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"On Wed, Mar 28, 2012 at 12:11:27PM -0700, Junio C Hamano wrote:\n> \"W. Trevor King\" <wking@drexel.edu> writes:\n> \n> > Grr.  Thanks.  I'm getting lots of rebase practice on this patch set,\n> > but I'm still missing things…\n> \n> How do you \"rebase\"?\n> \n> It often is the easiest to check out the tip of the previous iteration,\n> fix all issues that were brought up in the working tree, eyeball the\n> output from \"git diff HEAD\" to make sure you addressed all the comments,o\n> and then make separate commits, using \"add -p\" to sift the fix-ups\n> according to which commit in the previous round they need to update.\n> \n> And then finally you run \"rebase -i\" to squash these fix-ups in.\n\nAh, that makes a lot of sense.  I had been running `rebase -i`,\nediting the earlier commits, and using `commit -a --amend` to squash\nthem on.  The problem with that approach is that you need to check the\nchanges vs the previous release before each amend, while with your\nsuggestion there's a single diff to look through.\n\nHopefully patches v8+ will be cleaner ;).\n\n-- \nThis email may be signed or encrypted with GnuPG (http://www.gnupg.org).\nFor more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\n"},{"id":"188008","messageId":"201203282328.08876.jnareb@gmail.com","threadId":"30083","inReplyTo":"20120328183712.GA5992@odin.tremily.us","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-28T21:28:08Z","receivedAt":"2012-03-28T21:28:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 28 Mar 2012, W. Trevor King wrote:\n> On Wed, Mar 28, 2012 at 07:11:31PM +0100, Jakub Narebski wrote:\n\n[...]\n> > And it was not caught by test because CGI.pm can output the last modified\n> > header as \"Last-modified\" (RFC 2616, sec 4.2 states \"Field names are\n> > case-insensitive\"), so the last check should be\n> > \n> >   +\t! grep -i \"Last-Modified\" gitweb.output\n> > \n> > Hmmm... why we use gitweb.output and not gitweb.headers?  Is it consistency\n> > with earlier tests?\n> \n> Yes, but I can switch to `gitweb.headers` if you'd like.  Should I\n> adjust all the header tests in t9501 to use `gitweb.headers` and `grep\n> -i`?  It should probably be a separate patch for the tests that\n> existed before my i-m-s additions.\n\nEh, don't worry about this.  First, I think we can assume that HTTP\nheaders from CGI.pm will all start with capital letter.\n\nSecond, for positive match being overly strict is safe - if assumption\ndoesn't hold we would get false failure.  The problem is for negative\nmatch - being overly strict means that we won't catch the breakage.\n\nI think that the gitweb.output vs gitweb.headers (and gitweb.body) is\nbecause those tests predate gitweb_run producing gitweb.headers file.\nBe consistent if you want, or use new feature in new test; you don't\nneed to modernize t9501.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188012","messageId":"201203282345.49673.jnareb@gmail.com","threadId":"30083","inReplyTo":"20120328192750.GA6909@odin.tremily.us","subject":"Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-28T21:45:49Z","receivedAt":"2012-03-28T21:45:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 28 Mar 2012, W. Trevor King wrote:\n> On Wed, Mar 28, 2012 at 12:11:27PM -0700, Junio C Hamano wrote:\n> > \"W. Trevor King\" <wking@drexel.edu> writes:\n> > \n> > > Grr.  Thanks.  I'm getting lots of rebase practice on this patch set,\n> > > but I'm still missing things…\n> > \n> > How do you \"rebase\"?\n> > \n> > It often is the easiest to check out the tip of the previous iteration,\n> > fix all issues that were brought up in the working tree, eyeball the\n> > output from \"git diff HEAD\" to make sure you addressed all the comments,o\n> > and then make separate commits, using \"add -p\" to sift the fix-ups\n> > according to which commit in the previous round they need to update.\n> > \n> > And then finally you run \"rebase -i\" to squash these fix-ups in.\n> \n> Ah, that makes a lot of sense.  I had been running `rebase -i`,\n> editing the earlier commits, and using `commit -a --amend` to squash\n> them on.  The problem with that approach is that you need to check the\n> changes vs the previous release before each amend, while with your\n> suggestion there's a single diff to look through.\n\nBTW. I personally use StGit (a patch management interface on top of git)\ninstead of interactive rebase.  Just in case, and to be able to write\ndifferences to previous version, I use git-format-patch to generate\npatches to a subdirectory, e.g. mdir.gitweb.v7/, and compare with previous\nversion.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188058","messageId":"cover.1333024238.git.wking@drexel.edu","threadId":"30083","inReplyTo":"201203282328.08876.jnareb@gmail.com","subject":"[PATCH v8 0/3] gitweb: refactor If-Modified-Since handling","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-29T12:45:46Z","receivedAt":"2012-03-29T12:45:46Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"Changes since v7:\n* Fix git_snapshot `-last_modified =>` switching.\n* Case insensitive Last-Modified match for tree snapshot test.\n* gitweb.output -> gitweb.headers in the new tests.\n\n\nW. Trevor King (3):\n  gitweb: add `status` headers to git_feed() responses.\n  gitweb: refactor If-Modified-Since handling\n  gitweb: add If-Modified-Since handling to git_snapshot().\n\n gitweb/gitweb.perl                       |   65 ++++++++++++++++++------------\n t/t9501-gitweb-standalone-http-status.sh |   60 +++++++++++++++++++++++++++-\n 2 files changed, 98 insertions(+), 27 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"188056","messageId":"4958e06536924de10105bb453d88e8f4c28bb8c1.1333024238.git.wking@drexel.edu","threadId":"30083","inReplyTo":"201203282328.08876.jnareb@gmail.com","subject":"[PATCH v8 1/3] gitweb: add `status` headers to git_feed() responses.","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-29T12:45:47Z","receivedAt":"2012-03-29T12:45:47Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"The git_feed() method was not setting a `Status` header unless it was\nresponding to an If-Modified-Since request with `304 Not Modified`.\nNow, when it is serving successful responses, it sets status to `200\nOK`.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a8b5fad..041da17 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7841,11 +7841,13 @@ sub git_feed {\n \t\tprint $cgi->header(\n \t\t\t-type => $content_type,\n \t\t\t-charset => 'utf-8',\n-\t\t\t-last_modified => $latest_date{'rfc2822'});\n+\t\t\t-last_modified => $latest_date{'rfc2822'},\n+\t\t\t-status => '200 OK');\n \t} else {\n \t\tprint $cgi->header(\n \t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8');\n+\t\t\t-charset => 'utf-8',\n+\t\t\t-status => '200 OK');\n \t}\n \n \t# Optimization: skip generating the body if client asks only\n-- \n1.7.3.4\n"},{"id":"188054","messageId":"8839345a047a7dee825565032029b1c08890a0f0.1333024238.git.wking@drexel.edu","threadId":"30083","inReplyTo":"201203282328.08876.jnareb@gmail.com","subject":"[PATCH v8 2/3] gitweb: refactor If-Modified-Since handling","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-29T12:45:48Z","receivedAt":"2012-03-29T12:45:48Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"The current gitweb only generates Last-Modified and handles\nIf-Modified-Since headers for the git_feed action.  This patch breaks\nthe Last-Modified and If-Modified-Since handling code out from\ngit_feed into a new function exit_if_unmodified_since.  This makes the\ncode easy to reuse for other actions.\n\nOnly gitweb actions which can easily calculate a modification time\nshould use exit_if_unmodified_since, as the goal is to balance local\nprocessing time vs. upload bandwidth.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl                       |   57 +++++++++++++++--------------\n t/t9501-gitweb-standalone-http-status.sh |   27 +++++++++++++-\n 2 files changed, 55 insertions(+), 29 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 041da17..6d3f9c0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7003,6 +7003,28 @@ sub snapshot_name {\n \treturn wantarray ? ($name, $name) : $name;\n }\n \n+sub exit_if_unmodified_since {\n+\tmy ($latest_epoch) = @_;\n+\tour $cgi;\n+\n+\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n+\tif (defined $if_modified) {\n+\t\tmy $since;\n+\t\tif (eval { require HTTP::Date; 1; }) {\n+\t\t\t$since = HTTP::Date::str2time($if_modified);\n+\t\t} elsif (eval { require Time::ParseDate; 1; }) {\n+\t\t\t$since = Time::ParseDate::parsedate($if_modified, GMT => 1);\n+\t\t}\n+\t\tif (defined $since && $latest_epoch <= $since) {\n+\t\t\tmy %latest_date = parse_date($latest_epoch);\n+\t\t\tprint $cgi->header(\n+\t\t\t\t-last_modified => $latest_date{'rfc2822'},\n+\t\t\t\t-status => '304 Not Modified');\n+\t\t\tgoto DONE_GITWEB;\n+\t\t}\n+\t}\n+}\n+\n sub git_snapshot {\n \tmy $format = $input_params{'snapshot_format'};\n \tif (!@snapshot_fmts) {\n@@ -7820,35 +7842,14 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n-\t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n-\t\tif (defined $if_modified) {\n-\t\t\tmy $since;\n-\t\t\tif (eval { require HTTP::Date; 1; }) {\n-\t\t\t\t$since = HTTP::Date::str2time($if_modified);\n-\t\t\t} elsif (eval { require Time::ParseDate; 1; }) {\n-\t\t\t\t$since = Time::ParseDate::parsedate($if_modified, GMT => 1);\n-\t\t\t}\n-\t\t\tif (defined $since && $latest_epoch <= $since) {\n-\t\t\t\tprint $cgi->header(\n-\t\t\t\t\t-type => $content_type,\n-\t\t\t\t\t-charset => 'utf-8',\n-\t\t\t\t\t-last_modified => $latest_date{'rfc2822'},\n-\t\t\t\t\t-status => '304 Not Modified');\n-\t\t\t\treturn;\n-\t\t\t}\n-\t\t}\n-\t\tprint $cgi->header(\n-\t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8',\n-\t\t\t-last_modified => $latest_date{'rfc2822'},\n-\t\t\t-status => '200 OK');\n-\t} else {\n-\t\tprint $cgi->header(\n-\t\t\t-type => $content_type,\n-\t\t\t-charset => 'utf-8',\n-\t\t\t-status => '200 OK');\n+\t\texit_if_unmodified_since($latest_epoch);\n+\t\t%latest_date = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n \t}\n+\tprint $cgi->header(\n+\t\t-type => $content_type,\n+\t\t-charset => 'utf-8',\n+\t\t%latest_date ? (-last_modified => $latest_date{'rfc2822'}) : (),\n+\t\t-status => '200 OK');\n \n \t# Optimization: skip generating the body if client asks only\n \t# for Last-Modified date.\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 31076ed..3580103 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -92,7 +92,7 @@ test_debug 'cat gitweb.output'\n test_expect_success 'snapshots: bad tree-ish id (tagged object)' '\n \techo object > tag-object &&\n \tgit add tag-object &&\n-\tgit commit -m \"Object to be tagged\" &&\n+\ttest_tick && git commit -m \"Object to be tagged\" &&\n \tgit tag tagged-object `git hash-object tag-object` &&\n \tgitweb_run \"p=.git;a=snapshot;h=tagged-object;sf=tgz\" &&\n \tgrep \"400 - Object is not a tree-ish\" gitweb.output\n@@ -112,6 +112,31 @@ test_expect_success 'snapshots: bad object id' '\n '\n test_debug 'cat gitweb.output'\n \n+# ----------------------------------------------------------------------\n+# modification times (Last-Modified and If-Modified-Since)\n+\n+test_expect_success 'modification: feed last-modified' '\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 200 OK\" gitweb.headers &&\n+\tgrep \"Last-modified: Thu, 7 Apr 2005 22:14:13 +0000\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: feed if-modified-since (modified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 200 OK\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: feed if-modified-since (unmodified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Thu, 7 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=atom;h=master\" &&\n+\tgrep \"Status: 304 Not Modified\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n \n # ----------------------------------------------------------------------\n # load checking\n-- \n1.7.3.4\n"},{"id":"188055","messageId":"bfbde5354e25dfbf535307a72016a6b5ac3a2c56.1333024238.git.wking@drexel.edu","threadId":"30083","inReplyTo":"201203282328.08876.jnareb@gmail.com","subject":"[PATCH v8 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"W. Trevor King","fromEmail":"wking@drexel.edu","sentAt":"2012-03-29T12:45:49Z","receivedAt":"2012-03-29T12:45:49Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"Because snapshots can be large, you can save some bandwidth by\nsupporting caching via If-Modified-Since.  This patch adds support for\nthe i-m-s request to git_snapshot() if the request is a commit.\nRequests for snapshots of trees, which lack well defined timestamps,\nare still handled as they were before.\n\nSigned-off-by: W Trevor King <wking@drexel.edu>\n---\n gitweb/gitweb.perl                       |   10 +++++++++\n t/t9501-gitweb-standalone-http-status.sh |   33 ++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6d3f9c0..ede804a 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7051,6 +7051,10 @@ sub git_snapshot {\n \n \tmy ($name, $prefix) = snapshot_name($project, $hash);\n \tmy $filename = \"$name$known_snapshot_formats{$format}{'suffix'}\";\n+\n+\tmy %co = parse_commit($hash);\n+\texit_if_unmodified_since($co{'committer_epoch'}) if %co;\n+\n \tmy $cmd = quote_command(\n \t\tgit_cmd(), 'archive',\n \t\t\"--format=$known_snapshot_formats{$format}{'format'}\",\n@@ -7060,9 +7064,15 @@ sub git_snapshot {\n \t}\n \n \t$filename =~ s/([\"\\\\])/\\\\$1/g;\n+\tmy %latest_date;\n+\tif (%co) {\n+\t\t%latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});\n+\t}\n+\n \tprint $cgi->header(\n \t\t-type => $known_snapshot_formats{$format}{'type'},\n \t\t-content_disposition => 'inline; filename=\"' . $filename . '\"',\n+\t\t%co ? (-last_modified => $latest_date{'rfc2822'}) : (),\n \t\t-status => '200 OK');\n \n \topen my $fd, \"-|\", $cmd\ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 3580103..fa2f65f 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -138,6 +138,39 @@ test_expect_success 'modification: feed if-modified-since (unmodified)' '\n '\n test_debug 'cat gitweb.headers'\n \n+test_expect_success 'modification: snapshot last-modified' '\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.headers &&\n+\tgrep \"Last-modified: Thu, 7 Apr 2005 22:14:13 +0000\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: snapshot if-modified-since (modified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: snapshot if-modified-since (unmodified)' '\n+\texport HTTP_IF_MODIFIED_SINCE=\"Thu, 7 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=master;sf=tgz\" &&\n+\tgrep \"Status: 304 Not Modified\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n+test_expect_success 'modification: tree snapshot' '\n+\tID=`git rev-parse --verify HEAD^{tree}` &&\n+\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n+\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n+\tgitweb_run \"p=.git;a=snapshot;h=$ID;sf=tgz\" &&\n+\tgrep \"Status: 200 OK\" gitweb.headers &&\n+\t! grep -i \"last-modified\" gitweb.headers\n+'\n+test_debug 'cat gitweb.headers'\n+\n # ----------------------------------------------------------------------\n # load checking\n \n-- \n1.7.3.4\n"},{"id":"188165","messageId":"201203301721.52657.jnareb@gmail.com","threadId":"30083","inReplyTo":"4958e06536924de10105bb453d88e8f4c28bb8c1.1333024238.git.wking@drexel.edu","subject":"Re: [PATCH v8 1/3] gitweb: add `status` headers to git_feed() responses.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-30T15:21:50Z","receivedAt":"2012-03-30T15:21:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 29 Mar 2012, W. Trevor King wrote:\n\n> The git_feed() method was not setting a `Status` header unless it was\n> responding to an If-Modified-Since request with `304 Not Modified`.\n> Now, when it is serving successful responses, it sets status to\n> `200 OK`.\n\nNice.  This change is IMHO worth applying even without the rest of series.\nSo\n\n  Acked-by: Jakub Narebski <jnareb@gmail.com>\n\nYou _might_ also add that this change would allow robust testing of\nIf-Modified-Since request handling in gitweb, but it is not really\nnecessary.\n\n> \n> Signed-off-by: W Trevor King <wking@drexel.edu>\n> ---\n>  gitweb/gitweb.perl |    6 ++++--\n>  1 files changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index a8b5fad..041da17 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -7841,11 +7841,13 @@ sub git_feed {\n>  \t\tprint $cgi->header(\n>  \t\t\t-type => $content_type,\n>  \t\t\t-charset => 'utf-8',\n> -\t\t\t-last_modified => $latest_date{'rfc2822'});\n> +\t\t\t-last_modified => $latest_date{'rfc2822'},\n> +\t\t\t-status => '200 OK');\n>  \t} else {\n>  \t\tprint $cgi->header(\n>  \t\t\t-type => $content_type,\n> -\t\t\t-charset => 'utf-8');\n> +\t\t\t-charset => 'utf-8',\n> +\t\t\t-status => '200 OK');\n>  \t}\n>  \n>  \t# Optimization: skip generating the body if client asks only\n> -- \n> 1.7.3.4\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188166","messageId":"201203301730.52615.jnareb@gmail.com","threadId":"30083","inReplyTo":"8839345a047a7dee825565032029b1c08890a0f0.1333024238.git.wking@drexel.edu","subject":"Re: [PATCH v8 2/3] gitweb: refactor If-Modified-Since handling","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-30T15:30:51Z","receivedAt":"2012-03-30T15:30:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W. Trevor King wrote:\n\n> The current gitweb only generates Last-Modified and handles\n> If-Modified-Since headers for the git_feed action.  This patch breaks\n> the Last-Modified and If-Modified-Since handling code out from\n> git_feed into a new function exit_if_unmodified_since.  This makes the\n> code easy to reuse for other actions.\n> \n> Only gitweb actions which can easily calculate a modification time\n> should use exit_if_unmodified_since, as the goal is to balance local\n> processing time vs. upload bandwidth.\n> \n> Signed-off-by: W Trevor King <wking@drexel.edu>\n\nNice and clear.\n\n  Acked-by: Jakub Narebski <jnareb@gmail.com>\n\nPerhaps the second paragraph of the commit message would read better if\nit started with \"Note that only ...\" or something like that, but this\nis not that important, and not worth another round.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188169","messageId":"201203301807.55284.jnareb@gmail.com","threadId":"30083","inReplyTo":"bfbde5354e25dfbf535307a72016a6b5ac3a2c56.1333024238.git.wking@drexel.edu","subject":"Re: [PATCH v8 3/3] gitweb: add If-Modified-Since handling to git_snapshot().","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-30T16:07:54Z","receivedAt":"2012-03-30T16:07:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 29 Mar 2012, W. Trevor King wrote:\n\n> Because snapshots can be large, you can save some bandwidth by\n> supporting caching via If-Modified-Since.  This patch adds support for\n> the i-m-s request to git_snapshot() if the request is a commit.\n>\nPerhaps it is worth clarifying that \"caching\" here means external HTTP\ncaching, either by web browser, or by web accelerator / reverse proxy.\nSupporting Last-Modified and If-Modified-Since helps that[1][2].\n\n[1]: http://www.mnot.net/cache_docs/#VALIDATE\n[2]: http://www.mnot.net/cache_docs/#SCRIPT\n\n> Requests for snapshots of trees, which lack well defined timestamps,\n> are still handled as they were before.\n\n\"still handled as they were before\" means \"do not support l-m / i-m-s\",\nisn't it?  Wouldn't it be better to write it explicitely?\n> \n> Signed-off-by: W Trevor King <wking@drexel.edu>\n\nAnyway, I like those changes:\n\n  Acked-by: Jakub Narebski <jnareb@gmail.com>\n\n[...]\n> +test_expect_success 'modification: tree snapshot' '\n> +\tID=`git rev-parse --verify HEAD^{tree}` &&\n> +\texport HTTP_IF_MODIFIED_SINCE=\"Wed, 6 Apr 2005 22:14:13 +0000\" &&\n> +\ttest_when_finished \"unset HTTP_IF_MODIFIED_SINCE\" &&\n> +\tgitweb_run \"p=.git;a=snapshot;h=$ID;sf=tgz\" &&\n> +\tgrep \"Status: 200 OK\" gitweb.headers &&\n> +\t! grep -i \"last-modified\" gitweb.headers\n\nIf we ignore case, we can write\n\n  +\t! grep -i \"Last-Modified:\" gitweb.headers\n\nwhich is IMVVVHO slightly more readable.\n\nNot that it matters much.  Just nitpicking.\n\n> +'\n> +test_debug 'cat gitweb.headers'\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188170","messageId":"7vzkaym5kk.fsf@alter.siamese.dyndns.org","threadId":"30083","inReplyTo":"cover.1333024238.git.wking@drexel.edu","subject":"Re: [PATCH v8 0/3] gitweb: refactor If-Modified-Since handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-30T16:11:55Z","receivedAt":"2012-03-30T16:11:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, both.  Looking good.  Will queue.\n"}]}