{"thread":{"id":"5603","subject":"[PATCH 0/2] Make git_get_refs_list do work of git_get_references","startedAt":"2006-09-19T12:30:51Z","lastAt":"2006-09-21T09:07:35Z","messageCount":9,"participants":["Jakub Narebski","Junio C Hamano","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"27157","messageId":"200609191430.51252.jnareb@gmail.com","threadId":"5603","inReplyTo":null,"subject":"[PATCH 0/2] Make git_get_refs_list do work of git_get_references","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-19T12:30:51Z","receivedAt":"2006-09-19T12:30:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is resend of short series of patches. Second patch in series\nwas corrected twice ([PATCH 2/2 (take 2)] and [PATCH 2/2 (take 3)]),\nbut neither of corrections made to git mailing list.\n\nShortlog:\n [PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references\n [PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references\n\nDiffstat:\n gitweb/gitweb.perl |   76 ++++++++++++++++++++++++++++++----------------------\n 1 files changed, 44 insertions(+), 32 deletions(-)\n-- \nJakub Narebski\nPoland\n"},{"id":"27158","messageId":"200609191431.49641.jnareb@gmail.com","threadId":"5603","inReplyTo":"200609191430.51252.jnareb@gmail.com","subject":"[PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-19T12:31:49Z","receivedAt":"2006-09-19T12:31:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Instead of trying to read info/refs file, which might not be present\n(we did fallback to git-ls-remote), always use git-peek-remote in\ngit_get_references.\n\nIt is preparation for git_get_refs_info to also return references\ninfo. We cannot use info/refs for git_get_refs_info as the information\ncontained therein is usually stale.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |   10 ++--------\n 1 files changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c77270c..034a88c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -828,16 +828,10 @@ sub git_get_project_owner {\n sub git_get_references {\n \tmy $type = shift || \"\";\n \tmy %refs;\n-\tmy $fd;\n \t# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c\trefs/tags/v2.6.11\n \t# c39ae07f393806ccf406ef966e9a15afc43cc36a\trefs/tags/v2.6.11^{}\n-\tif (-f \"$projectroot/$project/info/refs\") {\n-\t\topen $fd, \"$projectroot/$project/info/refs\"\n-\t\t\tor return;\n-\t} else {\n-\t\topen $fd, \"-|\", git_cmd(), \"ls-remote\", \".\"\n-\t\t\tor return;\n-\t}\n+\topen my $fd, \"-|\", $GIT, \"peek-remote\", \"$projectroot/$project/\"\n+\t\tor return;\n \n \twhile (my $line = <$fd>) {\n \t\tchomp $line;\n-- \n1.4.2.1\n"},{"id":"27159","messageId":"200609191433.22878.jnareb@gmail.com","threadId":"5603","inReplyTo":"200609191430.51252.jnareb@gmail.com","subject":"[PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-19T12:33:22Z","receivedAt":"2006-09-19T12:33:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Make git_get_refs_list do also work of git_get_references, to avoid\ncalling git-peek-remote twice.  Change meaning of git_get_refs_list\nmeaning: it is now type, and not a full path, e.g. we now use\ngit_get_refs_list(\"heads\") instead of former\ngit_get_refs_list(\"refs/heads\").\n\nModify git_summary to use only one call to git_get_refs_list instead\nof one call to git_get_references and two to git_get_refs_list.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nBenchmarks show that this version is slightly faster than\n\"before\" version:\n\n  2626.133 +/- 98.1 ms after vs 3086.579 +/- 85.0 ms before (summary)\n  2233.814 +/- 13.9 ms after vs 2504.025 +/- 46.1 ms before (tags)\n   869.533 +/-  7.7 ms after vs  979.281 +/- 88.1 ms before (heads)\n\n\n gitweb/gitweb.perl |   66 +++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 42 insertions(+), 24 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 034a88c..01fae94 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1119,7 +1119,8 @@ ## .....................................\n ## parse to array of hashes functions\n \n sub git_get_refs_list {\n-\tmy $ref_dir = shift;\n+\tmy $type = shift || \"\";\n+\tmy %refs;\n \tmy @reflist;\n \n \tmy @refs;\n@@ -1127,14 +1128,21 @@ sub git_get_refs_list {\n \t\tor return;\n \twhile (my $line = <$fd>) {\n \t\tchomp $line;\n-\t\tif ($line =~ m/^([0-9a-fA-F]{40})\\t$ref_dir\\/?([^\\^]+)$/) {\n-\t\t\tpush @refs, { hash => $1, name => $2 };\n-\t\t} elsif ($line =~ m/^[0-9a-fA-F]{40}\\t$ref_dir\\/?(.*)\\^\\{\\}$/ &&\n-\t\t         $1 eq $refs[-1]{'name'}) {\n-\t\t\t# most likely a tag is followed by its peeled\n-\t\t\t# (deref) one, and when that happens we know the\n-\t\t\t# previous one was of type 'tag'.\n-\t\t\t$refs[-1]{'type'} = \"tag\";\n+\t\tif ($line =~ m/^([0-9a-fA-F]{40})\\trefs\\/($type\\/?([^\\^]+))(\\^\\{\\})?$/) {\n+\t\t\tif (defined $refs{$1}) {\n+\t\t\t\tpush @{$refs{$1}}, $2;\n+\t\t\t} else {\n+\t\t\t\t$refs{$1} = [ $2 ];\n+\t\t\t}\n+\n+\t\t\tif (! $4) { # unpeeled, direct reference\n+\t\t\t\tpush @refs, { hash => $1, name => $3 }; # without type\n+\t\t\t} elsif ($3 eq $refs[-1]{'name'}) {\n+\t\t\t\t# most likely a tag is followed by its peeled\n+\t\t\t\t# (deref) one, and when that happens we know the\n+\t\t\t\t# previous one was of type 'tag'.\n+\t\t\t\t$refs[-1]{'type'} = \"tag\";\n+\t\t\t}\n \t\t}\n \t}\n \tclose $fd;\n@@ -1150,7 +1158,7 @@ sub git_get_refs_list {\n \t}\n \t# sort refs by age\n \t@reflist = sort {$b->{'epoch'} <=> $a->{'epoch'}} @reflist;\n-\treturn \\@reflist;\n+\treturn (\\@reflist, \\%refs);\n }\n \n ## ----------------------------------------------------------------------\n@@ -2114,14 +2122,14 @@ sub git_tags_body {\n \n sub git_heads_body {\n \t# uses global variable $project\n-\tmy ($taglist, $head, $from, $to, $extra) = @_;\n+\tmy ($headlist, $head, $from, $to, $extra) = @_;\n \t$from = 0 unless defined $from;\n-\t$to = $#{$taglist} if (!defined $to || $#{$taglist} < $to);\n+\t$to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);\n \n \tprint \"<table class=\\\"heads\\\" cellspacing=\\\"0\\\">\\n\";\n \tmy $alternate = 0;\n \tfor (my $i = $from; $i <= $to; $i++) {\n-\t\tmy $entry = $taglist->[$i];\n+\t\tmy $entry = $headlist->[$i];\n \t\tmy %tag = %$entry;\n \t\tmy $curr = $tag{'id'} eq $head;\n \t\tif ($alternate) {\n@@ -2291,7 +2299,19 @@ sub git_summary {\n \n \tmy $owner = git_get_project_owner($project);\n \n-\tmy $refs = git_get_references();\n+\tmy ($reflist, $refs) = git_get_refs_list();\n+\n+\tmy @taglist;\n+\tmy @headlist;\n+\tforeach my $ref (@$reflist) {\n+\t\tif ($ref->{'name'} =~ s!^heads/!!) {\n+\t\t\tpush @headlist, $ref;\n+\t\t} else {\n+\t\t\t$ref->{'name'} =~ s!^tags/!!;\n+\t\t\tpush @taglist, $ref;\n+\t\t}\n+\t}\n+\n \tgit_header_html();\n \tgit_print_page_nav('summary','', $head);\n \n@@ -2321,17 +2341,15 @@ sub git_summary {\n \tgit_shortlog_body(\\@revlist, 0, 15, $refs,\n \t                  $cgi->a({-href => href(action=>\"shortlog\")}, \"...\"));\n \n-\tmy $taglist = git_get_refs_list(\"refs/tags\");\n-\tif (defined @$taglist) {\n+\tif (@taglist) {\n \t\tgit_print_header_div('tags');\n-\t\tgit_tags_body($taglist, 0, 15,\n+\t\tgit_tags_body(\\@taglist, 0, 15,\n \t\t              $cgi->a({-href => href(action=>\"tags\")}, \"...\"));\n \t}\n \n-\tmy $headlist = git_get_refs_list(\"refs/heads\");\n-\tif (defined @$headlist) {\n+\tif (@headlist) {\n \t\tgit_print_header_div('heads');\n-\t\tgit_heads_body($headlist, $head, 0, 15,\n+\t\tgit_heads_body(\\@headlist, $head, 0, 15,\n \t\t               $cgi->a({-href => href(action=>\"heads\")}, \"...\"));\n \t}\n \n@@ -2542,7 +2560,7 @@ sub git_tags {\n \tgit_print_page_nav('','', $head,undef,$head);\n \tgit_print_header_div('summary', $project);\n \n-\tmy $taglist = git_get_refs_list(\"refs/tags\");\n+\tmy ($taglist) = git_get_refs_list(\"tags\");\n \tif (defined @$taglist) {\n \t\tgit_tags_body($taglist);\n \t}\n@@ -2555,9 +2573,9 @@ sub git_heads {\n \tgit_print_page_nav('','', $head,undef,$head);\n \tgit_print_header_div('summary', $project);\n \n-\tmy $taglist = git_get_refs_list(\"refs/heads\");\n-\tif (defined @$taglist) {\n-\t\tgit_heads_body($taglist, $head);\n+\tmy ($headlist) = git_get_refs_list(\"heads\");\n+\tif (defined @$headlist) {\n+\t\tgit_heads_body($headlist, $head);\n \t}\n \tgit_footer_html();\n }\n-- \n1.4.2.1\n"},{"id":"27243","messageId":"7v64fijz90.fsf@assigned-by-dhcp.cox.net","threadId":"5603","inReplyTo":"200609191431.49641.jnareb@gmail.com","subject":"Re: [PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-20T16:09:47Z","receivedAt":"2006-09-20T16:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Instead of trying to read info/refs file, which might not be present\n> (we did fallback to git-ls-remote), always use git-peek-remote in\n> git_get_references.\n>\n> It is preparation for git_get_refs_info to also return references\n> info. We cannot use info/refs for git_get_refs_info as the information\n> contained therein is usually stale.\n\nWhat the patch does is sane, but I think the last sentence of\nthe proposed log message is not.  If info/refs is \"usually\nstale\", it is a bug in the repository to have such a stale file.\n\nThe real reason for this patch is that a repository served by\ngitweb is not necessarily meant to be fetched over HTTP and\ninfo/refs does not have to be there.\n"},{"id":"27272","messageId":"7veju6h0et.fsf@assigned-by-dhcp.cox.net","threadId":"5603","inReplyTo":"200609191433.22878.jnareb@gmail.com","subject":"Re: [PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-20T18:12:58Z","receivedAt":"2006-09-20T18:12:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Make git_get_refs_list do also work of git_get_references, to avoid\n> calling git-peek-remote twice.  Change meaning of git_get_refs_list\n> meaning: it is now type, and not a full path, e.g. we now use\n> git_get_refs_list(\"heads\") instead of former\n> git_get_refs_list(\"refs/heads\").\n>\n> Modify git_summary to use only one call to git_get_refs_list instead\n> of one call to git_get_references and two to git_get_refs_list.\n\nWill apply as-is; the following comments might be useful for\nfurther improvements.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 034a88c..01fae94 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1127,14 +1128,21 @@ sub git_get_refs_list {\n> +\t\tif ($line =~ m/^([0-9a-fA-F]{40})\\trefs\\/($type\\/?([^\\^]+))(\\^\\{\\})?$/) {\n> +\t\t\tif (defined $refs{$1}) {\n> +\t\t\t\tpush @{$refs{$1}}, $2;\n> +\t\t\t} else {\n> +\t\t\t\t$refs{$1} = [ $2 ];\n> +\t\t\t}\n\nLogically this should be \"exists $refs{$1}\" but defined would\nwork fine in this particular case, because the elements of the\nhash are always an arrayref (this is just an advise to have the\nright habit).\n\n> +\t\t\tif (! $4) { # unpeeled, direct reference\n> +\t\t\t\tpush @refs, { hash => $1, name => $3 }; # without type\n> +\t\t\t} elsif ($3 eq $refs[-1]{'name'}) {\n> +\t\t\t\t# most likely a tag is followed by its peeled\n> +\t\t\t\t# (deref) one, and when that happens we know the\n> +\t\t\t\t# previous one was of type 'tag'.\n> +\t\t\t\t$refs[-1]{'type'} = \"tag\";\n> +\t\t\t}\n>...\n> -\treturn \\@reflist;\n> +\treturn (\\@reflist, \\%refs);\n\nYou are maintaining reflist (an array of hashrefs each element\nof which describes name, type, hash and other parse_ref()\ninformation for the ref) and refs (a hash that maps from name to\nhash) separatly.  I wonder if this really has the performance\nadvantage over just compute and return reflist from here and\nhave the callers who need the mapping to derive it from the list\n(i.e.\n\n\tmy ($reflist) = git_get_refs_list();\n\tmy %refs = map { $_->{name} => $_->{hash} } @$reflist;\n).\n\n> @@ -2114,14 +2122,14 @@ sub git_tags_body {\n>  \n>  sub git_heads_body {\n>  \t# uses global variable $project\n> -\tmy ($taglist, $head, $from, $to, $extra) = @_;\n> +\tmy ($headlist, $head, $from, $to, $extra) = @_;\n>  \t$from = 0 unless defined $from;\n> -\t$to = $#{$taglist} if (!defined $to || $#{$taglist} < $to);\n> +\t$to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);\n>  \n>  \tprint \"<table class=\\\"heads\\\" cellspacing=\\\"0\\\">\\n\";\n>  \tmy $alternate = 0;\n>  \tfor (my $i = $from; $i <= $to; $i++) {\n> -\t\tmy $entry = $taglist->[$i];\n> +\t\tmy $entry = $headlist->[$i];\n>  \t\tmy %tag = %$entry;\n>  \t\tmy $curr = $tag{'id'} eq $head;\n>  \t\tif ($alternate) {\n\nI guess either would do but I think it is more conventional to\nsay:\n\n\tmy $limit = @list;\n        for (my $i = $from; $i < $limit; $i++) { ... }\n\nthan \n\n\tmy $limit = $#list;\n        for (my $i = $from; $i <= $limit; $i++) { ... }\n\nAt least the former would be easier to read for C types among us.\n\nThe remaining hunks are very nice improvements.\n\n> @@ -2291,7 +2299,19 @@ sub git_summary {\n> @@ -2321,17 +2341,15 @@ sub git_summary {\n> @@ -2542,7 +2560,7 @@ sub git_tags {\n> @@ -2555,9 +2573,9 @@ sub git_heads {\n"},{"id":"27273","messageId":"200609202013.38457.jnareb@gmail.com","threadId":"5603","inReplyTo":"7v64fijz90.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-20T18:13:38Z","receivedAt":"2006-09-20T18:13:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Instead of trying to read info/refs file, which might not be present\n> > (we did fallback to git-ls-remote), always use git-peek-remote in\n> > git_get_references.\n> >\n> > It is preparation for git_get_refs_info to also return references\n> > info. We cannot use info/refs for git_get_refs_info as the information\n> > contained therein is usually stale.\n> \n> What the patch does is sane, but I think the last sentence of\n> the proposed log message is not.  If info/refs is \"usually\n> stale\", it is a bug in the repository to have such a stale file.\n> \n> The real reason for this patch is that a repository served by\n> gitweb is not necessarily meant to be fetched over HTTP and\n> info/refs does not have to be there.\n\nIf the repository served by gitweb is updated using push, then\ninfo/refs updated using post-update hook is up-to-date. If repository\nis \"live\" repository, updated also using commit, rebase and such,\ninfo/refs is usually stale.\n\nIf there were post-commit hook, and it's contents was the default \npost-update hook, info/refs would be never stale. And we could read\nfrom into/refs, and fallback to git-peek-remote if it doesn't exist...\nbut we don't know if info/refs has current info.\n-- \nJakub Narebski\nPoland\n"},{"id":"27304","messageId":"200609210040.52886.jnareb@gmail.com","threadId":"5603","inReplyTo":"7veju6h0et.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-20T22:40:52Z","receivedAt":"2006-09-20T22:40:52Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski wrote:\n>> +                     if (! $4) { # unpeeled, direct reference\n>> +                             push @refs, { hash => $1, name => $3 }; # without type\n>> +                     } elsif ($3 eq $refs[-1]{'name'}) {\n>> +                             # most likely a tag is followed by its peeled\n>> +                             # (deref) one, and when that happens we know the\n>> +                             # previous one was of type 'tag'.\n>> +                             $refs[-1]{'type'} = \"tag\";\n>> +                     }\n>>...\n>> -     return \\@reflist;\n>> +     return (\\@reflist, \\%refs);\n> \n> You are maintaining reflist (an array of hashrefs each element\n> of which describes name, type, hash and other parse_ref()\n> information for the ref) and refs (a hash that maps from name to\n> hash) separatly.  I wonder if this really has the performance\n> advantage over just compute and return reflist from here and\n> have the callers who need the mapping to derive it from the list\n> (i.e.\n> \n>         my ($reflist) = git_get_refs_list();\n>         my %refs = map { $_->{name} => $_->{hash} } @$reflist;\n> ).\n\nThe %refs hash contain also map from peeled (dereferenced) object\nto ref name. The information about derefs is not present in @reflist.\nSo the answer is that you can't derive %refs from @reflist. \n-- \nJakub Narebski\nShadeHawk on #git\nPoland\n"},{"id":"27312","messageId":"7v4pv2f7vg.fsf@assigned-by-dhcp.cox.net","threadId":"5603","inReplyTo":"200609210040.52886.jnareb@gmail.com","subject":"Re: [PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-20T23:14:43Z","receivedAt":"2006-09-20T23:14:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> The %refs hash contain also map from peeled (dereferenced) object\n> to ref name. The information about derefs is not present in @reflist.\n> So the answer is that you can't derive %refs from @reflist. \n\nThere we found one candidate for further interface clean-up ;-).\n"},{"id":"27351","messageId":"45125657.4010001@op5.se","threadId":"5603","inReplyTo":"200609202013.38457.jnareb@gmail.com","subject":"Re: [PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-09-21T09:07:35Z","receivedAt":"2006-09-21T09:07:35Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Jakub Narebski wrote:\n> \n> If there were post-commit hook, and it's contents was the default \n> post-update hook, info/refs would be never stale. And we could read\n> from into/refs, and fallback to git-peek-remote if it doesn't exist...\n> but we don't know if info/refs has current info.\n\nls -l .git/hooks/post-commit\n-rw-r----- 1 exon exon 152 Sep 21 10:54 .git/hooks/post-commit\n\n\nMy post-update hook doesn't maintain info/refs for me. I'm sure that if \nsomeone set it up to do so they would also set the post-commit hook up \nto do the same.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"}]}