{"thread":{"id":"43495","subject":"[RFC/PATCH] git-svn: convert to using Git.pm","startedAt":"2006-12-15T18:59:54Z","lastAt":"2006-12-15T19:47:58Z","messageCount":2,"participants":["Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"295123","messageId":"11662091941543-git-send-email-normalperson@yhbt.net","threadId":"43495","inReplyTo":null,"subject":"[RFC/PATCH] git-svn: convert to using Git.pm","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2006-12-15T18:59:54Z","receivedAt":"2006-12-15T18:59:54Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thanks to Git.pm, I've been able to greatly reduce the amount\nof extra work that needs to be done to manage input/output\npipes in Perl.\n\nchomp usage has also been greatly reduced, too.\n\nAll tests (including full-svn-test) still pass, but this has\nnot been tested extensively in the real-world.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n\nJunio: please don't apply this to master just yet.\n\n git-svn.perl |  328 ++++++++++++++++++++++++----------------------------------\n 1 files changed, 135 insertions(+), 193 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 73ab8d8..f453c9a 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -46,6 +46,8 @@ use File::Copy qw/copy/;\n use POSIX qw/strftime/;\n use IPC::Open3;\n use Memoize;\n+use Git qw/command command_oneline command_noisy\n+           command_output_pipe command_input_pipe command_close_pipe/;\n memoize('revisions_eq');\n memoize('cmt_metadata');\n memoize('get_commit_time');\n@@ -232,28 +234,27 @@ sub version {\n }\n \n sub rebuild {\n-\tif (quiet_run(qw/git-rev-parse --verify/,\"refs/remotes/$GIT_SVN^0\")) {\n+\tif (!verify_ref(\"refs/remotes/$GIT_SVN^0\")) {\n \t\tcopy_remote_ref();\n \t}\n \t$SVN_URL = shift or undef;\n \tmy $newest_rev = 0;\n \tif ($_upgrade) {\n-\t\tsys('git-update-ref',\"refs/remotes/$GIT_SVN\",\"$GIT_SVN-HEAD\");\n+\t\tcommand_noisy('update-ref',\"refs/remotes/$GIT_SVN\",\"\n+\t\t              $GIT_SVN-HEAD\");\n \t} else {\n \t\tcheck_upgrade_needed();\n \t}\n \n-\tmy $pid = open(my $rev_list,'-|');\n-\tdefined $pid or croak $!;\n-\tif ($pid == 0) {\n-\t\texec(\"git-rev-list\",\"refs/remotes/$GIT_SVN\") or croak $!;\n-\t}\n+\tmy ($rev_list, $ctx) = command_output_pipe(\"rev-list\",\n+\t                                           \"refs/remotes/$GIT_SVN\");\n \tmy $latest;\n \twhile (<$rev_list>) {\n \t\tchomp;\n \t\tmy $c = $_;\n \t\tcroak \"Non-SHA1: $c\\n\" unless $c =~ /^$sha1$/o;\n-\t\tmy @commit = grep(/^git-svn-id: /,`git-cat-file commit $c`);\n+\t\tmy @commit = grep(/^git-svn-id: /,\n+\t\t                  command(qw/cat-file commit/, $c));\n \t\tnext if (!@commit); # skip merges\n \t\tmy ($url, $rev, $uuid) = extract_metadata($commit[$#commit]);\n \t\tif (!defined $rev || !$uuid) {\n@@ -279,7 +280,7 @@ sub rebuild {\n \t\tprint \"r$rev = $c\\n\";\n \t\t$newest_rev = $rev if ($rev > $newest_rev);\n \t}\n-\tclose $rev_list or croak $?;\n+\tcommand_close_pipe($rev_list, $ctx);\n \n \tgoto out if $_use_lib;\n \tif (!chdir $SVN_WC) {\n@@ -287,7 +288,7 @@ sub rebuild {\n \t\tchdir $SVN_WC or croak $!;\n \t}\n \n-\t$pid = fork;\n+\tmy $pid = fork;\n \tdefined $pid or croak $!;\n \tif ($pid == 0) {\n \t\tmy @svn_up = qw(svn up);\n@@ -323,10 +324,10 @@ sub init {\n \n \t$SVN_URL = $url;\n \tunless (-d $GIT_DIR) {\n-\t\tmy @init_db = ('git-init-db');\n+\t\tmy @init_db = ('init-db');\n \t\tpush @init_db, \"--template=$_template\" if defined $_template;\n \t\tpush @init_db, \"--shared\" if defined $_shared;\n-\t\tsys(@init_db);\n+\t\tcommand_noisy(@init_db);\n \t}\n \tsetup_git_svn();\n }\n@@ -335,9 +336,8 @@ sub fetch {\n \tcheck_upgrade_needed();\n \t$SVN_URL ||= file_to_s(\"$GIT_SVN_DIR/info/url\");\n \tmy $ret = $_use_lib ? fetch_lib(@_) : fetch_cmd(@_);\n-\tif ($ret->{commit} && quiet_run(qw(git-rev-parse --verify\n-\t\t\t\t\t\trefs/heads/master^0))) {\n-\t\tsys(qw(git-update-ref refs/heads/master),$ret->{commit});\n+\tif ($ret->{commit} && !verify_ref('refs/heads/master^0')) {\n+\t\tcommand_noisy(qw(update-ref refs/heads/master),$ret->{commit});\n \t}\n \treturn $ret;\n }\n@@ -416,16 +416,16 @@ sub fetch_lib {\n \tread_uuid();\n \tif (defined $last_commit) {\n \t\tunless (-e $GIT_SVN_INDEX) {\n-\t\t\tsys(qw/git-read-tree/, $last_commit);\n+\t\t\tcommand_noisy('read-tree', $last_commit);\n \t\t}\n-\t\tchomp (my $x = `git-write-tree`);\n-\t\tmy ($y) = (`git-cat-file commit $last_commit`\n+\t\tmy $x = command_oneline('write-tree');\n+\t\tmy ($y) = (command(qw/cat-file commit/, $last_commit)\n \t\t\t\t\t\t\t=~ /^tree ($sha1)/m);\n \t\tif ($y ne $x) {\n \t\t\tunlink $GIT_SVN_INDEX or croak $!;\n-\t\t\tsys(qw/git-read-tree/, $last_commit);\n+\t\t\tcommand_noisy('read-tree', $last_commit);\n \t\t}\n-\t\tchomp ($x = `git-write-tree`);\n+\t\t$x = command_oneline('write-tree');\n \t\tif ($y ne $x) {\n \t\t\tprint STDERR \"trees ($last_commit) $y != $x\\n\",\n \t\t\t\t \"Something is seriously wrong...\\n\";\n@@ -489,16 +489,15 @@ sub commit {\n \t}\n \tmy @revs;\n \tforeach my $c (@commits) {\n-\t\tchomp(my @tmp = safe_qx('git-rev-parse',$c));\n+\t\tmy @tmp = command('rev-parse',$c);\n \t\tif (scalar @tmp == 1) {\n \t\t\tpush @revs, $tmp[0];\n \t\t} elsif (scalar @tmp > 1) {\n-\t\t\tpush @revs, reverse (safe_qx('git-rev-list',@tmp));\n+\t\t\tpush @revs, reverse(command('rev-list',@tmp));\n \t\t} else {\n \t\t\tdie \"Failed to rev-parse $c\\n\";\n \t\t}\n \t}\n-\tchomp @revs;\n \t$_use_lib ? commit_lib(@revs) : commit_cmd(@revs);\n \tprint \"Done committing \",scalar @revs,\" revisions to SVN\\n\";\n }\n@@ -606,10 +605,10 @@ sub commit_lib {\n sub dcommit {\n \tmy $head = shift || 'HEAD';\n \tmy $gs = \"refs/remotes/$GIT_SVN\";\n-\tchomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, \"$gs..$head\"));\n+\tmy @refs = command(qw/rev-list --no-merges/, \"$gs..$head\");\n \tmy $last_rev;\n \tforeach my $d (reverse @refs) {\n-\t\tif (quiet_run('git-rev-parse','--verify',\"$d~1\") != 0) {\n+\t\tif (!verify_ref(\"$d~1\")) {\n \t\t\tdie \"Commit $d\\n\",\n \t\t\t    \"has no parent commit, and therefore \",\n \t\t\t    \"nothing to diff against.\\n\",\n@@ -633,7 +632,7 @@ sub dcommit {\n \t}\n \treturn if $_dry_run;\n \tfetch();\n-\tmy @diff = safe_qx('git-diff-tree', $head, $gs);\n+\tmy @diff = command('diff-tree', $head, $gs, '--');\n \tmy @finish;\n \tif (@diff) {\n \t\t@finish = qw/rebase/;\n@@ -645,7 +644,7 @@ sub dcommit {\n \t\t      \"Resetting to the latest $gs\\n\";\n \t\t@finish = qw/reset --mixed/;\n \t}\n-\tsys('git', @finish, $gs);\n+\tcommand_noisy(@finish, $gs);\n }\n \n sub show_ignore {\n@@ -689,7 +688,7 @@ sub graft_branches {\n \n \tif (%$grafts) {\n \t\t# temporarily disable our grafts file to make this idempotent\n-\t\tchomp($gr_sha1 = safe_qx(qw/git-hash-object -w/,$gr_file));\n+\t\tchomp($gr_sha1 = command(qw/hash-object -w/,$gr_file));\n \t\trename $gr_file, \"$gr_file~$gr_sha1\" or croak $!;\n \t}\n \n@@ -743,7 +742,7 @@ sub multi_init {\n \tunless (-d $GIT_SVN_DIR) {\n \t\tprint \"GIT_SVN_ID set to 'trunk' for $_trunk\\n\" if $ch_id;\n \t\tinit($_trunk);\n-\t\tsys('git-repo-config', 'svn.trunk', $_trunk);\n+\t\tcommand_noisy('repo-config', 'svn.trunk', $_trunk);\n \t}\n \tcomplete_url_ls_init($url, $_branches, '--branches/-b', '');\n \tcomplete_url_ls_init($url, $_tags, '--tags/-t', 'tags/');\n@@ -781,11 +780,8 @@ sub show_log {\n \t}\n \n \tconfig_pager();\n-\tmy $pid = open(my $log,'-|');\n-\tdefined $pid or croak $!;\n-\tif (!$pid) {\n-\t\texec(git_svn_log_cmd($r_min,$r_max), @args) or croak $!;\n-\t}\n+\t@args = (git_svn_log_cmd($r_min, $r_max), @args);\n+\tmy $log = command_output_pipe(@args);\n \trun_pager();\n \tmy (@k, $c, $d);\n \n@@ -832,7 +828,7 @@ sub show_log {\n \t\tprocess_commit($_, $r_min, $r_max) foreach reverse @k;\n \t}\n out:\n-\tclose $log;\n+\teval { command_close_pipe($log) };\n \tprint '-' x72,\"\\n\" unless $_incremental || $_oneline;\n }\n \n@@ -912,7 +908,7 @@ sub cmt_showable {\n \treturn 1 if defined $c->{r};\n \tif ($c->{l} && $c->{l}->[-1] eq \"...\\n\" &&\n \t\t\t\t$c->{a_raw} =~ /\\@([a-f\\d\\-]+)>$/) {\n-\t\tmy @msg = safe_qx(qw/git-cat-file commit/, $c->{c});\n+\t\tmy @msg = command(qw/cat-file commit/, $c->{c});\n \t\tshift @msg while ($msg[0] ne \"\\n\");\n \t\tshift @msg;\n \t\t@{$c->{l}} = grep !/^git-svn-id: /, @msg;\n@@ -961,7 +957,7 @@ sub log_use_color {\n \n sub git_svn_log_cmd {\n \tmy ($r_min, $r_max) = @_;\n-\tmy @cmd = (qw/git-log --abbrev-commit --pretty=raw\n+\tmy @cmd = (qw/log --abbrev-commit --pretty=raw\n \t\t\t--default/, \"refs/remotes/$GIT_SVN\");\n \tpush @cmd, '-r' unless $_non_recursive;\n \tpush @cmd, qw/--raw --name-status/ if $_verbose;\n@@ -1071,7 +1067,7 @@ sub complete_url_ls_init {\n \twaitpid $pid, 0;\n \tcroak $? if $?;\n \tmy ($n) = ($switch =~ /^--(\\w+)/);\n-\tsys('git-repo-config', \"svn.$n\", $var);\n+\tcommand_noisy('repo-config', \"svn.$n\", $var);\n }\n \n sub common_prefix {\n@@ -1103,11 +1099,8 @@ sub graft_tree_joins {\n \n \tgit_svn_each(sub {\n \t\tmy $i = shift;\n-\t\tdefined(my $pid = open my $fh, '-|') or croak $!;\n-\t\tif (!$pid) {\n-\t\t\texec qw/git-rev-list --pretty=raw/,\n-\t\t\t\t\t\"refs/remotes/$i\" or croak $!;\n-\t\t}\n+\t\tmy @args = (qw/rev-list --pretty=raw/, \"refs/remotes/$i\");\n+\t\tmy ($fh, $ctx) = command_output_pipe(@args);\n \t\twhile (<$fh>) {\n \t\t\tnext unless /^commit ($sha1)$/o;\n \t\t\tmy $c = $1;\n@@ -1130,9 +1123,7 @@ sub graft_tree_joins {\n \n \t\t\tforeach my $p (@{$tree_map{$t}}) {\n \t\t\t\tnext if $p eq $c;\n-\t\t\t\tmy $mb = eval {\n-\t\t\t\t\tsafe_qx('git-merge-base', $c, $p)\n-\t\t\t\t};\n+\t\t\t\tmy $mb = eval { command('merge-base', $c, $p) };\n \t\t\t\tnext unless ($@ || $?);\n \t\t\t\tif (defined $r_a) {\n \t\t\t\t\t# see if SVN says it's a relative\n@@ -1161,7 +1152,7 @@ sub graft_tree_joins {\n \t\t\t\t# what should we do when $ct == $s ?\n \t\t\t}\n \t\t}\n-\t\tclose $fh or croak $?;\n+\t\tcommand_close_pipe($fh, $ctx);\n \t});\n }\n \n@@ -1297,6 +1288,11 @@ sub read_uuid {\n \t}\n }\n \n+sub verify_ref {\n+\tmy ($ref) = @_;\n+\teval { command_oneline([ 'rev-parse', $ref ], { STDERR => 0 }) };\n+}\n+\n sub quiet_run {\n \tmy $pid = fork;\n \tdefined $pid or croak $!;\n@@ -1379,13 +1375,14 @@ sub assert_svn_wc_clean {\n sub get_tree_from_treeish {\n \tmy ($treeish) = @_;\n \tcroak \"Not a sha1: $treeish\\n\" unless $treeish =~ /^$sha1$/o;\n-\tchomp(my $type = `git-cat-file -t $treeish`);\n+\tmy $type = command_oneline(qw/cat-file -t/, $treeish);\n \tmy $expected;\n \twhile ($type eq 'tag') {\n-\t\tchomp(($treeish, $type) = `git-cat-file tag $treeish`);\n+\t\t($treeish, $type) = command(qw/cat-file tag/, $treeish);\n \t}\n \tif ($type eq 'commit') {\n-\t\t$expected = (grep /^tree /,`git-cat-file commit $treeish`)[0];\n+\t\t$expected = (grep /^tree /, command(qw/cat-file commit/,\n+\t\t                                    $treeish))[0];\n \t\t($expected) = ($expected =~ /^tree ($sha1)$/);\n \t\tdie \"Unable to get tree from $treeish\\n\" unless $expected;\n \t} elsif ($type eq 'tree') {\n@@ -1407,7 +1404,7 @@ sub assert_tree {\n \t}\n \tmy $old_index = set_index($tmpindex);\n \tindex_changes(1);\n-\tchomp(my $tree = `git-write-tree`);\n+\tmy $tree = command_oneline('write-tree');\n \trestore_index($old_index);\n \tif ($tree ne $expected) {\n \t\tcroak \"Tree mismatch, Got: $tree, Expected: $expected\\n\";\n@@ -1415,8 +1412,20 @@ sub assert_tree {\n \tunlink $tmpindex;\n }\n \n-sub parse_diff_tree {\n-\tmy $diff_fh = shift;\n+sub get_diff {\n+\tmy ($from, $treeish) = @_;\n+\tassert_tree($from);\n+\tprint \"diff-tree $from $treeish\\n\";\n+\tmy @diff_tree = qw(diff-tree -z -r);\n+\tif ($_cp_similarity) {\n+\t\tpush @diff_tree, \"-C$_cp_similarity\";\n+\t} else {\n+\t\tpush @diff_tree, '-C';\n+\t}\n+\tpush @diff_tree, '--find-copies-harder' if $_find_copies_harder;\n+\tpush @diff_tree, \"-l$_l\" if defined $_l;\n+\tpush @diff_tree, $from, $treeish;\n+\tmy ($diff_fh, $ctx) = command_output_pipe(@diff_tree);\n \tlocal $/ = \"\\0\";\n \tmy $state = 'meta';\n \tmy @mods;\n@@ -1452,8 +1461,7 @@ sub parse_diff_tree {\n \t\t\tcroak \"Error parsing $_\\n\";\n \t\t}\n \t}\n-\tclose $diff_fh or croak $?;\n-\n+\tcommand_close_pipe($diff_fh, $ctx);\n \treturn \\@mods;\n }\n \n@@ -1547,26 +1555,6 @@ sub precommit_check {\n }\n \n \n-sub get_diff {\n-\tmy ($from, $treeish) = @_;\n-\tassert_tree($from);\n-\tprint \"diff-tree $from $treeish\\n\";\n-\tmy $pid = open my $diff_fh, '-|';\n-\tdefined $pid or croak $!;\n-\tif ($pid == 0) {\n-\t\tmy @diff_tree = qw(git-diff-tree -z -r);\n-\t\tif ($_cp_similarity) {\n-\t\t\tpush @diff_tree, \"-C$_cp_similarity\";\n-\t\t} else {\n-\t\t\tpush @diff_tree, '-C';\n-\t\t}\n-\t\tpush @diff_tree, '--find-copies-harder' if $_find_copies_harder;\n-\t\tpush @diff_tree, \"-l$_l\" if defined $_l;\n-\t\texec(@diff_tree, $from, $treeish) or croak $!;\n-\t}\n-\treturn parse_diff_tree($diff_fh);\n-}\n-\n sub svn_checkout_tree {\n \tmy ($from, $treeish) = @_;\n \tmy $mods = get_diff($from->{commit}, $treeish);\n@@ -1676,14 +1664,10 @@ sub get_commit_message {\n \tmy %log_msg = ( msg => '' );\n \topen my $msg, '>', $commit_msg or croak $!;\n \n-\tchomp(my $type = `git-cat-file -t $commit`);\n+\tmy $type = command_oneline(qw/cat-file -t/, $commit);\n \tif ($type eq 'commit' || $type eq 'tag') {\n-\t\tmy $pid = open my $msg_fh, '-|';\n-\t\tdefined $pid or croak $!;\n-\n-\t\tif ($pid == 0) {\n-\t\t\texec('git-cat-file', $type, $commit) or croak $!;\n-\t\t}\n+\t\tmy ($msg_fh, $ctx) = command_output_pipe('cat-file',\n+\t\t                                         $type, $commit);\n \t\tmy $in_msg = 0;\n \t\twhile (<$msg_fh>) {\n \t\t\tif (!$in_msg) {\n@@ -1695,7 +1679,7 @@ sub get_commit_message {\n \t\t\t\tprint $msg $_ or croak $!;\n \t\t\t}\n \t\t}\n-\t\tclose $msg_fh or croak $?;\n+\t\tcommand_close_pipe($msg_fh, $ctx);\n \t}\n \tclose $msg or croak $!;\n \n@@ -1771,13 +1755,8 @@ sub svn_commit_tree {\n }\n \n sub rev_list_raw {\n-\tmy (@args) = @_;\n-\tmy $pid = open my $fh, '-|';\n-\tdefined $pid or croak $!;\n-\tif (!$pid) {\n-\t\texec(qw/git-rev-list --pretty=raw/, @args) or croak $!;\n-\t}\n-\treturn { fh => $fh, t => { } };\n+\tmy ($fh, $c) = command_output_pipe(qw/rev-list --pretty=raw/, @_);\n+\treturn { fh => $fh, ctx => $c, t => { } };\n }\n \n sub next_rev_list_entry {\n@@ -1799,6 +1778,7 @@ sub next_rev_list_entry {\n \t\t\t$x->{m} .= $_;\n \t\t}\n \t}\n+\tcommand_close_pipe($fh, $rl->{ctx});\n \treturn ($x != $rl->{t}) ? $x : undef;\n }\n \n@@ -1924,15 +1904,10 @@ sub sys { system(@_) == 0 or croak $? }\n sub do_update_index {\n \tmy ($z_cmd, $cmd, $no_text_base) = @_;\n \n-\tmy $z = open my $p, '-|';\n-\tdefined $z or croak $!;\n-\tunless ($z) { exec @$z_cmd or croak $! }\n+\tmy ($p, $pctx) = command_output_pipe(@$z_cmd);\n \n-\tmy $pid = open my $ui, '|-';\n-\tdefined $pid or croak $!;\n-\tunless ($pid) {\n-\t\texec('git-update-index',\"--$cmd\",'-z','--stdin') or croak $!;\n-\t}\n+\tmy ($ui, $uctx) = command_input_pipe('update-index',\n+\t                                     \"--$cmd\",'-z','--stdin');\n \tlocal $/ = \"\\0\";\n \twhile (my $x = <$p>) {\n \t\tchomp $x;\n@@ -1956,7 +1931,8 @@ sub do_update_index {\n \t\t}\n \t\tprint $ui $x,\"\\0\";\n \t}\n-\tclose $ui or croak $?;\n+\tcommand_close_pipe($p, $pctx);\n+\tcommand_close_pipe($ui, $uctx);\n }\n \n sub index_changes {\n@@ -1968,10 +1944,10 @@ sub index_changes {\n \t\tclose $fd or croak $!;\n \t}\n \tmy $no_text_base = shift;\n-\tdo_update_index([qw/git-diff-files --name-only -z/],\n+\tdo_update_index([qw/diff-files --name-only -z/],\n \t\t\t'remove',\n \t\t\t$no_text_base);\n-\tdo_update_index([qw/git-ls-files -z --others/,\n+\tdo_update_index([qw/ls-files -z --others/,\n \t\t\t\t\"--exclude-from=$GIT_SVN_DIR/info/exclude\"],\n \t\t\t'add',\n \t\t\t$no_text_base);\n@@ -2004,10 +1980,10 @@ sub assert_revision_unknown {\n \n sub trees_eq {\n \tmy ($x, $y) = @_;\n-\tmy @x = safe_qx('git-cat-file','commit',$x);\n-\tmy @y = safe_qx('git-cat-file','commit',$y);\n-\tif (($y[0] ne $x[0]) || $x[0] !~ /^tree $sha1\\n$/\n-\t\t\t\t|| $y[0] !~ /^tree $sha1\\n$/) {\n+\tmy @x = command(qw/cat-file commit/,$x);\n+\tmy @y = command(qw/cat-file commit/,$y);\n+\tif (($y[0] ne $x[0]) || $x[0] ne \"tree $sha1\"\n+\t\t             || $y[0] ne \"tree $sha1\") {\n \t\tprint STDERR \"Trees not equal: $y[0] != $x[0]\\n\";\n \t\treturn 0\n \t}\n@@ -2039,33 +2015,24 @@ sub git_commit {\n \tif (!defined $tree) {\n \t\tmy $index = set_index($GIT_SVN_INDEX);\n \t\tindex_changes();\n-\t\tchomp($tree = `git-write-tree`);\n+\t\t$tree = command_oneline('write-tree');\n \t\tcroak $? if $?;\n \t\trestore_index($index);\n \t}\n \n \t# just in case we clobber the existing ref, we still want that ref\n \t# as our parent:\n-\topen my $null, '>', '/dev/null' or croak $!;\n-\topen my $stderr, '>&', \\*STDERR or croak $!;\n-\topen STDERR, '>&', $null or croak $!;\n-\tif (my $cur = eval { safe_qx('git-rev-parse',\n-\t                             \"refs/remotes/$GIT_SVN^0\") }) {\n+\tif (my $cur = verify_ref(\"refs/remotes/$GIT_SVN^0\")) {\n \t\tchomp $cur;\n \t\tpush @tmp_parents, $cur;\n \t}\n-\topen STDERR, '>&', $stderr or croak $!;\n-\tclose $stderr or croak $!;\n-\tclose $null or croak $!;\n \n \tif (exists $tree_map{$tree}) {\n \t\tforeach my $p (@{$tree_map{$tree}}) {\n \t\t\tmy $skip;\n \t\t\tforeach (@tmp_parents) {\n \t\t\t\t# see if a common parent is found\n-\t\t\t\tmy $mb = eval {\n-\t\t\t\t\tsafe_qx('git-merge-base', $_, $p)\n-\t\t\t\t};\n+\t\t\t\tmy $mb = eval { command('merge-base', $_, $p) };\n \t\t\t\tnext if ($@ || $?);\n \t\t\t\t$skip = 1;\n \t\t\t\tlast;\n@@ -2107,7 +2074,7 @@ sub git_commit {\n \tif ($commit !~ /^$sha1$/o) {\n \t\tdie \"Failed to commit, invalid sha1: $commit\\n\";\n \t}\n-\tsys('git-update-ref',\"refs/remotes/$GIT_SVN\",$commit);\n+\tcommand_noisy('update-ref',\"refs/remotes/$GIT_SVN\",$commit);\n \trevdb_set($REVDB, $log_msg->{revision}, $commit);\n \n \t# this output is read via pipe, do not change:\n@@ -2119,7 +2086,8 @@ sub git_commit {\n sub check_repack {\n \tif ($_repack && (--$_repack_nr == 0)) {\n \t\t$_repack_nr = $_repack;\n-\t\tsys(\"git repack $_repack_flags\");\n+\t\t# repack doesn't use any arguments with spaces in them, does it?\n+\t\tcommand_noisy('repack', split(/\\s+/, $_repack_flags));\n \t}\n }\n \n@@ -2238,20 +2206,11 @@ sub check_upgrade_needed {\n \t\topen my $fh, '>>',$REVDB or croak $!;\n \t\tclose $fh;\n \t}\n-\tmy $old = eval {\n-\t\tmy $pid = open my $child, '-|';\n-\t\tdefined $pid or croak $!;\n-\t\tif ($pid == 0) {\n-\t\t\tclose STDERR;\n-\t\t\texec('git-rev-parse',\"$GIT_SVN-HEAD\") or croak $!;\n-\t\t}\n-\t\tmy @ret = (<$child>);\n-\t\tclose $child or croak $?;\n-\t\tdie $? if $?; # just in case close didn't error out\n-\t\treturn wantarray ? @ret : join('',@ret);\n+\treturn unless eval {\n+\t\tcommand([qw/rev-parse --verify/,\"$GIT_SVN-HEAD^0\"],\n+\t\t        {STDERR => 0});\n \t};\n-\treturn unless $old;\n-\tmy $head = eval { safe_qx('git-rev-parse',\"refs/remotes/$GIT_SVN\") };\n+\tmy $head = eval { command('rev-parse',\"refs/remotes/$GIT_SVN\") };\n \tif ($@ || !$head) {\n \t\tprint STDERR \"Please run: $0 rebuild --upgrade\\n\";\n \t\texit 1;\n@@ -2263,12 +2222,8 @@ sub check_upgrade_needed {\n sub map_tree_joins {\n \tmy %seen;\n \tforeach my $br (@_branch_from) {\n-\t\tmy $pid = open my $pipe, '-|';\n-\t\tdefined $pid or croak $!;\n-\t\tif ($pid == 0) {\n-\t\t\texec(qw(git-rev-list --topo-order --pretty=raw), $br)\n-\t\t\t\t\t\t\t\tor croak $!;\n-\t\t}\n+\t\tmy $pipe = command_output_pipe(qw/rev-list\n+\t\t                            --topo-order --pretty=raw/, $br);\n \t\twhile (<$pipe>) {\n \t\t\tif (/^commit ($sha1)$/o) {\n \t\t\t\tmy $commit = $1;\n@@ -2284,7 +2239,7 @@ sub map_tree_joins {\n \t\t\t\t$seen{$commit} = 1;\n \t\t\t}\n \t\t}\n-\t\tclose $pipe; # we could be breaking the pipe early\n+\t\teval { command_close_pipe($pipe) };\n \t}\n }\n \n@@ -2296,7 +2251,7 @@ sub load_all_refs {\n \n \t# don't worry about rev-list on non-commit objects/tags,\n \t# it shouldn't blow up if a ref is a blob or tree...\n-\tchomp(@_branch_from = `git-rev-parse --symbolic --all`);\n+\t@_branch_from = command(qw/rev-parse --symbolic --all/);\n }\n \n # '<svn username> = real-name <email address>' mapping based on git-svnimport:\n@@ -2330,7 +2285,7 @@ sub svn_propget_base {\n \n sub git_svn_each {\n \tmy $sub = shift;\n-\tforeach (`git-rev-parse --symbolic --all`) {\n+\tforeach (command(qw/rev-parse --symbolic --all/)) {\n \t\tnext unless s#^refs/remotes/##;\n \t\tchomp $_;\n \t\tnext unless -f \"$GIT_DIR/svn/$_/info/url\";\n@@ -2371,7 +2326,7 @@ sub migration_check {\n \t\t\t\t\"$GIT_SVN_DIR\\n\\t(required for this version \",\n \t\t\t\t\"($VERSION) of git-svn) does not.\\n\";\n \n-\tforeach my $x (`git-rev-parse --symbolic --all`) {\n+\tforeach my $x (command(qw/rev-parse --symbolic --all/)) {\n \t\tnext unless $x =~ s#^refs/remotes/##;\n \t\tchomp $x;\n \t\tnext unless -f \"$GIT_DIR/$x/info/url\";\n@@ -2476,11 +2431,7 @@ sub write_grafts {\n \t\tmy $p = $grafts->{$c};\n \t\tmy %x; # real parents\n \t\tdelete $p->{$c}; # commits are not self-reproducing...\n-\t\tmy $pid = open my $ch, '-|';\n-\t\tdefined $pid or croak $!;\n-\t\tif (!$pid) {\n-\t\t\texec(qw/git-cat-file commit/, $c) or croak $!;\n-\t\t}\n+\t\tmy $ch = command_output_pipe(qw/cat-file commit/, $c);\n \t\twhile (<$ch>) {\n \t\t\tif (/^parent ($sha1)/) {\n \t\t\t\t$x{$1} = $p->{$1} = 1;\n@@ -2488,7 +2439,7 @@ sub write_grafts {\n \t\t\t\tlast unless /^\\S/;\n \t\t\t}\n \t\t}\n-\t\tclose $ch; # breaking the pipe\n+\t\teval { command_close_pipe($ch) }; # breaking the pipe\n \n \t\t# if real parents are the only ones in the grafts, drop it\n \t\tnext if join(' ',sort keys %$p) eq join(' ',sort keys %x);\n@@ -2500,7 +2451,7 @@ sub write_grafts {\n \t\t\tnext if $del{$i} || $p->{$i} == 2;\n \t\t\tforeach my $j (@jp) {\n \t\t\t\tnext if $i eq $j || $del{$j} || $p->{$j} == 2;\n-\t\t\t\t$mb = eval { safe_qx('git-merge-base',$i,$j) };\n+\t\t\t\t$mb = eval { command('merge-base', $i, $j) };\n \t\t\t\tnext unless $mb;\n \t\t\t\tchomp $mb;\n \t\t\t\tnext if $x{$mb};\n@@ -2571,15 +2522,12 @@ sub extract_metadata {\n \n sub cmt_metadata {\n \treturn extract_metadata((grep(/^git-svn-id: /,\n-\t\tsafe_qx(qw/git-cat-file commit/, shift)))[-1]);\n+\t\tcommand(qw/cat-file commit/, shift)))[-1]);\n }\n \n sub get_commit_time {\n \tmy $cmt = shift;\n-\tdefined(my $pid = open my $fh, '-|') or croak $!;\n-\tif (!$pid) {\n-\t\texec qw/git-rev-list --pretty=raw -n1/, $cmt or croak $!;\n-\t}\n+\tmy $fh = command_output_pipe(qw/rev-list --pretty=raw -n1/, $cmt);\n \twhile (<$fh>) {\n \t\t/^committer\\s(?:.+) (\\d+) ([\\-\\+]?\\d+)$/ or next;\n \t\tmy ($s, $tz) = ($1, $2);\n@@ -2588,7 +2536,7 @@ sub get_commit_time {\n \t\t} elsif ($tz =~ s/^\\-//) {\n \t\t\t$s -= tz_to_s_offset($tz);\n \t\t}\n-\t\tclose $fh;\n+\t\teval { command_close_pipe($fh) };\n \t\treturn $s;\n \t}\n \tdie \"Can't get commit time for commit: $cmt\\n\";\n@@ -2748,7 +2696,6 @@ sub libsvn_load {\n \t\tpush @SVN::Git::Editor::ISA, 'SVN::Delta::Editor';\n \t\tpush @SVN::Git::Fetcher::ISA, 'SVN::Delta::Editor';\n \t\t*SVN::Git::Fetcher::process_rm = *process_rm;\n-\t\t*SVN::Git::Fetcher::safe_qx = *safe_qx;\n \t\tmy $kill_stupid_warnings = $SVN::Node::none.$SVN::Node::file.\n \t\t\t\t\t$SVN::Node::dir.$SVN::Node::unknown.\n \t\t\t\t\t$SVN::Node::none.$SVN::Node::file.\n@@ -2965,7 +2912,7 @@ sub libsvn_get_file {\n \tmy $mode = exists $props->{'svn:executable'} ? '100755' : '100644';\n \tif (exists $props->{'svn:special'}) {\n \t\t$mode = '120000';\n-\t\tmy $link = `git-cat-file blob $hash`;\n+\t\tmy $link = `git-cat-file blob $hash`; # no chomping symlinks\n \t\t$link =~ s/^link // or die \"svn:special file with contents: <\",\n \t\t\t\t\t\t$link, \"> is not understood\\n\";\n \t\tdefined($pid = open3($in, $out, '>&STDERR',\n@@ -3069,19 +3016,17 @@ sub libsvn_log_entry {\n sub process_rm {\n \tmy ($gui, $last_commit, $f, $q) = @_;\n \t# remove entire directories.\n-\tif (safe_qx('git-ls-tree',$last_commit,'--',$f) =~ /^040000 tree/) {\n-\t\tdefined(my $pid = open my $ls, '-|') or croak $!;\n-\t\tif (!$pid) {\n-\t\t\texec(qw/git-ls-tree -r --name-only -z/,\n-\t\t\t\t$last_commit,'--',$f) or croak $!;\n-\t\t}\n+\tif (command('ls-tree',$last_commit,'--',$f) =~ /^040000 tree/) {\n+\t\tmy ($ls, $ctx) = command_output_pipe(qw/ls-tree\n+\t\t                                     -r --name-only -z/,\n+\t\t\t\t                     $last_commit,'--',$f);\n \t\tlocal $/ = \"\\0\";\n \t\twhile (<$ls>) {\n \t\t\tprint $gui '0 ',0 x 40,\"\\t\",$_ or croak $!;\n \t\t\tprint \"\\tD\\t$_\\n\" unless $q;\n \t\t}\n \t\tprint \"\\tD\\t$f/\\n\" unless $q;\n-\t\tclose $ls or croak $?;\n+\t\tcommand_close_pipe($ls, $ctx);\n \t\treturn $SVN::Node::dir;\n \t} else {\n \t\tprint $gui '0 ',0 x 40,\"\\t\",$f,\"\\0\" or croak $!;\n@@ -3112,7 +3057,7 @@ sub libsvn_fetch_delta {\n \n sub libsvn_fetch_full {\n \tmy ($last_commit, $paths, $rev, $author, $date, $msg) = @_;\n-\topen my $gui, '| git-update-index -z --index-info' or croak $!;\n+\tmy ($gui, $ctx) = command_input_pipe(qw/update-index -z --index-info/);\n \tmy %amr;\n \tmy $ut = { empty => {}, dir_prop => {}, file_prop => {} };\n \tmy $p = $SVN->{svn_path};\n@@ -3166,20 +3111,14 @@ sub libsvn_fetch_full {\n \t\t%{$ut->{dir_prop}->{''}} = %$props;\n \t\t$pool->clear;\n \t}\n-\tclose $gui or croak $?;\n+\tcommand_close_pipe($gui, $ctx);\n \tlibsvn_log_entry($rev, $author, $date, $msg, [$last_commit], $ut);\n }\n \n sub svn_grab_base_rev {\n-\tdefined(my $pid = open my $fh, '-|') or croak $!;\n-\tif (!$pid) {\n-\t\topen my $null, '>', '/dev/null' or croak $!;\n-\t\topen STDERR, '>&', $null or croak $!;\n-\t\texec qw/git-rev-parse --verify/,\"refs/remotes/$GIT_SVN^0\"\n-\t\t\t\t\t\t\t\tor croak $!;\n-\t}\n-\tchomp(my $c = do { local $/; <$fh> });\n-\tclose $fh;\n+\tmy $c = eval { command_oneline([qw/rev-parse --verify/,\n+\t                                \"refs/remotes/$GIT_SVN^0\"],\n+\t\t\t\t        { STDERR => 0 }) };\n \tif (defined $c && length $c) {\n \t\tmy ($url, $rev, $uuid) = cmt_metadata($c);\n \t\treturn ($rev, $c) if defined $rev;\n@@ -3358,7 +3297,7 @@ sub libsvn_find_parent_branch {\n \tif (revisions_eq($branch_from, $r0, $r)) {\n \t\tunlink $GIT_SVN_INDEX;\n \t\tprint STDERR \"Found branch parent: ($GIT_SVN) $parent\\n\";\n-\t\tsys(qw/git-read-tree/, $parent);\n+\t\tcommand_noisy('read-tree', $parent);\n \t\tunless (libsvn_can_do_switch()) {\n \t\t\treturn libsvn_fetch_full($parent, $paths, $rev,\n \t\t\t\t\t\t$author, $date, $msg);\n@@ -3367,7 +3306,7 @@ sub libsvn_find_parent_branch {\n \t\t# included with SVN 1.4.2 (the latest version at the moment),\n \t\t# so we can't rely on it.\n \t\tmy $ra = libsvn_connect(\"$url/$branch_from\");\n-\t\tmy $ed = SVN::Git::Fetcher->new({c => $parent, q => $_q});\n+\t\tmy $ed = SVN::Git::Fetcher->new({c => $parent, q => $_q });\n \t\tmy $pool = SVN::Pool->new;\n \t\tmy $reporter = $ra->do_switch($rev, '', 1, $SVN->{url},\n \t\t                              $ed, $pool);\n@@ -3413,9 +3352,10 @@ sub libsvn_new_tree {\n \t\t$ut = $ed;\n \t} else {\n \t\t$ut = { empty => {}, dir_prop => {}, file_prop => {} };\n-\t\topen my $gui, '| git-update-index -z --index-info' or croak $!;\n+\t        my ($gui, $ctx) = command_input_pipe(qw/update-index\n+\t                                             -z --index-info/);\n \t\tlibsvn_traverse($gui, '', $SVN->{svn_path}, $rev, undef, $ut);\n-\t\tclose $gui or croak $?;\n+\t\tcommand_close_pipe($gui, $ctx);\n \t}\n \tlibsvn_log_entry($rev, $author, $date, $msg, [], $ut);\n }\n@@ -3487,7 +3427,7 @@ sub libsvn_commit_cb {\n \t\tmy $log = libsvn_log_entry($rev,$committer,$date,$msg);\n \t\t$log->{tree} = get_tree_from_treeish($c);\n \t\tmy $cmt = git_commit($log, $cmt_last, $c);\n-\t\tmy @diff = safe_qx('git-diff-tree', $cmt, $c);\n+\t\tmy @diff = command('diff-tree', $cmt, $c);\n \t\tif (@diff) {\n \t\t\tprint STDERR \"Trees differ: $cmt $c\\n\",\n \t\t\t\t\tjoin('',@diff),\"\\n\";\n@@ -3579,8 +3519,8 @@ sub revdb_get {\n sub copy_remote_ref {\n \tmy $origin = $_cp_remote ? $_cp_remote : 'origin';\n \tmy $ref = \"refs/remotes/$GIT_SVN\";\n-\tif (safe_qx('git-ls-remote', $origin, $ref)) {\n-\t\tsys(qw/git fetch/, $origin, \"$ref:$ref\");\n+\tif (command('ls-remote', $origin, $ref)) {\n+\t\tcommand_noisy('fetch', $origin, \"$ref:$ref\");\n \t} elsif ($_cp_remote && !$_upgrade) {\n \t\tdie \"Unable to find remote reference: \",\n \t\t\t\t\"refs/remotes/$GIT_SVN on $origin\\n\";\n@@ -3592,14 +3532,14 @@ use strict;\n use warnings;\n use Carp qw/croak/;\n use IO::File qw//;\n+use Git qw/command command_oneline command_noisy\n+           command_output_pipe command_input_pipe command_close_pipe/;\n \n # file baton members: path, mode_a, mode_b, pool, fh, blob, base\n sub new {\n \tmy ($class, $git_svn) = @_;\n \tmy $self = SVN::Delta::Editor->new;\n \tbless $self, $class;\n-\topen my $gui, '| git-update-index -z --index-info' or croak $!;\n-\t$self->{gui} = $gui;\n \t$self->{c} = $git_svn->{c} if exists $git_svn->{c};\n \t$self->{q} = $git_svn->{q};\n \t$self->{empty} = {};\n@@ -3607,6 +3547,8 @@ sub new {\n \t$self->{file_prop} = {};\n \t$self->{absent_dir} = {};\n \t$self->{absent_file} = {};\n+\t($self->{gui}, $self->{ctx}) = command_input_pipe(\n+\t                                     qw/update-index -z --index-info/);\n \trequire Digest::MD5;\n \t$self;\n }\n@@ -3629,7 +3571,7 @@ sub delete_entry {\n \n sub open_file {\n \tmy ($self, $path, $pb, $rev) = @_;\n-\tmy ($mode, $blob) = (safe_qx('git-ls-tree',$self->{c},'--',$path)\n+\tmy ($mode, $blob) = (command('ls-tree', $self->{c}, '--',$path)\n \t                     =~ /^(\\d{6}) blob ([a-f\\d]{40})\\t/);\n \tunless (defined $mode && defined $blob) {\n \t\tdie \"$path was not found in commit $self->{c} (r$rev)\\n\";\n@@ -3764,13 +3706,13 @@ sub close_file {\n \n sub abort_edit {\n \tmy $self = shift;\n-\tclose $self->{gui};\n+\teval { command_close_pipe($self->{gui}, $self->{ctx}) };\n \t$self->SUPER::abort_edit(@_);\n }\n \n sub close_edit {\n \tmy $self = shift;\n-\tclose $self->{gui} or croak $!;\n+\tcommand_close_pipe($self->{gui}, $self->{ctx});\n \t$self->{git_commit_ok} = 1;\n \t$self->SUPER::close_edit(@_);\n }\n@@ -3781,6 +3723,8 @@ use strict;\n use warnings;\n use Carp qw/croak/;\n use IO::File;\n+use Git qw/command command_oneline command_noisy\n+           command_output_pipe command_input_pipe command_close_pipe/;\n \n sub new {\n \tmy $class = shift;\n@@ -3830,10 +3774,8 @@ sub rmdirs {\n \tdelete $rm->{''}; # we never delete the url we're tracking\n \treturn unless %$rm;\n \n-\tdefined(my $pid = open my $fh,'-|') or croak $!;\n-\tif (!$pid) {\n-\t\texec qw/git-ls-tree --name-only -r -z/, $self->{c} or croak $!;\n-\t}\n+\tmy ($fh, $ctx) = command_output_pipe(\n+\t                           qw/ls-tree --name-only -r -z/, $self->{c});\n \tlocal $/ = \"\\0\";\n \twhile (<$fh>) {\n \t\tchomp;\n@@ -3842,11 +3784,11 @@ sub rmdirs {\n \t\t\tdelete $rm->{join '/', @dn};\n \t\t}\n \t\tunless (%$rm) {\n-\t\t\tclose $fh;\n+\t\t\teval { command_close_pipe($fh) };\n \t\t\treturn;\n \t\t}\n \t}\n-\tclose $fh;\n+\tcommand_close_pipe($fh, $ctx);\n \n \tmy ($r, $p, $bat) = ($self->{r}, $self->{pool}, $self->{bat});\n \tforeach my $d (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$rm) {\n-- \n1.4.4.2.g0817\n"},{"id":"296573","messageId":"7virgdvt3l.fsf@assigned-by-dhcp.cox.net","threadId":"43495","inReplyTo":"11662091941543-git-send-email-normalperson@yhbt.net","subject":"Re: [RFC/PATCH] git-svn: convert to using Git.pm","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-15T19:47:58Z","receivedAt":"2006-12-15T19:47:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> Thanks to Git.pm, I've been able to greatly reduce the amount\n> of extra work that needs to be done to manage input/output\n> pipes in Perl.\n>\n> chomp usage has also been greatly reduced, too.\n>\n> All tests (including full-svn-test) still pass, but this has\n> not been tested extensively in the real-world.\n>\n> Signed-off-by: Eric Wong <normalperson@yhbt.net>\n> ---\n>\n> Junio: please don't apply this to master just yet.\n\nWill queue for 'next'.  Thanks for the warning.\n"}]}