{"thread":{"id":"12506","subject":"[PATCH] gitweb: Fix and simplify pickaxe search","startedAt":"2008-03-04T23:15:53Z","lastAt":"2008-03-05T08:31:55Z","messageCount":2,"participants":["Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71010","messageId":"20080304230905.17447.32472.stgit@localhost.localdomain","threadId":"12506","inReplyTo":null,"subject":"[PATCH] gitweb: Fix and simplify pickaxe search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-04T23:15:53Z","receivedAt":"2008-03-04T23:15:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Instead of using \"git-rev-list | git-diff-tree\" pipeline for pickaxe\nsearch, use git-log with appropriate options.  Besides reducing number\nof forks by one, this allows to use list form of open, which in turn\nallow to not worry about quoting arguments and to avoid forking shell.\n\nThe options to git-log were chosen to reduce required changes in\npickaxe git command output parsing; gitweb still parses returned\ncommits one by one.\n\nParsing \"pickaxe\" output is simplified: git_search now reuses\nparse_difftree_raw_line and writes affected files as they arrive using\nthe fact that commit name goes always before [raw] diff.\n\nWhile at it long bug of pickaxe search was fixed, namely that the last\ncommit found by pickaxe search was never shown.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nOn linux using 'git-log' instead of 'git-rev-list' ... 'git-diff-tree'\npipeline improves performance only insignificantly, at least on\nLinux. But that might differ on fork-performance challenged operating\nsystems.\n\nLoooong time bug: if I have checked correctly, it was there from\nintroduction of pickaxe search in c994d62 (v220), when gitweb was\ndeveloped separately, mainly by Kay Sievers.\n\n gitweb/gitweb.perl |   85 ++++++++++++++++++++++++++--------------------------\n 1 files changed, 43 insertions(+), 42 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 922dee9..0bf679e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5305,51 +5305,19 @@ sub git_search {\n \t\tprint \"<table class=\\\"pickaxe search\\\">\\n\";\n \t\tmy $alternate = 1;\n \t\t$/ = \"\\n\";\n-\t\tmy $git_command = git_cmd_str();\n-\t\tmy $searchqtext = $searchtext;\n-\t\t$searchqtext =~ s/'/'\\\\''/;\n-\t\tmy $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';\n-\t\topen my $fd, \"-|\", \"$git_command rev-list $hash | \" .\n-\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\' $pickaxe_flags\";\n+\t\topen my $fd, '-|', git_cmd(), '--no-pager', 'log',\n+\t\t\t'--pretty=format:%H', '--no-abbrev', '--raw', \"-S$searchtext\",\n+\t\t\t($search_use_regexp ? '--pickaxe-regex' : ());\n \t\tundef %co;\n \t\tmy @files;\n \t\twhile (my $line = <$fd>) {\n-\t\t\tif (%co && $line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\\t(.*)$/) {\n-\t\t\t\tmy %set;\n-\t\t\t\t$set{'file'} = $6;\n-\t\t\t\t$set{'from_id'} = $3;\n-\t\t\t\t$set{'to_id'} = $4;\n-\t\t\t\t$set{'id'} = $set{'to_id'};\n-\t\t\t\tif ($set{'id'} =~ m/0{40}/) {\n-\t\t\t\t\t$set{'id'} = $set{'from_id'};\n-\t\t\t\t}\n-\t\t\t\tif ($set{'id'} =~ m/0{40}/) {\n-\t\t\t\t\tnext;\n-\t\t\t\t}\n-\t\t\t\tpush @files, \\%set;\n-\t\t\t} elsif ($line =~ m/^([0-9a-fA-F]{40})$/){\n+\t\t\tchomp $line;\n+\t\t\tnext unless $line;\n+\n+\t\t\tmy %set = parse_difftree_raw_line($line);\n+\t\t\tif (defined $set{'commit'}) {\n+\t\t\t\t# finish previous commit\n \t\t\t\tif (%co) {\n-\t\t\t\t\tif ($alternate) {\n-\t\t\t\t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tprint \"<tr class=\\\"light\\\">\\n\";\n-\t\t\t\t\t}\n-\t\t\t\t\t$alternate ^= 1;\n-\t\t\t\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n-\t\t\t\t\tprint \"<td title=\\\"$co{'age_string_age'}\\\"><i>$co{'age_string_date'}</i></td>\\n\" .\n-\t\t\t\t\t      \"<td><i>\" . $author . \"</i></td>\\n\" .\n-\t\t\t\t\t      \"<td>\" .\n-\t\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n-\t\t\t\t\t              -class => \"list subject\"},\n-\t\t\t\t\t              chop_and_escape_str($co{'title'}, 50) . \"<br/>\");\n-\t\t\t\t\twhile (my $setref = shift @files) {\n-\t\t\t\t\t\tmy %set = %$setref;\n-\t\t\t\t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash_base=>$co{'id'},\n-\t\t\t\t\t\t                             hash=>$set{'id'}, file_name=>$set{'file'}),\n-\t\t\t\t\t\t              -class => \"list\"},\n-\t\t\t\t\t\t              \"<span class=\\\"match\\\">\" . esc_path($set{'file'}) . \"</span>\") .\n-\t\t\t\t\t\t      \"<br/>\\n\";\n-\t\t\t\t\t}\n \t\t\t\t\tprint \"</td>\\n\" .\n \t\t\t\t\t      \"<td class=\\\"link\\\">\" .\n \t\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'})}, \"commit\") .\n@@ -5358,11 +5326,44 @@ sub git_search {\n \t\t\t\t\tprint \"</td>\\n\" .\n \t\t\t\t\t      \"</tr>\\n\";\n \t\t\t\t}\n-\t\t\t\t%co = parse_commit($1);\n+\n+\t\t\t\tif ($alternate) {\n+\t\t\t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n+\t\t\t\t} else {\n+\t\t\t\t\tprint \"<tr class=\\\"light\\\">\\n\";\n+\t\t\t\t}\n+\t\t\t\t$alternate ^= 1;\n+\t\t\t\t%co = parse_commit($set{'commit'});\n+\t\t\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n+\t\t\t\tprint \"<td title=\\\"$co{'age_string_age'}\\\"><i>$co{'age_string_date'}</i></td>\\n\" .\n+\t\t\t\t      \"<td><i>$author</i></td>\\n\" .\n+\t\t\t\t      \"<td>\" .\n+\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n+\t\t\t\t              -class => \"list subject\"},\n+\t\t\t\t              chop_and_escape_str($co{'title'}, 50) . \"<br/>\");\n+\t\t\t} elsif (defined $set{'to_id'}) {\n+\t\t\t\tnext if ($set{'to_id'} =~ m/^0{40}$/);\n+\n+\t\t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash_base=>$co{'id'},\n+\t\t\t\t                             hash=>$set{'to_id'}, file_name=>$set{'to_file'}),\n+\t\t\t\t              -class => \"list\"},\n+\t\t\t\t              \"<span class=\\\"match\\\">\" . esc_path($set{'file'}) . \"</span>\") .\n+\t\t\t\t      \"<br/>\\n\";\n \t\t\t}\n \t\t}\n \t\tclose $fd;\n \n+\t\t# finish last commit (warning: repetition!)\n+\t\tif (%co) {\n+\t\t\tprint \"</td>\\n\" .\n+\t\t\t      \"<td class=\\\"link\\\">\" .\n+\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'})}, \"commit\") .\n+\t\t\t      \" | \" .\n+\t\t\t      $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$co{'id'})}, \"tree\");\n+\t\t\tprint \"</td>\\n\" .\n+\t\t\t      \"</tr>\\n\";\n+\t\t}\n+\n \t\tprint \"</table>\\n\";\n \t}\n \n\n\n-- \nStacked GIT 0.14.1\ngit version 1.5.4.2\nPython version 2.4.3 (#1, Jun 13 2006, 16:41:18) \n[GCC 4.0.2 20051125 (Red Hat 4.0.2-8)]\n"},{"id":"71047","messageId":"20080305083019.20727.66526.stgit@localhost.localdomain","threadId":"12506","inReplyTo":"20080304230905.17447.32472.stgit@localhost.localdomain","subject":"[PATCH v2] gitweb: Fix and simplify pickaxe search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-05T08:31:55Z","receivedAt":"2008-03-05T08:31:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Instead of using \"git-rev-list | git-diff-tree\" pipeline for pickaxe\nsearch, use git-log with appropriate options.  Besides reducing number\nof forks by one, this allows to use list form of open, which in turn\nallow to not worry about quoting arguments and to avoid forking shell.\n\nThe options to git-log were chosen to reduce required changes in\npickaxe git command output parsing; gitweb still parses returned\ncommits one by one.\n\nParsing \"pickaxe\" output is simplified: git_search now reuses\nparse_difftree_raw_line and writes affected files as they arrive using\nthe fact that commit name goes always before [raw] diff.\n\nWhile at it long bug of pickaxe search was fixed, namely that the last\ncommit found by pickaxe search was never shown.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI forgot to include @diff_opts in git-log invocation, therefore for\nexample pickaxe was run without (default) rename detection.\n\n gitweb/gitweb.perl |   85 ++++++++++++++++++++++++++--------------------------\n 1 files changed, 43 insertions(+), 42 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 922dee9..ec73cb1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5305,51 +5305,19 @@ sub git_search {\n \t\tprint \"<table class=\\\"pickaxe search\\\">\\n\";\n \t\tmy $alternate = 1;\n \t\t$/ = \"\\n\";\n-\t\tmy $git_command = git_cmd_str();\n-\t\tmy $searchqtext = $searchtext;\n-\t\t$searchqtext =~ s/'/'\\\\''/;\n-\t\tmy $pickaxe_flags = $search_use_regexp ? '--pickaxe-regex' : '';\n-\t\topen my $fd, \"-|\", \"$git_command rev-list $hash | \" .\n-\t\t\t\"$git_command diff-tree -r --stdin -S\\'$searchqtext\\' $pickaxe_flags\";\n+\t\topen my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts,\n+\t\t\t'--pretty=format:%H', '--no-abbrev', '--raw', \"-S$searchtext\",\n+\t\t\t($search_use_regexp ? '--pickaxe-regex' : ());\n \t\tundef %co;\n \t\tmy @files;\n \t\twhile (my $line = <$fd>) {\n-\t\t\tif (%co && $line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\\t(.*)$/) {\n-\t\t\t\tmy %set;\n-\t\t\t\t$set{'file'} = $6;\n-\t\t\t\t$set{'from_id'} = $3;\n-\t\t\t\t$set{'to_id'} = $4;\n-\t\t\t\t$set{'id'} = $set{'to_id'};\n-\t\t\t\tif ($set{'id'} =~ m/0{40}/) {\n-\t\t\t\t\t$set{'id'} = $set{'from_id'};\n-\t\t\t\t}\n-\t\t\t\tif ($set{'id'} =~ m/0{40}/) {\n-\t\t\t\t\tnext;\n-\t\t\t\t}\n-\t\t\t\tpush @files, \\%set;\n-\t\t\t} elsif ($line =~ m/^([0-9a-fA-F]{40})$/){\n+\t\t\tchomp $line;\n+\t\t\tnext unless $line;\n+\n+\t\t\tmy %set = parse_difftree_raw_line($line);\n+\t\t\tif (defined $set{'commit'}) {\n+\t\t\t\t# finish previous commit\n \t\t\t\tif (%co) {\n-\t\t\t\t\tif ($alternate) {\n-\t\t\t\t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tprint \"<tr class=\\\"light\\\">\\n\";\n-\t\t\t\t\t}\n-\t\t\t\t\t$alternate ^= 1;\n-\t\t\t\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n-\t\t\t\t\tprint \"<td title=\\\"$co{'age_string_age'}\\\"><i>$co{'age_string_date'}</i></td>\\n\" .\n-\t\t\t\t\t      \"<td><i>\" . $author . \"</i></td>\\n\" .\n-\t\t\t\t\t      \"<td>\" .\n-\t\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n-\t\t\t\t\t              -class => \"list subject\"},\n-\t\t\t\t\t              chop_and_escape_str($co{'title'}, 50) . \"<br/>\");\n-\t\t\t\t\twhile (my $setref = shift @files) {\n-\t\t\t\t\t\tmy %set = %$setref;\n-\t\t\t\t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash_base=>$co{'id'},\n-\t\t\t\t\t\t                             hash=>$set{'id'}, file_name=>$set{'file'}),\n-\t\t\t\t\t\t              -class => \"list\"},\n-\t\t\t\t\t\t              \"<span class=\\\"match\\\">\" . esc_path($set{'file'}) . \"</span>\") .\n-\t\t\t\t\t\t      \"<br/>\\n\";\n-\t\t\t\t\t}\n \t\t\t\t\tprint \"</td>\\n\" .\n \t\t\t\t\t      \"<td class=\\\"link\\\">\" .\n \t\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'})}, \"commit\") .\n@@ -5358,11 +5326,44 @@ sub git_search {\n \t\t\t\t\tprint \"</td>\\n\" .\n \t\t\t\t\t      \"</tr>\\n\";\n \t\t\t\t}\n-\t\t\t\t%co = parse_commit($1);\n+\n+\t\t\t\tif ($alternate) {\n+\t\t\t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n+\t\t\t\t} else {\n+\t\t\t\t\tprint \"<tr class=\\\"light\\\">\\n\";\n+\t\t\t\t}\n+\t\t\t\t$alternate ^= 1;\n+\t\t\t\t%co = parse_commit($set{'commit'});\n+\t\t\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n+\t\t\t\tprint \"<td title=\\\"$co{'age_string_age'}\\\"><i>$co{'age_string_date'}</i></td>\\n\" .\n+\t\t\t\t      \"<td><i>$author</i></td>\\n\" .\n+\t\t\t\t      \"<td>\" .\n+\t\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n+\t\t\t\t              -class => \"list subject\"},\n+\t\t\t\t              chop_and_escape_str($co{'title'}, 50) . \"<br/>\");\n+\t\t\t} elsif (defined $set{'to_id'}) {\n+\t\t\t\tnext if ($set{'to_id'} =~ m/^0{40}$/);\n+\n+\t\t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash_base=>$co{'id'},\n+\t\t\t\t                             hash=>$set{'to_id'}, file_name=>$set{'to_file'}),\n+\t\t\t\t              -class => \"list\"},\n+\t\t\t\t              \"<span class=\\\"match\\\">\" . esc_path($set{'file'}) . \"</span>\") .\n+\t\t\t\t      \"<br/>\\n\";\n \t\t\t}\n \t\t}\n \t\tclose $fd;\n \n+\t\t# finish last commit (warning: repetition!)\n+\t\tif (%co) {\n+\t\t\tprint \"</td>\\n\" .\n+\t\t\t      \"<td class=\\\"link\\\">\" .\n+\t\t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'})}, \"commit\") .\n+\t\t\t      \" | \" .\n+\t\t\t      $cgi->a({-href => href(action=>\"tree\", hash=>$co{'tree'}, hash_base=>$co{'id'})}, \"tree\");\n+\t\t\tprint \"</td>\\n\" .\n+\t\t\t      \"</tr>\\n\";\n+\t\t}\n+\n \t\tprint \"</table>\\n\";\n \t}\n \n\n"}]}