Re: git-svn failure when symlink added in svn

[Eric Wong <normalperson@yhbt.net>]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Junio C Hamano <junkio@cox.net> wrote:
> >> diff --git a/git-svn.perl b/git-svn.perl
> >> index 4be8576..cef6697 100755
> >> --- a/git-svn.perl
> >> +++ b/git-svn.perl
> >> @@ -2464,15 +2464,15 @@ sub close_file {
> >>  	my $hash;
> >>  	my $path = $self->git_path($fb->{path});
> >>  	if (my $fh = $fb->{fh}) {
> >> -		seek($fh, 0, 0) or croak $!;
> >> +		sysseek($fh, 0, 0) or croak $!;
> >>  		my $md5 = Digest::MD5->new;
> >>  		$md5->addfile($fh);
> >
> > We may want to keep the plain seek() here and do both seek and sysseek,
> > I'm not sure if $md5->addfile() uses read or sysread internally.
> 
> Ok.  The seek before Digest::MD5 can stay as it has been that
> way for a long time without causing problems.  How about this as
> an replacement then?

Looks good to me.  Seth?

If Seth is okay with it, then:
Acked-by: Eric Wong <normalperson@yhbt.net>

> -- >8 --
> [PATCH] Fix symlink handling in git-svn, related to PerlIO
> 
> After reading the leading contents from a symlink data obtained
> from subversion, which we expect to begin with 'link ', the code
> forked to hash the remainder (which should match readlink()
> result) using git-hash-objects, by redirecting its STDIN from
> the filehandle we read that 'link ' from.  This was Ok with Perl
> on modern Linux, but on Mac OS, the read in the parent process
> slurped more than we asked for in stdio buffer, and the child
> did not correctly see the "remainder".
> 
> This attempts to fix the issue by using lower level sysseek and
> sysread instead of seek and read to bypass the stdio buffer.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
>  git-svn.perl |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 4be8576..6f509f8 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2470,9 +2470,9 @@ sub close_file {
>  		my $got = $md5->hexdigest;
>  		die "Checksum mismatch: $path\n",
>  		    "expected: $exp\n    got: $got\n" if ($got ne $exp);
> -		seek($fh, 0, 0) or croak $!;
> +		sysseek($fh, 0, 0) or croak $!;
>  		if ($fb->{mode_b} == 120000) {
> -			read($fh, my $buf, 5) == 5 or croak $!;
> +			sysread($fh, my $buf, 5) == 5 or croak $!;
>  			$buf eq 'link ' or die "$path has mode 120000",
>  			                       "but is not a link\n";
>  		}
> -- 
> 1.5.2.rc0.781.g5868

-- 
Eric Wong