Re: [PATCH 4/3] git-svn: correctly handle packed-refs in refs/remotes/

[Eric Wong <normalperson@yhbt.net>]

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.

--