{"thread":{"id":"4250","subject":"[PATCH 2/2] cvsimport: cleanup commit function","startedAt":"2006-05-23T07:27:46Z","lastAt":"2006-05-23T07:27:46Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"20515","messageId":"22174.0949004472$1148369283@news.gmane.org","threadId":"4250","inReplyTo":"1148369266352-git-send-email-1","subject":"[PATCH 2/2] cvsimport: cleanup commit function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-05-23T07:27:46Z","receivedAt":"2006-05-23T07:27:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This change attempts to clean up the commit function to make it a bit\neasier to read (or at least the first half of it). It also improves\nrobustness and performance. Specifically:\n  - report get_headref errors on opening ref unless the error is ENOENT\n  - use regex to check for sha1 instead of length\n  - use lexically scoped filehandles which get cleaned up automagically\n  - check for error on both 'print' and 'close' (since output is buffered)\n  - avoid \"fork, do some perl, then exec\" in commit(). It's not necessary,\n    and we probably end up COW'ing parts of the perl process. Plus the code\n    is much smaller because we can use open2()\n  - avoid calling strftime over and over (mainly a readability cleanup)\n\n---\n\nThis is a repost with some minor fixups from Junio (and based off of the\nfixed 1/2 patch).\n\n3408c8d8364f816a7c4a34a03045f466bf028540\n git-cvsimport.perl |  150 ++++++++++++++++++++++------------------------------\n 1 files changed, 64 insertions(+), 86 deletions(-)\n\n3408c8d8364f816a7c4a34a03045f466bf028540\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex a65bea6..219f6dc 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -23,7 +23,7 @@ use File::Basename qw(basename dirname);\n use Time::Local;\n use IO::Socket;\n use IO::Pipe;\n-use POSIX qw(strftime dup2);\n+use POSIX qw(strftime dup2 :errno_h);\n use IPC::Open2;\n \n $SIG{'PIPE'}=\"IGNORE\";\n@@ -429,22 +429,25 @@ sub getwd() {\n \treturn $pwd;\n }\n \n+sub is_sha1 {\n+\tmy $s = shift;\n+\treturn $s =~ /^[a-f0-9]{40}$/;\n+}\n \n-sub get_headref($$) {\n+sub get_headref ($$) {\n     my $name    = shift;\n     my $git_dir = shift; \n-    my $sha;\n     \n-    if (open(C,\"$git_dir/refs/heads/$name\")) {\n-\tchomp($sha = <C>);\n-\tclose(C);\n-\tlength($sha) == 40\n-\t    or die \"Cannot get head id for $name ($sha): $!\\n\";\n+    my $f = \"$git_dir/refs/heads/$name\";\n+    if(open(my $fh, $f)) {\n+      \t    chomp(my $r = <$fh>);\n+\t    is_sha1($r) or die \"Cannot get head id for $name ($r): $!\";\n+\t    return $r;\n     }\n-    return $sha;\n+    die \"unable to open $f: $!\" unless $! == POSIX::ENOENT;\n+    return undef;\n }\n \n-\n -d $git_tree\n \tor mkdir($git_tree,0777)\n \tor die \"Could not create $git_tree: $!\";\n@@ -561,90 +564,67 @@ #---------------------\n \n my $state = 0;\n \n-my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);\n-my(@old,@new,@skipped);\n-sub commit {\n-\tmy $pid;\n-\n+sub update_index (\\@\\@) {\n+\tmy $old = shift;\n+\tmy $new = shift;\n \topen(my $fh, '|-', qw(git-update-index -z --index-info))\n \t\tor die \"unable to open git-update-index: $!\";\n \tprint $fh \n \t\t(map { \"0 0000000000000000000000000000000000000000\\t$_\\0\" }\n-\t\t\t@old),\n+\t\t\t@$old),\n \t\t(map { '100' . sprintf('%o', $_->[0]) . \" $_->[1]\\t$_->[2]\\0\" }\n-\t\t\t@new)\n+\t\t\t@$new)\n \t\tor die \"unable to write to git-update-index: $!\";\n \tclose $fh\n \t\tor die \"unable to write to git-update-index: $!\";\n \t$? and die \"git-update-index reported error: $?\";\n-\t@old = @new = ();\n+}\n \n-\t$pid = open(C,\"-|\");\n-\tdie \"Cannot fork: $!\" unless defined $pid;\n-\tunless($pid) {\n-\t\texec(\"git-write-tree\");\n-\t\tdie \"Cannot exec git-write-tree: $!\\n\";\n-\t}\n-\tchomp(my $tree = <C>);\n-\tlength($tree) == 40\n-\t\tor die \"Cannot get tree id ($tree): $!\\n\";\n-\tclose(C)\n+sub write_tree () {\n+\topen(my $fh, '-|', qw(git-write-tree))\n+\t\tor die \"unable to open git-write-tree: $!\";\n+\tchomp(my $tree = <$fh>);\n+\tis_sha1($tree)\n+\t\tor die \"Cannot get tree id ($tree): $!\";\n+\tclose($fh)\n \t\tor die \"Error running git-write-tree: $?\\n\";\n \tprint \"Tree ID $tree\\n\" if $opt_v;\n+\treturn $tree;\n+}\n \n-\tmy $parent = \"\";\n-\tif(open(C,\"$git_dir/refs/heads/$last_branch\")) {\n-\t\tchomp($parent = <C>);\n-\t\tclose(C);\n-\t\tlength($parent) == 40\n-\t\t\tor die \"Cannot get parent id ($parent): $!\\n\";\n-\t\tprint \"Parent ID $parent\\n\" if $opt_v;\n-\t}\n-\n-\tmy $pr = IO::Pipe->new() or die \"Cannot open pipe: $!\\n\";\n-\tmy $pw = IO::Pipe->new() or die \"Cannot open pipe: $!\\n\";\n-\t$pid = fork();\n-\tdie \"Fork: $!\\n\" unless defined $pid;\n-\tunless($pid) {\n-\t\t$pr->writer();\n-\t\t$pw->reader();\n-\t\topen(OUT,\">&STDOUT\");\n-\t\tdup2($pw->fileno(),0);\n-\t\tdup2($pr->fileno(),1);\n-\t\t$pr->close();\n-\t\t$pw->close();\n-\n-\t\tmy @par = ();\n-\t\t@par = (\"-p\",$parent) if $parent;\n-\n-\t\t# loose detection of merges\n-\t\t# based on the commit msg\n-\t\tforeach my $rx (@mergerx) {\n-\t\t\tif ($logmsg =~ $rx) {\n-\t\t\t\tmy $mparent = $1;\n-\t\t\t\tif ($mparent eq 'HEAD') { $mparent = $opt_o };\n-\t\t\t\tif ( -e \"$git_dir/refs/heads/$mparent\") {\n-\t\t\t\t\t$mparent = get_headref($mparent, $git_dir);\n-\t\t\t\t\tpush @par, '-p', $mparent;\n-\t\t\t\t\tprint OUT \"Merge parent branch: $mparent\\n\" if $opt_v;\n-\t\t\t\t}\n-\t\t\t}\n+my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);\n+my(@old,@new,@skipped);\n+sub commit {\n+\tupdate_index(@old, @new);\n+\t@old = @new = ();\n+\tmy $tree = write_tree();\n+\tmy $parent = get_headref($last_branch, $git_dir);\n+\tprint \"Parent ID \" . ($parent ? $parent : \"(empty)\") . \"\\n\" if $opt_v;\n+\n+\tmy @commit_args;\n+\tpush @commit_args, (\"-p\", $parent) if $parent;\n+\n+\t# loose detection of merges\n+\t# based on the commit msg\n+\tforeach my $rx (@mergerx) {\n+\t\tnext unless $logmsg =~ $rx && $1;\n+\t\tmy $mparent = $1 eq 'HEAD' ? $opt_o : $1;\n+\t\tif(my $sha1 = get_headref($mparent, $git_dir)) {\n+\t\t\tpush @commit_args, '-p', $mparent;\n+\t\t\tprint \"Merge parent branch: $mparent\\n\" if $opt_v;\n \t\t}\n-\n-\t\texec(\"env\",\n-\t\t\t\"GIT_AUTHOR_NAME=$author_name\",\n-\t\t\t\"GIT_AUTHOR_EMAIL=$author_email\",\n-\t\t\t\"GIT_AUTHOR_DATE=\".strftime(\"+0000 %Y-%m-%d %H:%M:%S\",gmtime($date)),\n-\t\t\t\"GIT_COMMITTER_NAME=$author_name\",\n-\t\t\t\"GIT_COMMITTER_EMAIL=$author_email\",\n-\t\t\t\"GIT_COMMITTER_DATE=\".strftime(\"+0000 %Y-%m-%d %H:%M:%S\",gmtime($date)),\n-\t\t\t\"git-commit-tree\", $tree,@par);\n-\t\tdie \"Cannot exec git-commit-tree: $!\\n\";\n-\n-\t\tclose OUT;\n \t}\n-\t$pw->writer();\n-\t$pr->reader();\n+\n+\tmy $commit_date = strftime(\"+0000 %Y-%m-%d %H:%M:%S\",gmtime($date));\n+\tmy $pid = open2(my $commit_read, my $commit_write,\n+\t\t'env',\n+\t\t\"GIT_AUTHOR_NAME=$author_name\",\n+\t\t\"GIT_AUTHOR_EMAIL=$author_email\",\n+\t\t\"GIT_AUTHOR_DATE=$commit_date\",\n+\t\t\"GIT_COMMITTER_NAME=$author_name\",\n+\t\t\"GIT_COMMITTER_EMAIL=$author_email\",\n+\t\t\"GIT_COMMITTER_DATE=$commit_date\",\n+\t\t'git-commit-tree', $tree, @commit_args);\n \n \t# compatibility with git2cvs\n \tsubstr($logmsg,32767) = \"\" if length($logmsg) > 32767;\n@@ -656,16 +636,14 @@ sub commit {\n \t    @skipped = ();\n \t}\n \n-\tprint $pw \"$logmsg\\n\"\n+\tprint($commit_write \"$logmsg\\n\") && close($commit_write)\n \t\tor die \"Error writing to git-commit-tree: $!\\n\";\n-\t$pw->close();\n \n-\tprint \"Committed patch $patchset ($branch \".strftime(\"%Y-%m-%d %H:%M:%S\",gmtime($date)).\")\\n\" if $opt_v;\n-\tchomp(my $cid = <$pr>);\n-\tlength($cid) == 40\n-\t\tor die \"Cannot get commit id ($cid): $!\\n\";\n+\tprint \"Committed patch $patchset ($branch $commit_date)\\n\" if $opt_v;\n+\tchomp(my $cid = <$commit_read>);\n+\tis_sha1($cid) or die \"Cannot get commit id ($cid): $!\\n\";\n \tprint \"Commit ID $cid\\n\" if $opt_v;\n-\t$pr->close();\n+\tclose($commit_read);\n \n \twaitpid($pid,0);\n \tdie \"Error running git-commit-tree: $?\\n\" if $?;\n-- \n1.3.3.g3408\n"}]}