{"thread":{"id":"17368","subject":"[PATCHv2 1/6] gitweb: channel image in rss feed","startedAt":"2009-01-26T11:50:10Z","lastAt":"2009-02-12T11:23:47Z","messageCount":37,"participants":["Giuseppe Bilotta","Junio C Hamano","Jakub Narebski","Deskin Miller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"101971","messageId":"1232970616-21167-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":null,"subject":"[PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:10Z","receivedAt":"2009-01-26T11:50:10Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This second revision adds two patches to improve client-side rss\ncaching: the last-modified header we output is based on commit rather\nthan creation time, and we now act on if-modified-since request headers.\n\nThe last patch requires either HTTP::Date (from libwww-perl) or\nTime::ParseDate\n\nGiuseppe Bilotta (6):\n  gitweb: channel image in rss feed\n  gitweb: feed generator metadata\n  gitweb: rss feed managingEditor\n  gitweb: rss channel date\n  gitweb: last-modified time should be commiter, not author\n  gitweb: check if-modified-since for feeds\n\n gitweb/gitweb.perl |   40 ++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 38 insertions(+), 2 deletions(-)\n"},{"id":"101969","messageId":"1232970616-21167-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 1/6] gitweb: channel image in rss feed","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:11Z","receivedAt":"2009-01-26T11:50:11Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Define the channel image for the rss feed when the logo or favicon are\ndefined, preferring the former to the latter. As suggested in the RSS\n2.0 specifications, the image's title and link as set to the same as the\nchannel's.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 931db4f..f8a5d2e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6075,6 +6075,16 @@ XML\n \t\t      \"<link>$alt_url</link>\\n\" .\n \t\t      \"<description>$descr</description>\\n\" .\n \t\t      \"<language>en</language>\\n\";\n+\t\tif (defined $logo || defined $favicon) {\n+\t\t\t# prefer the logo to the favicon, since RSS\n+\t\t\t# doesn't allow both\n+\t\t\tmy $img = esc_url($logo || $favicon);\n+\t\t\tprint \"<image>\\n\" .\n+\t\t\t      \"<url>$img</url>\\n\" .\n+\t\t\t      \"<title>$title</title>\\n\" .\n+\t\t\t      \"<link>$alt_url</link>\\n\" .\n+\t\t\t      \"</image>\\n\";\n+\t\t}\n \t} elsif ($format eq 'atom') {\n \t\tprint <<XML;\n <feed xmlns=\"http://www.w3.org/2005/Atom\">\n-- \n1.5.6.5\n"},{"id":"101970","messageId":"1232970616-21167-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 2/6] gitweb: feed generator metadata","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:12Z","receivedAt":"2009-01-26T11:50:12Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Add <generator> tag to RSS and Atom feed. Versioning info (gitweb/git\ncore versions, separated by a literal slash) is stored in the\nappropriate attribute for the Atom feed, and in the tag content for the\nRSS feed.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f8a5d2e..3d94f50 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6085,6 +6085,7 @@ XML\n \t\t\t      \"<link>$alt_url</link>\\n\" .\n \t\t\t      \"</image>\\n\";\n \t\t}\n+\t\tprint \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n \t} elsif ($format eq 'atom') {\n \t\tprint <<XML;\n <feed xmlns=\"http://www.w3.org/2005/Atom\">\n@@ -6111,6 +6112,7 @@ XML\n \t\t} else {\n \t\t\tprint \"<updated>$latest_date{'iso-8601'}</updated>\\n\";\n \t\t}\n+\t\tprint \"<generator version='$version/$git_version'>gitweb</generator>\\n\";\n \t}\n \n \t# contents\n-- \n1.5.6.5\n"},{"id":"101973","messageId":"1232970616-21167-4-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 3/6] gitweb: rss feed managingEditor","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:13Z","receivedAt":"2009-01-26T11:50:13Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The RSS 2.0 specification allows an optional managingEditor tag for the\nchannel, containing the \"email address for person responsible for editorial\ncontent\", which is basically the project owner.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3d94f50..cc6d0fb 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6074,7 +6074,9 @@ XML\n \t\tprint \"<title>$title</title>\\n\" .\n \t\t      \"<link>$alt_url</link>\\n\" .\n \t\t      \"<description>$descr</description>\\n\" .\n-\t\t      \"<language>en</language>\\n\";\n+\t\t      \"<language>en</language>\\n\" .\n+\t\t      # project owner is responsible for 'editorial' content\n+\t\t      \"<managingEditor>$owner</managingEditor>\\n\";\n \t\tif (defined $logo || defined $favicon) {\n \t\t\t# prefer the logo to the favicon, since RSS\n \t\t\t# doesn't allow both\n-- \n1.5.6.5\n"},{"id":"101974","messageId":"1232970616-21167-5-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-4-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 4/6] gitweb: rss channel date","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:14Z","receivedAt":"2009-01-26T11:50:14Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The RSS 2.0 specifications defines not one but _two_ dates for its\nchannel element! Woohoo! Luckily, it seems that consensus seems to be\nthat if both are present they should be equal, except for some very\nobscure and discouraged cases. Since lastBuildDate would make more sense\nfor us and pubDate seems to be the most commonly used, we defined both\nand make them equal.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cc6d0fb..756868a 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6087,6 +6087,10 @@ XML\n \t\t\t      \"<link>$alt_url</link>\\n\" .\n \t\t\t      \"</image>\\n\";\n \t\t}\n+\t\tif (%latest_date) {\n+\t\t\tprint \"<pubDate>$latest_date{'rfc2822'}</pubDate>\\n\";\n+\t\t\tprint \"<lastBuildDate>$latest_date{'rfc2822'}</lastBuildDate>\\n\";\n+\t\t}\n \t\tprint \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n \t} elsif ($format eq 'atom') {\n \t\tprint <<XML;\n-- \n1.5.6.5\n"},{"id":"101972","messageId":"1232970616-21167-6-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-5-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:15Z","receivedAt":"2009-01-26T11:50:15Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The last-modified time header added by RSS to increase cache hits from\nreaders should be set to the date the repository was last modified. The\nauthor time in this respect is not a good guess because the last commit\nmight come from a oldish patch.\n\nUse the committer time for the last-modified header to ensure a more\ncorrect guess of the last time the repository was modified.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 756868a..8c49c75 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6015,7 +6015,7 @@ sub git_feed {\n \t}\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n-\t\t%latest_date   = parse_date($latest_commit{'author_epoch'});\n+\t\t%latest_date   = parse_date($latest_commit{'committer_epoch'});\n \t\tprint $cgi->header(\n \t\t\t-type => $content_type,\n \t\t\t-charset => 'utf-8',\n-- \n1.5.6.5\n"},{"id":"101975","messageId":"1232970616-21167-7-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-6-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv2 6/6] gitweb: check if-modified-since for feeds","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-26T11:50:16Z","receivedAt":"2009-01-26T11:50:16Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Offering Last-modified header for feeds is only half the work, even if\nwe bail out early on HEAD requests. We should also check that same date\nagainst If-modified-since, and bail out early with 304 Not Modified if\nthat's the case.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |   20 +++++++++++++++++++-\n 1 files changed, 19 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8c49c75..f4defb0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6015,7 +6015,25 @@ sub git_feed {\n \t}\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n-\t\t%latest_date   = parse_date($latest_commit{'committer_epoch'});\n+\t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n+\t\t%latest_date   = parse_date($latest_epoch);\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-- \n1.5.6.5\n"},{"id":"102348","messageId":"7vskn3m9l1.fsf@gitster.siamese.dyndns.org","threadId":"17368","inReplyTo":"1232970616-21167-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T20:58:34Z","receivedAt":"2009-01-28T20:58:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is independent from your other topic on PATH_INFO, right?  I am aware\nthat other one is being polished between you and Jakub, but is anobody\ndoing anything on this one, or is it already polished enough?\n"},{"id":"102356","messageId":"200901282248.18611.jnareb@gmail.com","threadId":"17368","inReplyTo":"7vskn3m9l1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-28T21:48:17Z","receivedAt":"2009-01-28T21:48:17Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 28 Jan 2009, Junio C Hamano wrote:\n> This is independent from your other topic on PATH_INFO, right?\n\nYes, it is independent on adding BASE for PATH_INFO.\n\n> I am aware that other one is being polished between you and Jakub,\n\nThe code (in BASE for PATH_INFO) looks now good, only commit\nmessage might be further improved.\n\n> but is anobody doing anything on this one, or is it already polished enough?\n\nI think it is good, but I do not use web feeds (Atom and RSS)\nfrom gitweb myself, therefore I don't feel competent to tell\nif it is good enough. With exception of last one there are\nquite simple and feel good; the last one is a bit more involved,\nbut I think also good (it would be nice to say in commit\nmessage that HTTP::Date can be found in perl-libwww-perl).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"102357","messageId":"cb7bb73a0901281357w52aa275i1a734b171396dd58@mail.gmail.com","threadId":"17368","inReplyTo":"7vskn3m9l1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-28T21:57:33Z","receivedAt":"2009-01-28T21:57:33Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Jan 28, 2009 at 9:58 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> This is independent from your other topic on PATH_INFO, right?  I am aware\n> that other one is being polished between you and Jakub, but is anobody\n> doing anything on this one, or is it already polished enough?\n\nYes it's a totally independent topic. For what it's worth, this series\nis running on ruby-rbot.org gitweb and it fixed a bunch of rss polling\nrelated issues we were having with the bot we use to watch it.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"102358","messageId":"7vfxj3ks0b.fsf@gitster.siamese.dyndns.org","threadId":"17368","inReplyTo":"cb7bb73a0901281357w52aa275i1a734b171396dd58@mail.gmail.com","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T22:03:32Z","receivedAt":"2009-01-28T22:03:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ok, let's apply it directly to 'master', then.\n\nThanks.\n"},{"id":"103223","messageId":"200902042356.34407.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 1/6] gitweb: channel image in rss feed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-04T22:56:31Z","receivedAt":"2009-02-04T22:56:31Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"I'm sorry for the delay reviewing this series.\n\nOn Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> Define the channel image for the rss feed when the logo or favicon are\n> defined, preferring the former to the latter. As suggested in the RSS\n> 2.0 specifications, the image's title and link as set to the same as the\n> channel's.\n\nI think it is a very good idea.\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n\nI don't personally use either RSS feeds, or Atom feeds from gitweb,\ntherefore I don't feel like I am able to ack this changes... but I do\nlike them.\n\n> ---\n>  gitweb/gitweb.perl |   10 ++++++++++\n>  1 files changed, 10 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 931db4f..f8a5d2e 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6075,6 +6075,16 @@ XML\n>  \t\t      \"<link>$alt_url</link>\\n\" .\n>  \t\t      \"<description>$descr</description>\\n\" .\n>  \t\t      \"<language>en</language>\\n\";\n> +\t\tif (defined $logo || defined $favicon) {\n> +\t\t\t# prefer the logo to the favicon, since RSS\n> +\t\t\t# doesn't allow both\n> +\t\t\tmy $img = esc_url($logo || $favicon);\n> +\t\t\tprint \"<image>\\n\" .\n> +\t\t\t      \"<url>$img</url>\\n\" .\n> +\t\t\t      \"<title>$title</title>\\n\" .\n> +\t\t\t      \"<link>$alt_url</link>\\n\" .\n> +\t\t\t      \"</image>\\n\";\n> +\t\t}\n\nA question: how should URL, which is _contents_ of a tag (instead of\nas _attribute_ value), quoted (escaped)? I don't think it is specified\nin RSS. I guess that using esc_url() is a safe solution.\n\n>  \t} elsif ($format eq 'atom') {\n>  \t\tprint <<XML;\n>  <feed xmlns=\"http://www.w3.org/2005/Atom\">\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103227","messageId":"200902050015.20170.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 2/6] gitweb: feed generator metadata","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-04T23:15:19Z","receivedAt":"2009-02-04T23:15:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> Add <generator> tag to RSS and Atom feed. Versioning info (gitweb/git\n> core versions, separated by a literal slash) is stored in the\n> appropriate attribute for the Atom feed, and in the tag content for the\n> RSS feed.\n\nVery good idea. I haven't examined either specification, so I don't\nknow what conventions are used, though... and what conventions _should_\nbe used.\n\nBy the way, gitweb uses in HTML header the following (see\ngit_header_html subroutine):\n\n  <meta name=\"generator\" content=\"gitweb/$version git/$git_version$mod_perl_version\"/>\n\nwhich tries to follow convention how _web servers_ like Apache return\nversion information in the 'Server:' HTTP response header (product\ntokens). Because it was used on only one place, it was not put into\nseparate subroutine; should it now?\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    2 ++\n>  1 files changed, 2 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index f8a5d2e..3d94f50 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6085,6 +6085,7 @@ XML\n>  \t\t\t      \"<link>$alt_url</link>\\n\" .\n>  \t\t\t      \"</image>\\n\";\n>  \t\t}\n> +\t\tprint \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n>  \t} elsif ($format eq 'atom') {\n>  \t\tprint <<XML;\n>  <feed xmlns=\"http://www.w3.org/2005/Atom\">\n> @@ -6111,6 +6112,7 @@ XML\n>  \t\t} else {\n>  \t\t\tprint \"<updated>$latest_date{'iso-8601'}</updated>\\n\";\n>  \t\t}\n> +\t\tprint \"<generator version='$version/$git_version'>gitweb</generator>\\n\";\n\nI'd rather use '\"' for attributes, i.e.\n\n+\t\tprint \"<generator version=\\\"$version/$git_version\\\">gitweb</generator>\\n\";\n\n\n>  \t}\n>  \n>  \t# contents\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103232","messageId":"200902050019.44016.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-4-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 3/6] gitweb: rss feed managingEditor","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-04T23:19:43Z","receivedAt":"2009-02-04T23:19:43Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> The RSS 2.0 specification allows an optional managingEditor tag for the\n> channel, containing the \"email address for person responsible for editorial\n> content\", which is basically the project owner.\n\nHmmm... does it make sense with gitweb? Perhaps it does...\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    4 +++-\n>  1 files changed, 3 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 3d94f50..cc6d0fb 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6074,7 +6074,9 @@ XML\n>  \t\tprint \"<title>$title</title>\\n\" .\n>  \t\t      \"<link>$alt_url</link>\\n\" .\n>  \t\t      \"<description>$descr</description>\\n\" .\n> -\t\t      \"<language>en</language>\\n\";\n> +\t\t      \"<language>en</language>\\n\" .\n> +\t\t      # project owner is responsible for 'editorial' content\n> +\t\t      \"<managingEditor>$owner</managingEditor>\\n\";\n\nShouldn't we esc_html($owner), just in case it is for example\n\"O Wner <owner@example.com>\" (either via gitweb.owner or via\nprojects_index being text file listing projects, because I don't\nsee GECOS having email in it...)?\n\n>  \t\tif (defined $logo || defined $favicon) {\n>  \t\t\t# prefer the logo to the favicon, since RSS\n>  \t\t\t# doesn't allow both\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103234","messageId":"200902050024.43634.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-5-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 4/6] gitweb: rss channel date","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-04T23:24:43Z","receivedAt":"2009-02-04T23:24:43Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> The RSS 2.0 specifications defines not one but _two_ dates for its\n> channel element! Woohoo! Luckily, it seems that consensus seems to be\n> that if both are present they should be equal, except for some very\n> obscure and discouraged cases. Since lastBuildDate would make more sense\n> for us and pubDate seems to be the most commonly used, we defined both\n> and make them equal.\n\nPerhaps it would make sense to quote RSS 2.0 standard format here\nin the commit message, e.g.:\n\n  pubDate\t The publication date for the content in the channel.\n  lastBuildDate\t The last time the content of the channel changed.\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    4 ++++\n>  1 files changed, 4 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index cc6d0fb..756868a 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6087,6 +6087,10 @@ XML\n>  \t\t\t      \"<link>$alt_url</link>\\n\" .\n>  \t\t\t      \"</image>\\n\";\n>  \t\t}\n> +\t\tif (%latest_date) {\n> +\t\t\tprint \"<pubDate>$latest_date{'rfc2822'}</pubDate>\\n\";\n> +\t\t\tprint \"<lastBuildDate>$latest_date{'rfc2822'}</lastBuildDate>\\n\";\n> +\t\t}\n\nI think it is good approximation of intended meaning of those two\nelements.\n\n>  \t\tprint \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n>  \t} elsif ($format eq 'atom') {\n>  \t\tprint <<XML;\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103237","messageId":"200902050038.57999.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-6-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-04T23:38:57Z","receivedAt":"2009-02-04T23:38:57Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> The last-modified time header added by RSS to increase cache hits from\n> readers should be set to the date the repository was last modified. The\n> author time in this respect is not a good guess because the last commit\n> might come from a oldish patch.\n> \n> Use the committer time for the last-modified header to ensure a more\n> correct guess of the last time the repository was modified.\n\nFirst, changing %latest_date from author time to committer time affects\nnot only Last-Modified HTTP header, but (after this series) also\nvarious \"publication dates\" in the feed contents.  But I think that for\nall those committer time is better approximation of publication date\nand last change date than author time.\n\nSecond, author time reflects when change (commit) was made, according\nto authors (perhaps skewed) clock.  Committer time reflects when given\ncommit (version of a commit) was entered into repository, or to be more\nexact into some clone of given project.  But there is also an issue of\nwhen changes got into given instance of repository (given clone): that\nI guess might be found by stat-ing HEAD (if it arrived by commit),\nFETCH_HEAD (if it arrived by fetch or pull) and ??? (if it arrived by\npush)... err... it looks like it wouldn't work in most common case,\nsorry, unless we want to stat all refs and packed-refs file. But while\nthis date might be better for Last-Modified, I'm not sure if it is good\nat all for publication date.\n\nSo committer time is better than author time, and looks like good\nmiddle ground.\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 756868a..8c49c75 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6015,7 +6015,7 @@ sub git_feed {\n>  \t}\n>  \tif (defined($commitlist[0])) {\n>  \t\t%latest_commit = %{$commitlist[0]};\n> -\t\t%latest_date   = parse_date($latest_commit{'author_epoch'});\n> +\t\t%latest_date   = parse_date($latest_commit{'committer_epoch'});\n\nNice and simple. Good.\n\n>  \t\tprint $cgi->header(\n>  \t\t\t-type => $content_type,\n>  \t\t\t-charset => 'utf-8',\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103262","messageId":"200902050303.43356.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-7-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 6/6] gitweb: check if-modified-since for feeds","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-05T02:03:42Z","receivedAt":"2009-02-05T02:03:42Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> Offering Last-modified header for feeds is only half the work, even if\n> we bail out early on HEAD requests. We should also check that same date\n> against If-modified-since, and bail out early with 304 Not Modified if\n> that's the case.\n\nIt looks now quite nice, but I'd like to see information about\ndependencies for this feature in the commit message, something like:\n\n   This feature (terminating early with '304 Not Modified' in response\n   to 'If-Modified-Since' conditional request) requires having either\n   HTTP::Date module (from libwww-perl) or Time::ParseDate module.\n   If neither is present gitweb falls back to earlier behaviour of not\n   reacting to 'If-Modified-Since'.\n\nNote also (although I'm not sure if it is worth mentioning in commit\nmessage) that it doesn't save gitweb as much work as one could think,\nbecause at this place the whole list of commits is already generated\nand parsed.  What we save is cost of running git-diff-tree for each\ncommit (we could do better here, I think), and of course bandwidth.\n\n\nI wonder if it would be possible to separate this code into subroutine,\nto make it possible to have support for \"cache control\" conditional\nrequests also in other cases where it might help (for the future of\ncourse, not in this commit).\n\nPerhaps if we run gitweb from Apache mod_perl in compatibility mode\n(ModPerl::Registry) to use Apache Perl API to respond to \n'If-Modified-Since' ($r->is_fresh or something). But that is also\njust idea for the future improvement.\n\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n\nHere I think:\n\nAcked-by: Jakub Narebski <jnareb@gmail.com>\n\n> ---\n>  gitweb/gitweb.perl |   20 +++++++++++++++++++-\n>  1 files changed, 19 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 8c49c75..f4defb0 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6015,7 +6015,25 @@ sub git_feed {\n>  \t}\n>  \tif (defined($commitlist[0])) {\n>  \t\t%latest_commit = %{$commitlist[0]};\n> -\t\t%latest_date   = parse_date($latest_commit{'committer_epoch'});\n> +\t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n> +\t\t%latest_date   = parse_date($latest_epoch);\n\nNitpick: either follow aligning on '=' characters, or skip it\naltogether and always use one space before '=' in this fragment.\n\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\nGood.\n\n>  \t\tprint $cgi->header(\n>  \t\t\t-type => $content_type,\n>  \t\t\t-charset => 'utf-8',\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103448","messageId":"cb7bb73a0902060255i6645fc25wf16755a866ab7a42@mail.gmail.com","threadId":"17368","inReplyTo":"200902042356.34407.jnareb@gmail.com","subject":"Re: [PATCHv2 1/6] gitweb: channel image in rss feed","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T10:55:19Z","receivedAt":"2009-02-06T10:55:19Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 4, 2009 at 11:56 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> +             if (defined $logo || defined $favicon) {\n>> +                     # prefer the logo to the favicon, since RSS\n>> +                     # doesn't allow both\n>> +                     my $img = esc_url($logo || $favicon);\n>> +                     print \"<image>\\n\" .\n>> +                           \"<url>$img</url>\\n\" .\n>> +                           \"<title>$title</title>\\n\" .\n>> +                           \"<link>$alt_url</link>\\n\" .\n>> +                           \"</image>\\n\";\n>> +             }\n>\n> A question: how should URL, which is _contents_ of a tag (instead of\n> as _attribute_ value), quoted (escaped)? I don't think it is specified\n> in RSS. I guess that using esc_url() is a safe solution.\n\nIndeed, it might not be needed, but it's doesn't do any damage to\nescape the URL.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103449","messageId":"cb7bb73a0902060301s2a2f81e2t1762377177fb550e@mail.gmail.com","threadId":"17368","inReplyTo":"200902050015.20170.jnareb@gmail.com","subject":"Re: [PATCHv2 2/6] gitweb: feed generator metadata","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T11:01:31Z","receivedAt":"2009-02-06T11:01:31Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 5, 2009 at 12:15 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> Add <generator> tag to RSS and Atom feed. Versioning info (gitweb/git\n>> core versions, separated by a literal slash) is stored in the\n>> appropriate attribute for the Atom feed, and in the tag content for the\n>> RSS feed.\n>\n> Very good idea. I haven't examined either specification, so I don't\n> know what conventions are used, though... and what conventions _should_\n> be used.\n>\n> By the way, gitweb uses in HTML header the following (see\n> git_header_html subroutine):\n>\n>  <meta name=\"generator\" content=\"gitweb/$version git/$git_version$mod_perl_version\"/>\n>\n> which tries to follow convention how _web servers_ like Apache return\n> version information in the 'Server:' HTTP response header (product\n> tokens). Because it was used on only one place, it was not put into\n> separate subroutine; should it now?\n\nRSS 2.0 spec for generator @\nhttp://cyber.law.harvard.edu/rss/rss.html#optionalChannelElements\nseems to suggest that the content for the tag in RSS feeds is pretty\nmuch free-form and we might use the same string we have in HTML pages.\nRequirements for Atom (see\nhttp://www.atomenabled.org/developers/syndication/atom-format-spec.php#element.generator\n) are rather more stringent, so it needs its own code anyway.\n\n>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n>> ---\n>>  gitweb/gitweb.perl |    2 ++\n>>  1 files changed, 2 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index f8a5d2e..3d94f50 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -6085,6 +6085,7 @@ XML\n>>                             \"<link>$alt_url</link>\\n\" .\n>>                             \"</image>\\n\";\n>>               }\n>> +             print \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n>>       } elsif ($format eq 'atom') {\n>>               print <<XML;\n>>  <feed xmlns=\"http://www.w3.org/2005/Atom\">\n>> @@ -6111,6 +6112,7 @@ XML\n>>               } else {\n>>                       print \"<updated>$latest_date{'iso-8601'}</updated>\\n\";\n>>               }\n>> +             print \"<generator version='$version/$git_version'>gitweb</generator>\\n\";\n>\n> I'd rather use '\"' for attributes, i.e.\n>\n> +               print \"<generator version=\\\"$version/$git_version\\\">gitweb</generator>\\n\";\n\nI can do that.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103451","messageId":"cb7bb73a0902060303s35527b3ey8572d60c54066e33@mail.gmail.com","threadId":"17368","inReplyTo":"200902050019.44016.jnareb@gmail.com","subject":"Re: [PATCHv2 3/6] gitweb: rss feed managingEditor","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T11:03:26Z","receivedAt":"2009-02-06T11:03:26Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 5, 2009 at 12:19 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> The RSS 2.0 specification allows an optional managingEditor tag for the\n>> channel, containing the \"email address for person responsible for editorial\n>> content\", which is basically the project owner.\n>\n> Hmmm... does it make sense with gitweb? Perhaps it does...\n\nI must confess I had my own doubts about this 8-D\n\n>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n>> ---\n>>  gitweb/gitweb.perl |    4 +++-\n>>  1 files changed, 3 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 3d94f50..cc6d0fb 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -6074,7 +6074,9 @@ XML\n>>               print \"<title>$title</title>\\n\" .\n>>                     \"<link>$alt_url</link>\\n\" .\n>>                     \"<description>$descr</description>\\n\" .\n>> -                   \"<language>en</language>\\n\";\n>> +                   \"<language>en</language>\\n\" .\n>> +                   # project owner is responsible for 'editorial' content\n>> +                   \"<managingEditor>$owner</managingEditor>\\n\";\n>\n> Shouldn't we esc_html($owner), just in case it is for example\n> \"O Wner <owner@example.com>\" (either via gitweb.owner or via\n> projects_index being text file listing projects, because I don't\n> see GECOS having email in it...)?\n>\n\nIndeed, itt's probably safer to do the escaping.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103453","messageId":"cb7bb73a0902060310s3fa19788t7f4ad0d39928d77@mail.gmail.com","threadId":"17368","inReplyTo":"200902050024.43634.jnareb@gmail.com","subject":"Re: [PATCHv2 4/6] gitweb: rss channel date","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T11:10:36Z","receivedAt":"2009-02-06T11:10:36Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 5, 2009 at 12:24 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> The RSS 2.0 specifications defines not one but _two_ dates for its\n>> channel element! Woohoo! Luckily, it seems that consensus seems to be\n>> that if both are present they should be equal, except for some very\n>> obscure and discouraged cases. Since lastBuildDate would make more sense\n>> for us and pubDate seems to be the most commonly used, we defined both\n>> and make them equal.\n>\n> Perhaps it would make sense to quote RSS 2.0 standard format here\n> in the commit message, e.g.:\n>\n>  pubDate        The publication date for the content in the channel.\n>  lastBuildDate  The last time the content of the channel changed.\n\nThat sort of spoils the very non-technical tone of the message, but\nyou're probably right 8-D\n\n>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n>> ---\n>>  gitweb/gitweb.perl |    4 ++++\n>>  1 files changed, 4 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index cc6d0fb..756868a 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -6087,6 +6087,10 @@ XML\n>>                             \"<link>$alt_url</link>\\n\" .\n>>                             \"</image>\\n\";\n>>               }\n>> +             if (%latest_date) {\n>> +                     print \"<pubDate>$latest_date{'rfc2822'}</pubDate>\\n\";\n>> +                     print \"<lastBuildDate>$latest_date{'rfc2822'}</lastBuildDate>\\n\";\n>> +             }\n>\n> I think it is good approximation of intended meaning of those two\n> elements.\n\nHowever, this is still not perfect. While this is absolutely fine for\nrss feeds that point at an explicit commit hash, rss feeds that point\nto  a dynamic ref (some branch head or whatever) can get an\ninteresting but confusing situation:\n\nA updates his local clone of the repo.\n\nB gets the feed of the repo, which is not updated to include A changes.\n\nA pushes.\n\nB gets the feed: the dates are actually BEFORE the date he last\nretrieved the feed, although they are after the date shown the last\ntime he retrieved the feed.\n\nIdeally, when the rss is publishing a dynamic ref, we should as\nbuildDate (and as Last-modified: HTTP header!) the date when the ref\nwas changed (e.g. the date of the push). However, I couldn't really\nthink of a robust way of getting such a date, which is why I'm using\nthe commit date which is what comes closest.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103454","messageId":"cb7bb73a0902060314u4741528bs196a43d45ff7f283@mail.gmail.com","threadId":"17368","inReplyTo":"200902050038.57999.jnareb@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T11:14:20Z","receivedAt":"2009-02-06T11:14:20Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 5, 2009 at 12:38 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> The last-modified time header added by RSS to increase cache hits from\n>> readers should be set to the date the repository was last modified. The\n>> author time in this respect is not a good guess because the last commit\n>> might come from a oldish patch.\n>>\n>> Use the committer time for the last-modified header to ensure a more\n>> correct guess of the last time the repository was modified.\n>\n> First, changing %latest_date from author time to committer time affects\n> not only Last-Modified HTTP header, but (after this series) also\n> various \"publication dates\" in the feed contents.  But I think that for\n> all those committer time is better approximation of publication date\n> and last change date than author time.\n>\n> Second, author time reflects when change (commit) was made, according\n> to authors (perhaps skewed) clock.  Committer time reflects when given\n> commit (version of a commit) was entered into repository, or to be more\n> exact into some clone of given project.  But there is also an issue of\n> when changes got into given instance of repository (given clone): that\n> I guess might be found by stat-ing HEAD (if it arrived by commit),\n> FETCH_HEAD (if it arrived by fetch or pull) and ??? (if it arrived by\n> push)... err... it looks like it wouldn't work in most common case,\n> sorry, unless we want to stat all refs and packed-refs file. But while\n> this date might be better for Last-Modified, I'm not sure if it is good\n> at all for publication date.\n>\n> So committer time is better than author time, and looks like good\n> middle ground.\n\nOops should have finished reading your comments before my previous\nreply. The solution would be to introduce a way to determine robustly\nwhen a branch was last _physically_ updated. Checking the\ncorresponding entry in refs/ would work for non-packed refs, and maybe\none would hope that if the ref got packed, it means it hasn't updated\nin a long time ... but I'm not enough of an expert on git's internal\nto really know about this. Suggestions?\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103455","messageId":"cb7bb73a0902060319q474f3daeu5f67ea859cfa97b1@mail.gmail.com","threadId":"17368","inReplyTo":"200902050303.43356.jnareb@gmail.com","subject":"Re: [PATCHv2 6/6] gitweb: check if-modified-since for feeds","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T11:19:29Z","receivedAt":"2009-02-06T11:19:29Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 5, 2009 at 3:03 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> Offering Last-modified header for feeds is only half the work, even if\n>> we bail out early on HEAD requests. We should also check that same date\n>> against If-modified-since, and bail out early with 304 Not Modified if\n>> that's the case.\n>\n> It looks now quite nice, but I'd like to see information about\n> dependencies for this feature in the commit message, something like:\n>\n>   This feature (terminating early with '304 Not Modified' in response\n>   to 'If-Modified-Since' conditional request) requires having either\n>   HTTP::Date module (from libwww-perl) or Time::ParseDate module.\n>   If neither is present gitweb falls back to earlier behaviour of not\n>   reacting to 'If-Modified-Since'.\n\nGood idea.\n\n> Note also (although I'm not sure if it is worth mentioning in commit\n> message) that it doesn't save gitweb as much work as one could think,\n> because at this place the whole list of commits is already generated\n> and parsed.  What we save is cost of running git-diff-tree for each\n> commit (we could do better here, I think), and of course bandwidth.\n\nIf it's possible to get the first commit without generating and\nparsing the entire commit list, this can probably be optimized\nfurther.\n\n> I wonder if it would be possible to separate this code into subroutine,\n> to make it possible to have support for \"cache control\" conditional\n> requests also in other cases where it might help (for the future of\n> course, not in this commit).\n\nYes, I thought about it too.\n\n>\n> Perhaps if we run gitweb from Apache mod_perl in compatibility mode\n> (ModPerl::Registry) to use Apache Perl API to respond to\n> 'If-Modified-Since' ($r->is_fresh or something). But that is also\n> just idea for the future improvement.\n\nAlso, it's way beyond my current knowledge of Apache, CGI and Perl 8-D\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103457","messageId":"200902061221.39607.jnareb@gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902060301s2a2f81e2t1762377177fb550e@mail.gmail.com","subject":"Re: [PATCHv2 2/6] gitweb: feed generator metadata","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-06T11:21:38Z","receivedAt":"2009-02-06T11:21:38Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 6 Feb 2009, Giuseppe Bilotta wrote:\n> On Thu, Feb 5, 2009 at 12:15 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>>\n>>> Add <generator> tag to RSS and Atom feed. Versioning info (gitweb/git\n>>> core versions, separated by a literal slash) is stored in the\n>>> appropriate attribute for the Atom feed, and in the tag content for the\n>>> RSS feed.\n>>\n>> Very good idea. I haven't examined either specification, so I don't\n>> know what conventions are used, though... and what conventions _should_\n>> be used.\n>>\n>> By the way, gitweb uses in HTML header the following (see\n>> git_header_html subroutine):\n>>\n>>  <meta name=\"generator\" content=\"gitweb/$version git/$git_version$mod_perl_version\"/>\n>>\n>> which tries to follow convention how _web servers_ like Apache return\n>> version information in the 'Server:' HTTP response header (product\n>> tokens). Because it was used on only one place, it was not put into\n>> separate subroutine; should it now?\n> \n> RSS 2.0 spec for generator @\n> http://cyber.law.harvard.edu/rss/rss.html#optionalChannelElements\n> seems to suggest that the content for the tag in RSS feeds is pretty\n> much free-form and we might use the same string we have in HTML pages.\n> Requirements for Atom (see\n> http://www.atomenabled.org/developers/syndication/atom-format-spec.php#element.generator\n> ) are rather more stringent, so it needs its own code anyway.\n\nI don't see there in given above Atom spec _how_ 'version' attribute\nshould be formatted. Here is relevant excerpt from mentioned page:\n\n  4.2.4 The \"atom:generator\" Element\n\n  The \"atom:generator\" element's content identifies the agent used\n  to generate a feed, for debugging and other purposes.\n  [...]\n\n  The content of this element, when present, MUST be a string that\n  is a human-readable name for the generating agent.\n  [...]\n\n  The atom:generator element MAY have a \"version\" attribute that\n  indicates the version of the generating agent.\n\nSo why not use something like:\n\n  <generator version=\"gitweb/$version git/$git_version$mod_perl_version\">\n  gitweb v$version</generator>\n \nfor Atom? Perhaps with 'Server:'-like version generation refactored\nto its own subroutine?\n\n>>> +             print \"<generator>gitweb v.$version/$git_version</generator>\\n\";\n>>>       } elsif ($format eq 'atom') {\n\n>>> +             print \"<generator version='$version/$git_version'>gitweb</generator>\\n\";\n\n-- \nJakub Narebski\nPoland\n"},{"id":"103555","messageId":"200902062212.08857.jnareb@gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902060314u4741528bs196a43d45ff7f283@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-06T21:12:05Z","receivedAt":"2009-02-06T21:12:05Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 06 Feb 2009, Giuseppe Bilotta wrote:\n> On Thu, Feb 5, 2009 at 12:38 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>>\n>>> The last-modified time header added by RSS to increase cache hits from\n>>> readers should be set to the date the repository was last modified. The\n>>> author time in this respect is not a good guess because the last commit\n>>> might come from a oldish patch.\n>>>\n>>> Use the committer time for the last-modified header to ensure a more\n>>> correct guess of the last time the repository was modified.\n\n[...]\n>> Second, author time reflects when change (commit) was made, according\n>> to authors (perhaps skewed) clock.  Committer time reflects when given\n>> commit (version of a commit) was entered into repository, or to be more\n>> exact into some clone of given project.  But there is also an issue of\n>> when changes got into given instance of repository (given clone): that\n>> I guess might be found by stat-ing HEAD (if it arrived by commit),\n>> FETCH_HEAD (if it arrived by fetch or pull) and ??? (if it arrived by\n>> push)... err... it looks like it wouldn't work in most common case,\n>> sorry, unless we want to stat all refs and packed-refs file. But while\n>> this date might be better for Last-Modified, I'm not sure if it is good\n>> at all for publication date.\n>>\n>> So committer time is better than author time, and looks like good\n>> middle ground.\n> \n> Oops should have finished reading your comments before my previous\n> reply. The solution would be to introduce a way to determine robustly\n> when a branch was last _physically_ updated. Checking the\n> corresponding entry in refs/ would work for non-packed refs, and maybe\n> one would hope that if the ref got packed, it means it hasn't updated\n> in a long time ... but I'm not enough of an expert on git's internal\n> to really know about this. Suggestions?\n\nThe point is I am not that sure if the date branch was _physically_\nupdated (it was updated in given clone of repository) is a good\nchoice for publish date in feed (RSS or Atom), whether it is good\nchoice for Last-Modified (and If-Modified-Since), and whether publish\ndate can be different from Last-Modified.  \n\nUsing committime doesn't ensure that it is monotonical (but it is\nhighly probable, much more than authortime), but it is the same for\nevery clone of repository, and for every gitweb installation that\nhosts given repository.\n\nUsing update time does ensure that it is monotonical, and it\nwouldn't ever be earlier than the time repository started to be\navailable, but it changes from clone to clone.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"103566","messageId":"200902062342.57994.jnareb@gmail.com","threadId":"17368","inReplyTo":"1232970616-21167-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-06T22:42:56Z","receivedAt":"2009-02-06T22:42:56Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n\n> This second revision adds two patches to improve client-side rss\n> caching: the last-modified header we output is based on commit rather\n> than creation time, and we now act on if-modified-since request headers.\n> \n> The last patch requires either HTTP::Date (from libwww-perl) or\n> Time::ParseDate\n\n\nBelow there is summary of my comments.\n \n> Giuseppe Bilotta (6):\n>   gitweb: channel image in rss feed\n\nO.K. esc_url is perhaps unnecessary, but safe\n\n>   gitweb: feed generator metadata\n\nBetter to use the same convention for version info as in meta.generator\nin HTML header.  Use '\"' for attribute values.\n\n>   gitweb: rss feed managingEditor\n\nNeed to esc_html (well, esc_xml ;-) $owner, because it might be set to\n\"O Wner <owner@example.com>\".\n\n>   gitweb: rss channel date\n\nO.K. (with question if it would be better to quote relevant fragment\nof RSS 2.0 standard with respect to meaning of those date elements).\n\n>   gitweb: last-modified time should be commiter, not author\n\nO.K. perhaps with clarification that it affects not only Last-Modified,\nbut also just introduced (and used earlier) \"publish\" dates for feed.\n\n>   gitweb: check if-modified-since for feeds\n\nO.K., with proposed addition to commit message.\n\n> \n>  gitweb/gitweb.perl |   40 ++++++++++++++++++++++++++++++++++++++--\n>  1 files changed, 38 insertions(+), 2 deletions(-)\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"103568","messageId":"cb7bb73a0902061500x12105a13la78703918937fe4b@mail.gmail.com","threadId":"17368","inReplyTo":"200902062212.08857.jnareb@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-06T23:00:11Z","receivedAt":"2009-02-06T23:00:11Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Fri, Feb 6, 2009 at 10:12 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> Using update time does ensure that it is monotonical, and it\n> wouldn't ever be earlier than the time repository started to be\n> available, but it changes from clone to clone.\n\nI don't think it changing from clone to clone is a problem. Monotony\nand the ability to properly catch when the underlying data really\nchanged are much more important factors. After all, there is no\n_requirement_ that Last-Modified be used for If-modified-since headers\n(although it is recommented, and clients that don't follow the\nrecommendation are warned to be aware of all possible odd problems\narising from not using it).\n\nThe real question is: _how_ do you get the (branch) update time?\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"103572","messageId":"7vab8z14d7.fsf@gitster.siamese.dyndns.org","threadId":"17368","inReplyTo":"200902062342.57994.jnareb@gmail.com","subject":"Re: [PATCHv2 0/6] gitweb: feed metadata enhancements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T00:24:04Z","receivedAt":"2009-02-07T00:24:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:\n>\n>> This second revision adds two patches to improve client-side rss\n>> caching: the last-modified header we output is based on commit rather\n>> than creation time, and we now act on if-modified-since request headers.\n>> \n>> The last patch requires either HTTP::Date (from libwww-perl) or\n>> Time::ParseDate\n>\n>\n> Below there is summary of my comments.\n\nThanks.\n"},{"id":"104137","messageId":"86d4c5e00902101910k5512efel2c9165f75d61ba53@mail.gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902061500x12105a13la78703918937fe4b@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Deskin Miller","fromEmail":"deskinm@gmail.com","sentAt":"2009-02-11T03:10:02Z","receivedAt":"2009-02-11T03:10:02Z","isPatch":false,"sender":{"key":"deskinm@gmail.com","avatar":null},"body":"On Fri, Feb 6, 2009 at 18:00, Giuseppe Bilotta\n<giuseppe.bilotta@gmail.com> wrote:\n>\n> The real question is: _how_ do you get the (branch) update time?\n>\n\nSorry the topic's cold, but...\n\ngit reflog?\n\nSeems like one could find the oldest time the commit appears in the\nreflog, for the branch one is interested in.  You can use the commit\ntime to limit the search through the reflog, but there would be clock\nskew concerns.\n\nDeskin Miller\n"},{"id":"104152","messageId":"cb7bb73a0902110102k5b7e5a21vd7d0932c6b6ba20e@mail.gmail.com","threadId":"17368","inReplyTo":"86d4c5e00902101910k5512efel2c9165f75d61ba53@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-11T09:02:00Z","receivedAt":"2009-02-11T09:02:00Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 11, 2009 at 4:10 AM, Deskin Miller <deskinm@gmail.com> wrote:\n> On Fri, Feb 6, 2009 at 18:00, Giuseppe Bilotta\n> <giuseppe.bilotta@gmail.com> wrote:\n>>\n>> The real question is: _how_ do you get the (branch) update time?\n>>\n>\n> Sorry the topic's cold, but...\n>\n> git reflog?\n>\n> Seems like one could find the oldest time the commit appears in the\n> reflog, for the branch one is interested in.  You can use the commit\n> time to limit the search through the reflog, but there would be clock\n> skew concerns.\n\nBingo! Thanks a lot\n\noblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n--since=\"two days ago\" master | cat\n7324b32... master@{0}: push\ne2dc08d... master@{1}: push\noblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n--since=\"yesterday\" master | cat\noblomov@rbot ~ $\n\nI'll try to work it in the next review for this patchset.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104155","messageId":"200902111018.39699.jnareb@gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902110102k5b7e5a21vd7d0932c6b6ba20e@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-11T09:18:38Z","receivedAt":"2009-02-11T09:18:38Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 11 Feb 2009, Giuseppe Bilotta wrote:\n> On Wed, Feb 11, 2009 at 4:10 AM, Deskin Miller <deskinm@gmail.com> wrote:\n>> On Fri, Feb 6, 2009 at 18:00, Giuseppe Bilotta\n>> <giuseppe.bilotta@gmail.com> wrote:\n>>>\n>>> The real question is: _how_ do you get the (branch) update time?\n>>>\n>>\n>> Sorry the topic's cold, but...\n>>\n>> git reflog?\n>>\n>> Seems like one could find the oldest time the commit appears in the\n>> reflog, for the branch one is interested in.  You can use the commit\n>> time to limit the search through the reflog, but there would be clock\n>> skew concerns.\n> \n> Bingo! Thanks a lot\n> \n> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n> --since=\"two days ago\" master | cat\n> 7324b32... master@{0}: push\n> e2dc08d... master@{1}: push\n> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n> --since=\"yesterday\" master | cat\n> oblomov@rbot ~ $\n> \n> I'll try to work it in the next review for this patchset.\n\nAssuming that you have reflog enabled (yes, it is default now)...\nSo you would have to provide fallback in the case there is no reflog.\n\nBTW. \"git reflog\" is porcelain; it would be better to parse reflog\ndirectly, I think.\n-- \nJakub Narebski\nPoland\n"},{"id":"104162","messageId":"cb7bb73a0902110154h325ed146h4d1161d4a8dcb1ca@mail.gmail.com","threadId":"17368","inReplyTo":"200902111018.39699.jnareb@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-11T09:54:46Z","receivedAt":"2009-02-11T09:54:46Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 11, 2009 at 10:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Wed, 11 Feb 2009, Giuseppe Bilotta wrote:\n>> On Wed, Feb 11, 2009 at 4:10 AM, Deskin Miller <deskinm@gmail.com> wrote:\n>>> git reflog?\n>>>\n>>> Seems like one could find the oldest time the commit appears in the\n>>> reflog, for the branch one is interested in.  You can use the commit\n>>> time to limit the search through the reflog, but there would be clock\n>>> skew concerns.\n>>\n>> Bingo! Thanks a lot\n>>\n>> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n>> --since=\"two days ago\" master | cat\n>> 7324b32... master@{0}: push\n>> e2dc08d... master@{1}: push\n>> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n>> --since=\"yesterday\" master | cat\n>> oblomov@rbot ~ $\n>>\n>> I'll try to work it in the next review for this patchset.\n>\n> Assuming that you have reflog enabled (yes, it is default now)...\n> So you would have to provide fallback in the case there is no reflog.\n>\n> BTW. \"git reflog\" is porcelain; it would be better to parse reflog\n> directly, I think.\n\nDoes disabling reflog remove old reflogs? IOW, can I check if reflog\nis enabled just by opening the reflog file and assuming reflog isn't\nenabled if it's not there? Falling back to the commit date would still\nwork decently.\n\nSince we're only interested in the last reflog date, what we can do is\nto read the last line and get the unix time which is held two places\nbefore the tab separating the metadata from the log message. Correct?\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104342","messageId":"86d4c5e00902112050m5ce6ec09t8fafd43c19880622@mail.gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902110154h325ed146h4d1161d4a8dcb1ca@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Deskin Miller","fromEmail":"deskinm@gmail.com","sentAt":"2009-02-12T04:50:56Z","receivedAt":"2009-02-12T04:50:56Z","isPatch":false,"sender":{"key":"deskinm@gmail.com","avatar":null},"body":"On Wed, Feb 11, 2009 at 04:54, Giuseppe Bilotta\n<giuseppe.bilotta@gmail.com> wrote:\n> On Wed, Feb 11, 2009 at 10:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> Assuming that you have reflog enabled (yes, it is default now)...\n>> So you would have to provide fallback in the case there is no reflog.\n>>\n>> BTW. \"git reflog\" is porcelain; it would be better to parse reflog\n>> directly, I think.\n\nAgreed on a fallback being necessary; as for parsing the log directly,\nthat seems fine, but I might be inclined to use git log -g --date=foo\nand then parse that.  But that's still using porcelain at some level.\n\nIn my copious free time I'm working on a patch to expose reflog\ninformation through git log -g using new --pretty=format: specifiers,\nbut even my initial attempts to carry reflog information through to\nthe point in the code where it could be output broke some testcases;\nit'll be a long while before I grok what's going on enough to make it\nwork.  But I digress.\n\n> Does disabling reflog remove old reflogs? IOW, can I check if reflog\n> is enabled just by opening the reflog file and assuming reflog isn't\n> enabled if it's not there? Falling back to the commit date would still\n> work decently.\n\nThis seems easy enough to test.  If switching core.logAllRefUpdates\nfrom true -> false doesn't remove the log, though (and why would\ngit-config delete reflog files?), you could have an old log which just\nhasn't been updated for some time, where the branch head might not\nappear at all.  But that's just idle speculation without testing the\nactual behaviour.\n\n> Since we're only interested in the last reflog date, what we can do is\n> to read the last line and get the unix time which is held two places\n> before the tab separating the metadata from the log message. Correct?\n\nLooking at the log format I'd say that seems fine.\n\nGlad to see I was thinking clearly, suggesting reflog :)\n\nDeskin Miller\n"},{"id":"104355","messageId":"200902121007.29867.jnareb@gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902110154h325ed146h4d1161d4a8dcb1ca@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-12T09:07:28Z","receivedAt":"2009-02-12T09:07:28Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 11 Feb 2009, Giuseppe Bilotta wrote:\n> On Wed, Feb 11, 2009 at 10:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Wed, 11 Feb 2009, Giuseppe Bilotta wrote:\n>>> On Wed, Feb 11, 2009 at 4:10 AM, Deskin Miller <deskinm@gmail.com> wrote:\n>>>>\n>>>> git reflog?\n>>>>\n>>>> Seems like one could find the oldest time the commit appears in the\n>>>> reflog, for the branch one is interested in.  You can use the commit\n>>>> time to limit the search through the reflog, but there would be clock\n>>>> skew concerns.\n>>>\n>>> Bingo! Thanks a lot\n>>>\n>>> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n>>> --since=\"two days ago\" master | cat\n>>> 7324b32... master@{0}: push\n>>> e2dc08d... master@{1}: push\n>>> oblomov@rbot ~ $ GIT_DIR=/var/git/rbot.git/ git reflog show\n>>> --since=\"yesterday\" master | cat\n>>> oblomov@rbot ~ $\n>>>\n>>> I'll try to work it in the next review for this patchset.\n>>\n>> Assuming that you have reflog enabled (yes, it is default now)...\n>> So you would have to provide fallback in the case there is no reflog.\n>>\n>> BTW. \"git reflog\" is porcelain; it would be better to parse reflog\n>> directly, I think.\n> \n> Does disabling reflog remove old reflogs? IOW, can I check if reflog\n> is enabled just by opening the reflog file and assuming reflog isn't\n> enabled if it's not there? Falling back to the commit date would still\n> work decently.\n\nYou can disable reflog by setting core.logAllRefUpdates to false...\nwhich of course do not remove reflog.  But checking for this config\nvariable in gitweb shouldn't be too hard, although you would need\nto somehow change assumption that we are interested only in ^gitweb\\.\nsection of config.\n\nThere is other side of this issue: reflog is expired, so you can\nhave empty reflog if branch was updated long time ago.\n\n>\n> Since we're only interested in the last reflog date, what we can do is\n> to read the last line and get the unix time which is held two places\n> before the tab separating the metadata from the log message. Correct?\n\nAlmost correct.  Some tools (old StGit for example) didn't provide\nreflog message, and with empty message they forgot to append TAB\nseparator.  So you would have to provide for legacy\n\n  <old sha-1> <new-sha-1> <committer> <timestamp> <timezone>\n\ninstead of correct (for empty reflog message)\n\n  <old sha-1> <new-sha-1> <committer> <timestamp> <timezone> TAB\n\n\nP.S. It is a pity that due to packed refs stat-ing branch tip file\n$GIT_DIR/refs/heads/<branch> is not enough...\n-- \nJakub Narebski\nPoland\n"},{"id":"104360","messageId":"cb7bb73a0902120152h7b3e0ac6w379bd9319712ea2b@mail.gmail.com","threadId":"17368","inReplyTo":"200902121007.29867.jnareb@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-12T09:52:06Z","receivedAt":"2009-02-12T09:52:06Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 12, 2009 at 10:07 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Wed, 11 Feb 2009, Giuseppe Bilotta wrote:\n>> On Wed, Feb 11, 2009 at 10:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n\n>>> Assuming that you have reflog enabled (yes, it is default now)...\n>>> So you would have to provide fallback in the case there is no reflog.\n>>>\n>>> BTW. \"git reflog\" is porcelain; it would be better to parse reflog\n>>> directly, I think.\n>>\n>> Does disabling reflog remove old reflogs? IOW, can I check if reflog\n>> is enabled just by opening the reflog file and assuming reflog isn't\n>> enabled if it's not there? Falling back to the commit date would still\n>> work decently.\n>\n> You can disable reflog by setting core.logAllRefUpdates to false...\n> which of course do not remove reflog.  But checking for this config\n> variable in gitweb shouldn't be too hard, although you would need\n> to somehow change assumption that we are interested only in ^gitweb\\.\n> section of config.\n>\n> There is other side of this issue: reflog is expired, so you can\n> have empty reflog if branch was updated long time ago.\n\nI'm thinking that what I could do (that should always work as\nexpected) is to consider as 'last modified' the most recent date\nbetween the commit date and the reflog date (if the reflog is found).\nThis basically implements an 'automatic' fallback for\ndisabled/obsolete/expired reflog to commit date. (Of course, assuming\nthe last commit doesn't suffer from a ridicously long clockskew in the\nfuture.)\n\n>> Since we're only interested in the last reflog date, what we can do is\n>> to read the last line and get the unix time which is held two places\n>> before the tab separating the metadata from the log message. Correct?\n>\n> Almost correct.  Some tools (old StGit for example) didn't provide\n> reflog message, and with empty message they forgot to append TAB\n> separator.  So you would have to provide for legacy\n>\n>  <old sha-1> <new-sha-1> <committer> <timestamp> <timezone>\n>\n> instead of correct (for empty reflog message)\n>\n>  <old sha-1> <new-sha-1> <committer> <timestamp> <timezone> TAB\n\nThat's the easy part: split at TAB, get the first array entry, split\nat spaces, get the last two. But thanks for making me aware of the\npossibly missing TAB\n\n> P.S. It is a pity that due to packed refs stat-ing branch tip file\n> $GIT_DIR/refs/heads/<branch> is not enough...\n\nYeah, that's what the other rbot developer suggested initially (then I\nfound out that cloning the repo was enough to NOT have branch heads).\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104362","messageId":"200902121111.51147.jnareb@gmail.com","threadId":"17368","inReplyTo":"cb7bb73a0902120152h7b3e0ac6w379bd9319712ea2b@mail.gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-12T10:11:44Z","receivedAt":"2009-02-12T10:11:44Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 12 Feb 2009, Giuseppe Bilotta wrote:\n> On Thu, Feb 12, 2009 at 10:07 AM, Jakub Narebski <jnareb@gmail.com> \n> wrote: \n\n>> You can disable reflog by setting core.logAllRefUpdates to false...\n>> which of course do not remove reflog.  But checking for this config\n>> variable in gitweb shouldn't be too hard, although you would need\n>> to somehow change assumption that we are interested only in ^gitweb\\.\n>> section of config.\n>>\n>> There is other side of this issue: reflog is expired, so you can\n>> have empty reflog if branch was updated long time ago.\n> \n> I'm thinking that what I could do (that should always work as\n> expected) is to consider as 'last modified' the most recent date\n> between the commit date and the reflog date (if the reflog is found).\n> This basically implements an 'automatic' fallback for\n> disabled/obsolete/expired reflog to commit date. (Of course, assuming\n> the last commit doesn't suffer from a ridicously long clockskew in the\n> future.)\n\nThat is, I think, a very good idea. It also covers situation where we\nuse non-head 'h'/'hb', for example explicit SHA-1 (not that it makes\nsense, but...) \n \nWell, that beside my little doubt whether using publish date is a good\nidea or not...\n\n\nP.S. What would you do for explicit and implicit HEAD? HEAD reflog,\nor 'current branch' reflog?\n-- \nJakub Narebski\nPoland\n"},{"id":"104372","messageId":"cb7bb73a0902120323ja05d79bw7470521d967eef03@mail.gmail.com","threadId":"17368","inReplyTo":"200902121111.51147.jnareb@gmail.com","subject":"Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-12T11:23:47Z","receivedAt":"2009-02-12T11:23:47Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 12, 2009 at 11:11 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>\n> Well, that beside my little doubt whether using publish date is a good\n> idea or not...\n>\n\nI'm open to suggestion for better ideas 8-)\n\n> P.S. What would you do for explicit and implicit HEAD? HEAD reflog,\n> or 'current branch' reflog?\n\nI would say the HEAD reflog.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"}]}