{"thread":{"id":"13091","subject":"[PATCH] Calculate lines changed for cvs log command","startedAt":"2008-04-13T15:27:18Z","lastAt":"2008-04-14T19:56:30Z","messageCount":5,"participants":["Fredrik Noring","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74258","messageId":"3F0821AA-C11D-4C42-A415-D346A7CD89F8@nocrew.org","threadId":"13091","inReplyTo":null,"subject":"[PATCH] Calculate lines changed for cvs log command","fromName":"Fredrik Noring","fromEmail":"noring@nocrew.org","sentAt":"2008-04-13T15:27:18Z","receivedAt":"2008-04-13T15:27:18Z","isPatch":true,"sender":{"key":"noring@nocrew.org","avatar":null},"body":"Hi,\n\nAttached patch attempts to report the correct lines changed when using  \nthe cvs log command with cvsserver. Is there any performance impact  \nusing \"git-log -1 --shortstat <object>\" instead of \"git-cat-file  \ncommit <object>\"?\n\n(Also, it would be nice if git-log --pretty=format:XXX could display  \ndiff information such as shortstat, to make parsing more robust.)\n\nPreviously, cvs log reported \"+2 -3\" for all commits. This patch  \nreports real values except when the commit is fetched from the SQLite  \nDB, which remains \"+2 -3\" at all times.\n\n(My mailer destroys inline patches, so I'm attaching it. Sorry about  \nthat.)\n\nSigned-off-by: Fredrik Noring <noring@nocrew.org>\n---\n  git-cvsserver.perl |   32 ++++++++++++++++++++++----------\n  1 files changed, 22 insertions(+), 10 deletions(-)\n\n\n\nFrom 6ff024e5238e52f4b61226187ffa2d21b99fe081 Mon Sep 17 00:00:00 2001\nFrom: Fredrik Noring <noring@nocrew.org>\nDate: Sun, 13 Apr 2008 16:54:40 +0200\nSubject: [PATCH] Calculate lines changed for cvs log command\n\nPreviously, \"+2 -3\" was reported for all commits. This patch reports\nlines changed except when the commit is fetched from the SQLite DB\n(which remains \"+2 -3\" at all times).\n\nSigned-off-by: Fredrik Noring <noring@nocrew.org>\n---\n git-cvsserver.perl |   32 ++++++++++++++++++++++----------\n 1 files changed, 22 insertions(+), 10 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 29dbfc9..5d31a95 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1729,8 +1729,9 @@ sub req_log\n             # reformat the date for log output\n             $revision->{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST->{$2}, $1, $4 ) if ( $revision->{modified} =~ /(\\d+)\\s+(\\w+)\\s+(\\d+)\\s+(\\S+)/ and defined($DATE_LIST->{$2}) );\n             $revision->{author} = cvs_author($revision->{author});\n-            print \"M date: $revision->{modified};  author: $revision->{author};  state: \" . ( $revision->{filehash} eq \"deleted\" ? \"dead\" : \"Exp\" ) . \";  lines: +2 -3\\n\";\n-            my $commitmessage = $updater->commitmessage($revision->{commithash});\n+            ( my $commitmessage, my $lines_changed ) =\n+\t\t$updater->commitinfo($revision->{commithash});\n+            print \"M date: $revision->{modified};  author: $revision->{author};  state: \" . ( $revision->{filehash} eq \"deleted\" ? \"dead\" : \"Exp\" ) . \";  lines: \" . $lines_changed . \"\\n\";\n             $commitmessage =~ s/^/M /mg;\n             print $commitmessage . \"\\n\";\n         }\n@@ -3023,12 +3024,13 @@ sub getmeta\n     return $db_query->fetchrow_hashref;\n }\n \n-=head2 commitmessage\n+=head2 commitinfo\n \n-this function takes a commithash and returns the commit message for that commit\n+this function takes a commithash and returns the commit message and\n+lines changed for that commit\n \n =cut\n-sub commitmessage\n+sub commitinfo\n {\n     my $self = shift;\n     my $commithash = shift;\n@@ -3045,14 +3047,24 @@ sub commitmessage\n     if ( defined ( $message ) )\n     {\n         $message .= \" \" if ( $message =~ /\\n$/ );\n-        return $message;\n+        return ( $message, \"+2 -3\" );   # FIXME: What's the real lines changed?\n     }\n \n-    my @lines = safe_pipe_capture(\"git-cat-file\", \"commit\", $commithash);\n-    shift @lines while ( $lines[0] =~ /\\S/ );\n-    $message = join(\"\",@lines);\n+    my @lines = safe_pipe_capture(\"git-log\", \"-1\", \"--pretty=format:%s%n%b\",\n+\t\t\t\t  \"--shortstat\", $commithash);\n+    my $files_changed = 0;\n+    my $insertions = 0;\n+    my $deletions = 0;\n+    ( $files_changed, $insertions, $deletions ) = ( $lines[-1] =~ / (\\d+) files changed, (\\d+) insertions.*, (\\d+) deletions/ );\n+    if ( $files_changed )\n+    {\n+\tpop(@lines);   # Remove shortstat\n+\tpop(@lines);   # Remove extra newline prior to shortstat\n+    }\n+    map(s/\\n$//s, @lines);\n+    $message = join(\"\\n\", @lines);\n     $message .= \" \" if ( $message =~ /\\n$/ );\n-    return $message;\n+    return ( $message, sprintf(\"+%d -%d\", $insertions, $deletions) );\n }\n \n =head2 gethistory\n-- \n1.5.5.40.g4cdda.dirty\n\n\n\n\n"},{"id":"74260","messageId":"m3zlrxsnbt.fsf@localhost.localdomain","threadId":"13091","inReplyTo":"3F0821AA-C11D-4C42-A415-D346A7CD89F8@nocrew.org","subject":"Re: [PATCH] Calculate lines changed for cvs log command","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-13T16:59:29Z","receivedAt":"2008-04-13T16:59:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Fredrik Noring <noring@nocrew.org> writes:\n\n> (My mailer destroys inline patches, so I'm attaching it. Sorry about\n> that.)\n\nCould you please attach it as 1.) attachement=inline if possible, to\nhave them displayed along the email; 2.) use text/plain mimetype, not\napplication/octet-stream (you might need to change patch extension\nfrom *.patch to *.txt; you can change default extension of files\ngenerated by git-format-patch via format.suffix configuration\nvariable), and what is tied with it 3.) use Transfer-Encoding: 8bit\n(or something like that), not base64, and preferably not\nquoted-printable?\n\nOr change your mailer (or configure it), or use git-send-email.\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"74261","messageId":"20AD7252-19A2-443F-88CF-F871AE7A392E@nocrew.org","threadId":"13091","inReplyTo":"m3zlrxsnbt.fsf@localhost.localdomain","subject":"Re: [PATCH] Calculate lines changed for cvs log command","fromName":"Fredrik Noring","fromEmail":"noring@nocrew.org","sentAt":"2008-04-13T18:44:29Z","receivedAt":"2008-04-13T18:44:29Z","isPatch":true,"sender":{"key":"noring@nocrew.org","avatar":null},"body":"Hi Jakub,\n\nOK, I'll look deeper into the mailer issues. Any comments on the  \nactual code in the patch, and the question regarding git-log vs git- \ncat-file?\n\nSpeaking of which -- is there any particular reason for not running  \ngit-log once, instead of forking it (or git-cat-file) for every commit  \nin the log? There seems to be a special case for merges, but it'd  \nappear to be more efficient to query the SQLite DB for the commits  \nprovided by git-log rather than the other way around, no?\n\nAny thoughts?\n\n(git-send-email appears to fail for me without providing an error.)\n\nMany thanks,\nFredrik\n\n13 apr 2008 kl. 18.59 skrev Jakub Narebski:\n> Fredrik Noring <noring@nocrew.org> writes:\n>\n>> (My mailer destroys inline patches, so I'm attaching it. Sorry about\n>> that.)\n>\n> Could you please attach it as 1.) attachement=inline if possible, to\n> have them displayed along the email; 2.) use text/plain mimetype, not\n> application/octet-stream (you might need to change patch extension\n> from *.patch to *.txt; you can change default extension of files\n> generated by git-format-patch via format.suffix configuration\n> variable), and what is tied with it 3.) use Transfer-Encoding: 8bit\n> (or something like that), not base64, and preferably not\n> quoted-printable?\n>\n> Or change your mailer (or configure it), or use git-send-email.\n> -- \n> Jakub Narebski\n> Poland\n> ShadeHawk on #git\n>\n"},{"id":"74270","messageId":"200804132243.40909.jnareb@gmail.com","threadId":"13091","inReplyTo":"20AD7252-19A2-443F-88CF-F871AE7A392E@nocrew.org","subject":"Re: [PATCH] Calculate lines changed for cvs log command","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-13T20:43:39Z","receivedAt":"2008-04-13T20:43:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Could you please do not toppost?  It is against natural reading order.\n\nDnia niedziela 13. kwietnia 2008 20:44, Fredrik Noring napisał:\n> 13 apr 2008 kl. 18.59 skrev Jakub Narebski:\n>> Fredrik Noring <noring@nocrew.org> writes:\n>>\n>>> (My mailer destroys inline patches, so I'm attaching it. Sorry about\n>>> that.)\n>>\n>> Could you please attach it as 1.) attachement=inline if possible, to\n>> have them displayed along the email; 2.) use text/plain mimetype, not\n>> application/octet-stream (you might need to change patch extension\n>> from *.patch to *.txt; you can change default extension of files\n>> generated by git-format-patch via format.suffix configuration\n>> variable), and what is tied with it 3.) use Transfer-Encoding: 8bit\n>> (or something like that), not base64, and preferably not\n>> quoted-printable?\n>>\n>> Or change your mailer (or configure it), or use git-send-email.\n>> \n> OK, I'll look deeper into the mailer issues. Any comments on the  \n> actual code in the patch, and the question regarding git-log vs git- \n> cat-file?\n\nFirst, having patch as attachement, moreover as _binary_ and _encoded_ \nattachement makes it hard to comment on it.  And if you make it hard to \ncomment on patch, people woundn't do it.\n\nSecond, your approach with \"--pretty=format:%s%n%b\" might be a good\nidea, but you don't get all the information as you get from \ngit-cat-file: no tree info, no true (recorded) parents info, no \ncommitter info, no author info.  I don't know if git-cvsserver makes \nuse of that info or not; from the patch it looks as if it discards\nall but commit message (message body). \n\nBTW. I'm not Perl expert, but if you want to discard two last\nelements in arrays, wouldn't using splice (or just decreasing\n$#lines by 2) be simpler solution?\n\n> Speaking of which -- is there any particular reason for not running  \n> git-log once, instead of forking it (or git-cat-file) for every commit  \n> in the log? There seems to be a special case for merges, but it'd  \n> appear to be more efficient to query the SQLite DB for the commits  \n> provided by git-log rather than the other way around, no?\n\nAbout git-log vs git-cat-file: take a look how gitweb does it, with \nparse_commits and parse_commit_text, and commit 756bbf548dbef5b738c\nby Robert Fitzsimons introducing it.  Note that IIRC this commit\npredates --pretty=format:<fmt> option to git-log / git-rev-list.\n\n> Any thoughts?\n> \n> (git-send-email appears to fail for me without providing an error.)\n\nYou have either configure sendmail, or set mail.smtp* options (or \nsomething like that) correctly.\n\nBTW. cannot you turn off \"format=flowed\" in Apple Mail?\n-- \nJakub Narebski\nPoland\n"},{"id":"74377","messageId":"FFB8FCA9-AD9C-4F7F-9238-0CB0CA3204AF@nocrew.org","threadId":"13091","inReplyTo":"200804132243.40909.jnareb@gmail.com","subject":"Re: [PATCH] Calculate lines changed for cvs log command","fromName":"Fredrik Noring","fromEmail":"noring@nocrew.org","sentAt":"2008-04-14T19:56:30Z","receivedAt":"2008-04-14T19:56:30Z","isPatch":true,"sender":{"key":"noring@nocrew.org","avatar":null},"body":"Hi Jakub,\n\n13 apr 2008 kl. 22.43 skrev Jakub Narebski:\n> First, having patch as attachement, moreover as _binary_ and _encoded_\n> attachement makes it hard to comment on it.  And if you make it hard  \n> to\n> comment on patch, people woundn't do it.\n\nAgain, my sincere apologies. I have a feeling my current mailer never  \nwill meet the patch submission requirements for this mailing list.  \nHowever, I will replace it at the next opportunity in order to comply.\n\nThe goal with these patches is to make cvsserver commands such as \"cvs  \nlog\" identical to the real thing, so that cvs users would feel at home  \nafter a cvs import to git. I'm getting closer with these tweaks/fixes  \nto the cvsserver. So far mainly handling of whitespace and calculating  \nproper \"shortstats\". Decent performance would be nice as well. I'm  \nplaying around with this but I'm in no hurry, so feel free to ignore. :)\n\n> Second, your approach with \"--pretty=format:%s%n%b\" might be a good\n> idea, but you don't get all the information as you get from\n> git-cat-file: no tree info, no true (recorded) parents info, no\n> committer info, no author info.  I don't know if git-cvsserver makes\n> use of that info or not; from the patch it looks as if it discards\n> all but commit message (message body).\n\nYeah -- it's all discarded. A small issue is that \"--pretty=format:%s%n \n%b\" only outputs the same commit message as git-cat-file in ~95 % of  \nmy test cases. I've seen differences regarding newlines after the  \nsubject line (git-cat-file seems to output this correctly, but not %s%n \n%b). Something to investigate further, perhaps.\n\n> BTW. I'm not Perl expert, but if you want to discard two last\n> elements in arrays, wouldn't using splice (or just decreasing\n> $#lines by 2) be simpler solution?\n\nI went for clarity and felt pop() made the job. ;)\n\n> About git-log vs git-cat-file: take a look how gitweb does it, with\n> parse_commits and parse_commit_text, and commit 756bbf548dbef5b738c\n> by Robert Fitzsimons introducing it.  Note that IIRC this commit\n> predates --pretty=format:<fmt> option to git-log / git-rev-list.\n\nSure, thanks!\n\n> BTW. cannot you turn off \"format=flowed\" in Apple Mail?\n\nWell, it appears easier to replace it altogether in order to follow  \nthe patch submission rules. I like Gnome Evolution and it'll hopefully  \nwork nicely in this case.\n\n> Could you please do not toppost?  It is against natural reading order.\n\n>\n\nHeh. Very funny you wrote that as a toppost yourself. ;)\n\nThanks,\nFredrik\n"}]}