From: Benoit SIGOURE <tsuna@lrde.epita.fr>
Date: Thu, 19 Jul 2007 17:41:07 GMT
Subject: Re: [PATCH] Proposal for git-svn
Message-ID: <6F6FBB0C-5CDB-4280-AD25-C805634B8B7B@lrde.epita.fr>
In-Reply-To: <20070719042255.GA17433@muzzle>

On Jul 19, 2007, at 6:22 AM, Eric Wong wrote:

> Benoit SIGOURE <tsuna@lrde.epita.fr> wrote:
>> Hello,
>>
>> I'm importing many SVN repositories in Git and I ran across a  
>> problem:
>> ufloat.h has mode 120000but is not a link
>>
>> I've read the code and checked-out the revision where the problem
>> occured and it turns out that some stupid user commited a broken
>> symlink and I think that's where the problem came from.  I'm
>> proposing the following trivial change to let git-svn clone continue
>> its work:
>>
>> diff --git a/git-svn.perl b/git-svn.perl
>> index 01c3904..a82baf4 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -2555,8 +2555,8 @@ sub close_file {
>>                 sysseek($fh, 0, 0) or croak $!;
>>                 if ($fb->{mode_b} == 120000) {
>>                         sysread($fh, my $buf, 5) == 5 or croak $!;
>> -                       $buf eq 'link ' or die "$path has mode  
>> 120000",
>> -                                              "but is not a link\n";
>> +                       $buf eq 'link ' or warn "$path has mode  
>> 120000",
>> +                                              " but is not a link 
>> \n";
>>                 }
>>                 defined(my $pid = open my $out,'-|') or die "Can't
>> fork: $!\n";
>>                 if (!$pid) {
>>
>> (I also added a whitespace because "120000but" does not look good :D)
>> I checked out the problematic revision in git and I see the broken
>> symlink just like in SVN so I assume this change is correct.
>
> Very strange.  Since $buf didn't have the string "link " in it, did it
> have a path name in it?  If so, the sysread() would've advanced the  
> $fh
> offset by 5 bytes; causing an even more broken symlink to be added  
> by git.
>
> Would the following be more correct?
>
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2552,9 +2552,15 @@ sub close_file {
>  		}
>  		sysseek($fh, 0, 0) or croak $!;
>  		if ($fb->{mode_b} == 120000) {
> -			sysread($fh, my $buf, 5) == 5 or croak $!;
> -			$buf eq 'link ' or die "$path has mode 120000",
> -			                       "but is not a link\n";
> +			eval {
> +				sysread($fh, my $buf, 5) == 5 or croak $!;
> +				$buf eq 'link ' or die "$path has mode 120000",
> +						       " but is not a link";
> +			};
> +			if ($@) {
> +				warn "$@\n";
> +				sysseek($fh, 0, 0) or croak $!;
> +			}
>  		}
>  		defined(my $pid = open my $out,'-|') or die "Can't fork: $!\n";
>  		if (!$pid) {
>

It does look more correct though I don't get the same sha1 sums with  
your patch.
Actually here is what happens:
At revision N, b0rken symlinks are added in the SVN repos.  Up to  
this revision, I get the same sha1 sums.
At N+1, the symlinks are removed from the SVN, and sha1 sums start to  
differ, for some reason.
I expected them to be either identical all the way through or to  
differ at N, but not at N+1.

In both cases the repository seem usable and I couldn't notice any  
other difference than the sha1 sums.

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory