git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 1/3] git-svn: enable logging of information not supported by git
@ 2006-12-12 22:47 Eric Wong
  2006-12-12 22:47 ` [PATCH 2/3] git-svn: allow dcommit to take an alternate head Eric Wong
  2006-12-12 22:47 ` [PATCH 3/3] git-svn: correctly display fatal() error messages Eric Wong
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Wong @ 2006-12-12 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong

The changes are now tracked in
  $GIT_DIR/svn/$GIT_SVN_ID/untracked.log

Information in the untracked.log include:
  * the addition and removal of empty directories
    (changes of these will also warn the user)
  * file and directory property changes, including (but not
    limited to) svk:merge and svn:externals
  * revision properties (revprops) are also tracked
  * users will be warned of 'absent' file and directories
    (if users are forbidden access)

Fields in entries are separated by spaces; "unsafe" characters
are URI-encoded so that each entry takes exactly one line.

There is currently no automated parser for dealing with the data
in untracked.log, but it should be possible to write one to
create empty directories on checkout and manage
externals/subprojects.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |  202 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 184 insertions(+), 18 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 1f8a3b0..06e89ff 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -21,6 +21,16 @@ $ENV{TZ} = 'UTC';
 $ENV{LC_ALL} = 'C';
 $| = 1; # unbuffer STDOUT
 
+# properties that we do not log:
+my %SKIP = ( 'svn:wc:ra_dav:version-url' => 1,
+             'svn:special' => 1,
+             'svn:executable' => 1,
+             'svn:entry:committed-rev' => 1,
+             'svn:entry:last-author' => 1,
+             'svn:entry:uuid' => 1,
+             'svn:entry:committed-date' => 1,
+);
+
 sub fatal (@) { print STDERR $@; exit 1 }
 # If SVN:: library support is added, please make the dependencies
 # optional and preserve the capability to use the command-line client.
@@ -2902,7 +2912,7 @@ sub libsvn_dup_ra {
 }
 
 sub libsvn_get_file {
-	my ($gui, $f, $rev, $chg) = @_;
+	my ($gui, $f, $rev, $chg, $untracked) = @_;
 	$f =~ s#^/##;
 	print "\t$chg\t$f\n" unless $_q;
 
@@ -2940,11 +2950,25 @@ sub libsvn_get_file {
 		waitpid $pid, 0;
 		$hash =~ /^$sha1$/o or die "not a sha1: $hash\n";
 	}
+	%{$untracked->{file_prop}->{$f}} = %$props;
 	print $gui $mode,' ',$hash,"\t",$f,"\0" or croak $!;
 }
 
+sub uri_encode {
+	my ($f) = @_;
+	$f =~ s#([^a-zA-Z0-9\*!\:_\./\-])#uc sprintf("%%%02x",ord($1))#eg;
+	$f
+}
+
+sub uri_decode {
+	my ($f) = @_;
+	$f =~ tr/+/ /;
+	$f =~ s/%([A-F0-9]{2})/chr hex($1)/ge;
+	$f
+}
+
 sub libsvn_log_entry {
-	my ($rev, $author, $date, $msg, $parents) = @_;
+	my ($rev, $author, $date, $msg, $parents, $untracked) = @_;
 	my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
 					 (\d\d)\:(\d\d)\:(\d\d).\d+Z$/x)
 				or die "Unable to parse date: $date\n";
@@ -2952,8 +2976,65 @@ sub libsvn_log_entry {
 		die "Author: $author not defined in $_authors file\n";
 	}
 	$msg = '' if ($rev == 0 && !defined $msg);
-	return { revision => $rev, date => "+0000 $Y-$m-$d $H:$M:$S",
-		author => $author, msg => $msg."\n", parents => $parents || [] }
+
+	open my $un, '>>', "$GIT_SVN_DIR/unhandled.log" or croak $!;
+	my $h;
+	print $un "r$rev\n" or croak $!;
+	$h = $untracked->{empty};
+	foreach (sort keys %$h) {
+		my $act = $h->{$_} ? '+empty_dir' : '-empty_dir';
+		print $un "  $act: ", uri_encode($_), "\n" or croak $!;
+		warn "W: $act: $_\n";
+	}
+	foreach my $t (qw/dir_prop file_prop/) {
+		$h = $untracked->{$t} or next;
+		foreach my $path (sort keys %$h) {
+			my $ppath = $path eq '' ? '.' : $path;
+			foreach my $prop (sort keys %{$h->{$path}}) {
+				next if $SKIP{$prop};
+				my $v = $h->{$path}->{$prop};
+				if (defined $v) {
+					print $un "  +$t: ",
+						  uri_encode($ppath), ' ',
+						  uri_encode($prop), ' ',
+						  uri_encode($v), "\n"
+						  or croak $!;
+				} else {
+					print $un "  -$t: ",
+						  uri_encode($ppath), ' ',
+						  uri_encode($prop), "\n"
+						  or croak $!;
+				}
+			}
+		}
+	}
+	foreach my $t (qw/absent_file absent_directory/) {
+		$h = $untracked->{$t} or next;
+		foreach my $parent (sort keys %$h) {
+			foreach my $path (sort @{$h->{$parent}}) {
+				print $un "  $t: ",
+				      uri_encode("$parent/$path"), "\n"
+				      or croak $!;
+				warn "W: $t: $parent/$path ",
+				     "Insufficient permissions?\n";
+			}
+		}
+	}
+
+	# revprops (make this optional? it's an extra network trip...)
+	my $pool = SVN::Pool->new;
+	my $rp = $SVN->rev_proplist($rev, $pool);
+	foreach (sort keys %$rp) {
+		next if /^svn:(?:author|date|log)$/;
+		print $un "  rev_prop: ", uri_encode($_), ' ',
+		          uri_encode($rp->{$_}), "\n";
+	}
+	$pool->clear;
+	close $un or croak $!;
+
+	{ revision => $rev, date => "+0000 $Y-$m-$d $H:$M:$S",
+	  author => $author, msg => $msg."\n", parents => $parents || [],
+	  revprops => $rp }
 }
 
 sub process_rm {
@@ -2972,9 +3053,11 @@ sub process_rm {
 		}
 		print "\tD\t$f/\n" unless $q;
 		close $ls or croak $?;
+		return $SVN::Node::dir;
 	} else {
 		print $gui '0 ',0 x 40,"\t",$f,"\0" or croak $!;
 		print "\tD\t$f\n" unless $q;
+		return $SVN::Node::file;
 	}
 }
 
@@ -2995,13 +3078,14 @@ sub libsvn_fetch_delta {
 	unless ($ed->{git_commit_ok}) {
 		die "SVN connection failed somewhere...\n";
 	}
-	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit]);
+	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit], $ed);
 }
 
 sub libsvn_fetch_full {
 	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
 	open my $gui, '| git-update-index -z --index-info' or croak $!;
 	my %amr;
+	my $ut = { empty => {}, dir_prop => {}, file_prop => {} };
 	my $p = $SVN->{svn_path};
 	foreach my $f (keys %$paths) {
 		my $m = $paths->{$f}->action();
@@ -3012,8 +3096,11 @@ sub libsvn_fetch_full {
 			$f =~ s#^/##;
 		}
 		if ($m =~ /^[DR]$/) {
-			process_rm($gui, $last_commit, $f, $_q);
-			next if $m eq 'D';
+			my $t = process_rm($gui, $last_commit, $f, $_q);
+			if ($m eq 'D') {
+				$ut->{empty}->{$f} = 0 if $t == $SVN::Node::dir;
+				next;
+			}
 			# 'R' can be file replacements, too, right?
 		}
 		my $pool = SVN::Pool->new;
@@ -3026,18 +3113,32 @@ sub libsvn_fetch_full {
 			}
 		} elsif ($t == $SVN::Node::dir && $m =~ /^[AR]$/) {
 			my @traversed = ();
-			libsvn_traverse($gui, '', $f, $rev, \@traversed);
-			foreach (@traversed) {
-				$amr{$_} = $m;
+			libsvn_traverse($gui, '', $f, $rev, \@traversed, $ut);
+			if (@traversed) {
+				foreach (@traversed) {
+					$amr{$_} = $m;
+				}
+			} else {
+				my ($dir, $file) = ($f =~ m#^(.*?)/?([^/]+)$#);
+				delete $ut->{empty}->{$dir};
+				$ut->{empty}->{$f} = 1;
 			}
 		}
 		$pool->clear;
 	}
 	foreach (keys %amr) {
-		libsvn_get_file($gui, $_, $rev, $amr{$_});
+		libsvn_get_file($gui, $_, $rev, $amr{$_}, $ut);
+		my ($d) = ($_ =~ m#^(.*?)/?(?:[^/]+)$#);
+		delete $ut->{empty}->{$d};
+	}
+	unless (exists $ut->{dir_prop}->{''}) {
+		my $pool = SVN::Pool->new;
+		my (undef, undef, $props) = $SVN->get_dir('', $rev, $pool);
+		%{$ut->{dir_prop}->{''}} = %$props;
+		$pool->clear;
 	}
 	close $gui or croak $?;
-	return libsvn_log_entry($rev, $author, $date, $msg, [$last_commit]);
+	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit], $ut);
 }
 
 sub svn_grab_base_rev {
@@ -3098,25 +3199,38 @@ sub libsvn_parse_revision {
 }
 
 sub libsvn_traverse {
-	my ($gui, $pfx, $path, $rev, $files) = @_;
+	my ($gui, $pfx, $path, $rev, $files, $untracked) = @_;
 	my $cwd = length $pfx ? "$pfx/$path" : $path;
 	my $pool = SVN::Pool->new;
 	$cwd =~ s#^\Q$SVN->{svn_path}\E##;
+	my $nr = 0;
 	my ($dirent, $r, $props) = $SVN->get_dir($cwd, $rev, $pool);
+	%{$untracked->{dir_prop}->{$cwd}} = %$props;
 	foreach my $d (keys %$dirent) {
 		my $t = $dirent->{$d}->kind;
 		if ($t == $SVN::Node::dir) {
-			libsvn_traverse($gui, $cwd, $d, $rev, $files);
+			my $i = libsvn_traverse($gui, $cwd, $d, $rev,
+			                        $files, $untracked);
+			if ($i) {
+				$nr += $i;
+			} else {
+				$untracked->{empty}->{"$cwd/$d"} = 1;
+			}
 		} elsif ($t == $SVN::Node::file) {
+			$nr++;
 			my $file = "$cwd/$d";
 			if (defined $files) {
 				push @$files, $file;
 			} else {
-				libsvn_get_file($gui, $file, $rev, 'A');
+				libsvn_get_file($gui, $file, $rev, 'A',
+				                $untracked);
+				my ($dir) = ($file =~ m#^(.*?)/?(?:[^/]+)$#);
+				delete $untracked->{empty}->{$dir};
 			}
 		}
 	}
 	$pool->clear;
+	$nr;
 }
 
 sub libsvn_traverse_ignore {
@@ -3255,6 +3369,7 @@ sub libsvn_new_tree {
 		return $log_entry;
 	}
 	my ($paths, $rev, $author, $date, $msg) = @_;
+	my $ut;
 	if ($_xfer_delta) {
 		my $pool = SVN::Pool->new;
 		my $ed = SVN::Git::Fetcher->new({q => $_q});
@@ -3266,12 +3381,14 @@ sub libsvn_new_tree {
 		unless ($ed->{git_commit_ok}) {
 			die "SVN connection failed somewhere...\n";
 		}
+		$ut = $ed;
 	} else {
+		$ut = { empty => {}, dir_prop => {}, file_prop => {} };
 		open my $gui, '| git-update-index -z --index-info' or croak $!;
-		libsvn_traverse($gui, '', $SVN->{svn_path}, $rev);
+		libsvn_traverse($gui, '', $SVN->{svn_path}, $rev, undef, $ut);
 		close $gui or croak $?;
 	}
-	return libsvn_log_entry($rev, $author, $date, $msg);
+	libsvn_log_entry($rev, $author, $date, $msg, [], $ut);
 }
 
 sub find_graft_path_commit {
@@ -3456,13 +3573,28 @@ sub new {
 	$self->{gui} = $gui;
 	$self->{c} = $git_svn->{c} if exists $git_svn->{c};
 	$self->{q} = $git_svn->{q};
+	$self->{empty} = {};
+	$self->{dir_prop} = {};
+	$self->{file_prop} = {};
+	$self->{absent_dir} = {};
+	$self->{absent_file} = {};
 	require Digest::MD5;
 	$self;
 }
 
+sub open_root {
+	{ path => '' };
+}
+
+sub open_directory {
+	my ($self, $path, $pb, $rev) = @_;
+	{ path => $path };
+}
+
 sub delete_entry {
 	my ($self, $path, $rev, $pb) = @_;
-	process_rm($self->{gui}, $self->{c}, $path, $self->{q});
+	my $t = process_rm($self->{gui}, $self->{c}, $path, $self->{q});
+	$self->{empty}->{$path} = 0 if $t == $SVN::Node::dir;
 	undef;
 }
 
@@ -3479,10 +3611,41 @@ sub open_file {
 
 sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
+	my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
+	delete $self->{empty}->{$dir};
 	{ path => $path, mode_a => 100644, mode_b => 100644,
 	  pool => SVN::Pool->new, action => 'A' };
 }
 
+sub add_directory {
+	my ($self, $path, $cp_path, $cp_rev) = @_;
+	my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
+	delete $self->{empty}->{$dir};
+	$self->{empty}->{$path} = 1;
+	{ path => $path };
+}
+
+sub change_dir_prop {
+	my ($self, $db, $prop, $value) = @_;
+	$self->{dir_prop}->{$db->{path}} ||= {};
+	$self->{dir_prop}->{$db->{path}}->{$prop} = $value;
+	undef;
+}
+
+sub absent_directory {
+	my ($self, $path, $pb) = @_;
+	$self->{absent_dir}->{$pb->{path}} ||= [];
+	push @{$self->{absent_dir}->{$pb->{path}}}, $path;
+	undef;
+}
+
+sub absent_file {
+	my ($self, $path, $pb) = @_;
+	$self->{absent_file}->{$pb->{path}} ||= [];
+	push @{$self->{absent_file}->{$pb->{path}}}, $path;
+	undef;
+}
+
 sub change_file_prop {
 	my ($self, $fb, $prop, $value) = @_;
 	if ($prop eq 'svn:executable') {
@@ -3491,6 +3654,9 @@ sub change_file_prop {
 		}
 	} elsif ($prop eq 'svn:special') {
 		$fb->{mode_b} = defined $value ? 120000 : 100644;
+	} else {
+		$self->{file_prop}->{$fb->{path}} ||= {};
+		$self->{file_prop}->{$fb->{path}}->{$prop} = $value;
 	}
 	undef;
 }
-- 
1.4.4.2.g6f98

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] git-svn: allow dcommit to take an alternate head
  2006-12-12 22:47 [PATCH 1/3] git-svn: enable logging of information not supported by git Eric Wong
@ 2006-12-12 22:47 ` Eric Wong
  2006-12-12 22:47 ` [PATCH 3/3] git-svn: correctly display fatal() error messages Eric Wong
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Wong @ 2006-12-12 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong

Previously dcommit would unconditionally commit all patches
up-to and including the current HEAD.  Now if an optional
command-line argument is specified, it will only commit
up to the specified revision.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-svn.txt |    6 ++++--
 git-svn.perl              |   11 ++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index a45067e..c589a98 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -57,11 +57,13 @@ See '<<fetch-args,Additional Fetch Arguments>>' if you are interested in
 manually joining branches on commit.
 
 'dcommit'::
-	Commit all diffs from the current HEAD directly to the SVN
+	Commit all diffs from a specified head directly to the SVN
 	repository, and then rebase or reset (depending on whether or
-	not there is a diff between SVN and HEAD).  It is recommended
+	not there is a diff between SVN and head).  It is recommended
 	that you run git-svn fetch and rebase (not pull) your commits
 	against the latest changes in the SVN repository.
+	An optional command-line argument may be specified as an
+	alternative to HEAD.
 	This is advantageous over 'commit' (below) because it produces
 	cleaner, more linear history.
 
diff --git a/git-svn.perl b/git-svn.perl
index 06e89ff..819584b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -604,8 +604,9 @@ sub commit_lib {
 }
 
 sub dcommit {
+	my $head = shift || 'HEAD';
 	my $gs = "refs/remotes/$GIT_SVN";
-	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..HEAD"));
+	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..$head"));
 	my $last_rev;
 	foreach my $d (reverse @refs) {
 		if (quiet_run('git-rev-parse','--verify',"$d~1") != 0) {
@@ -632,16 +633,16 @@ sub dcommit {
 	}
 	return if $_dry_run;
 	fetch();
-	my @diff = safe_qx(qw/git-diff-tree HEAD/, $gs);
+	my @diff = safe_qx('git-diff-tree', $head, $gs);
 	my @finish;
 	if (@diff) {
 		@finish = qw/rebase/;
 		push @finish, qw/--merge/ if $_merge;
 		push @finish, "--strategy=$_strategy" if $_strategy;
-		print STDERR "W: HEAD and $gs differ, using @finish:\n", @diff;
+		print STDERR "W: $head and $gs differ, using @finish:\n", @diff;
 	} else {
-		print "No changes between current HEAD and $gs\n",
-		      "Hard resetting to the latest $gs\n";
+		print "No changes between current $head and $gs\n",
+		      "Resetting to the latest $gs\n";
 		@finish = qw/reset --mixed/;
 	}
 	sys('git', @finish, $gs);
-- 
1.4.4.2.g6f98

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] git-svn: correctly display fatal() error messages
  2006-12-12 22:47 [PATCH 1/3] git-svn: enable logging of information not supported by git Eric Wong
  2006-12-12 22:47 ` [PATCH 2/3] git-svn: allow dcommit to take an alternate head Eric Wong
@ 2006-12-12 22:47 ` Eric Wong
  2006-12-13  0:45   ` [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/ Eric Wong
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Wong @ 2006-12-12 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong

If I wanted to print $@, I'd pass $@ to fatal().  This looks like
a stupid typo on my part.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 819584b..c746a3c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -31,7 +31,7 @@ my %SKIP = ( 'svn:wc:ra_dav:version-url' => 1,
              'svn:entry:committed-date' => 1,
 );
 
-sub fatal (@) { print STDERR $@; exit 1 }
+sub fatal (@) { print STDERR @_; exit 1 }
 # If SVN:: library support is added, please make the dependencies
 # optional and preserve the capability to use the command-line client.
 # use eval { require SVN::... } to make it lazy load
-- 
1.4.4.2.g6f98

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/
  2006-12-12 22:47 ` [PATCH 3/3] git-svn: correctly display fatal() error messages Eric Wong
@ 2006-12-13  0:45   ` Eric Wong
  2006-12-13  1:27     ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Wong @ 2006-12-13  0:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

We now use git-rev-parse universally to read refs, instead
of our own file_to_s function (which I plan on removing).

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 1f8a3b0..aac8f73 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2016,9 +2016,17 @@ sub git_commit {
 
 	# just in case we clobber the existing ref, we still want that ref
 	# as our parent:
-	if (my $cur = eval { file_to_s("$GIT_DIR/refs/remotes/$GIT_SVN") }) {
+	open my $null, '>', '/dev/null' or croak $!;
+	open my $stderr, '>&', \*STDERR or croak $!;
+	open STDERR, '>&', $null or croak $!;
+	if (my $cur = eval { safe_qx('git-rev-parse',
+	                             "refs/remotes/$GIT_SVN^0") }) {
+		chomp $cur;
 		push @tmp_parents, $cur;
 	}
+	open STDERR, '>&', $stderr or croak $!;
+	close $stderr or croak $!;
+	close $null or croak $!;
 
 	if (exists $tree_map{$tree}) {
 		foreach my $p (@{$tree_map{$tree}}) {
-- 
1.4.4.2.g6f98

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/
  2006-12-13  0:45   ` [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/ Eric Wong
@ 2006-12-13  1:27     ` Junio C Hamano
  2006-12-13  2:26       ` Eric Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2006-12-13  1:27 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Eric Wong <normalperson@yhbt.net> writes:

> We now use git-rev-parse universally to read refs, instead
> of our own file_to_s function (which I plan on removing).
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  git-svn.perl |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 1f8a3b0..aac8f73 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2016,9 +2016,17 @@ sub git_commit {
>  
>  	# just in case we clobber the existing ref, we still want that ref
>  	# as our parent:
> -	if (my $cur = eval { file_to_s("$GIT_DIR/refs/remotes/$GIT_SVN") }) {
> +	open my $null, '>', '/dev/null' or croak $!;
> +	open my $stderr, '>&', \*STDERR or croak $!;
> +	open STDERR, '>&', $null or croak $!;
> +	if (my $cur = eval { safe_qx('git-rev-parse',
> +	                             "refs/remotes/$GIT_SVN^0") }) {
> +		chomp $cur;
>  		push @tmp_parents, $cur;
>  	}
> +	open STDERR, '>&', $stderr or croak $!;
> +	close $stderr or croak $!;
> +	close $null or croak $!;
>  
>  	if (exists $tree_map{$tree}) {
>  		foreach my $p (@{$tree_map{$tree}}) {

It's a neat trick you do to stash away STDERR and redirect to
/dev/null ;-).  It might be worth doing something like this:

	sub without_stderr {
        	my ($cmd, @param) = @_;
                # stash away magic
                my @return = eval {
                	safe_qx($cmd, @param);
                };
                # restore magic
                return @return;
	}

in Git.pm?

Then the caller would do something like this instead:

	sub git_commit {
        	...
		if (my ($cur) = without_stderr(qw(git rev-parse --verify),
			"refs/remotes/$GIT_SVN^0")) {
                	...


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/
  2006-12-13  1:27     ` Junio C Hamano
@ 2006-12-13  2:26       ` Eric Wong
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2006-12-13  2:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > We now use git-rev-parse universally to read refs, instead
> > of our own file_to_s function (which I plan on removing).
> >
> > Signed-off-by: Eric Wong <normalperson@yhbt.net>
> > ---
> >  git-svn.perl |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> >
> > diff --git a/git-svn.perl b/git-svn.perl
> > index 1f8a3b0..aac8f73 100755
> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -2016,9 +2016,17 @@ sub git_commit {
> >  
> >  	# just in case we clobber the existing ref, we still want that ref
> >  	# as our parent:
> > -	if (my $cur = eval { file_to_s("$GIT_DIR/refs/remotes/$GIT_SVN") }) {
> > +	open my $null, '>', '/dev/null' or croak $!;
> > +	open my $stderr, '>&', \*STDERR or croak $!;
> > +	open STDERR, '>&', $null or croak $!;
> > +	if (my $cur = eval { safe_qx('git-rev-parse',
> > +	                             "refs/remotes/$GIT_SVN^0") }) {
> > +		chomp $cur;
> >  		push @tmp_parents, $cur;
> >  	}
> > +	open STDERR, '>&', $stderr or croak $!;
> > +	close $stderr or croak $!;
> > +	close $null or croak $!;
> >  
> >  	if (exists $tree_map{$tree}) {
> >  		foreach my $p (@{$tree_map{$tree}}) {
> 
> It's a neat trick you do to stash away STDERR and redirect to
> /dev/null ;-).  It might be worth doing something like this:

I also did it for libsvn_get_file() with STDOUT :)

> 	sub without_stderr {
>         	my ($cmd, @param) = @_;
>                 # stash away magic
>                 my @return = eval {
>                 	safe_qx($cmd, @param);
>                 };
>                 # restore magic
>                 return @return;
> 	}
> 
> in Git.pm?

Looking at Git.pm for the first time, it seems that _command_common_pipe
(and all it's users) already support closing/redirecting STDERR.

I'll look into revamping git-svn to use Git.pm sometime tonight or
tomorrow.

-- 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-12-13  2:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-12 22:47 [PATCH 1/3] git-svn: enable logging of information not supported by git Eric Wong
2006-12-12 22:47 ` [PATCH 2/3] git-svn: allow dcommit to take an alternate head Eric Wong
2006-12-12 22:47 ` [PATCH 3/3] git-svn: correctly display fatal() error messages Eric Wong
2006-12-13  0:45   ` [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/ Eric Wong
2006-12-13  1:27     ` Junio C Hamano
2006-12-13  2:26       ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).