Re: [PATCH] git-svn: doublecheck if really file or dir

[Andrej Manduch <amanduch@gmail.com>]

Hi,

On 07/24/2014 12:04 AM, Eric Wong wrote:
> Andrej Manduch <amanduch@gmail.com> wrote:
>> * this fixes 'git svn info `pwd`' buggy behaviour
> 
> Good catch, the commit could use a better description, something like:
> --------------------------- 8< ----------------------------
> Subject: [PATCH] git-svn: "info" checks for dirs more carefully
> 
> This avoids a "Reading from filehandle failed at ..." error when
> running "git svn info `pwd`".
> 
> Signed-off-by: Andrej Manduch <amanduch@gmail.com>
> --------------------------- 8< ----------------------------
> 
> While your patch avoids an error, but the output isn't right, either.
> I tried it running in /home/ew/ruby, the URL field is bogus:
> 
>     ~/ruby$ git svn info `pwd`
>     Path: /home/ew/ruby
>     URL: svn+ssh://svn.ruby-lang.org/ruby/trunk/home/ew/ruby
>     Repository Root: svn+ssh://svn.ruby-lang.org/ruby
>     Repository UUID: b2dd03c8-39d4-4d8f-98ff-823fe69b080e
>     Revision: 46901
>     Node Kind: directory
>     Schedule: normal
>     Last Changed Author: hsbt
>     Last Changed Rev: 46901
>     Last Changed Date: 2014-07-22 19:06:12 +0000 (Tue, 22 Jul 2014)
> 
> The URL should be:
> 
>     URL: svn+ssh://svn.ruby-lang.org/ruby/trunk
> 
> It's better than an error, but it'd be nice if someone who uses
> this command can fix it (*hint* :).

Thx, I missed this. However this bug was not introduced with my patch,
it was there before. If you try use `git svn info full_path` and
directory is not a root dir this bug will occour even wihout my patch.

However I'll try to find some time to fix this too.

On 07/24/2014 12:04 AM, Eric Wong wrote:
> 
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -2029,7 +2029,7 @@ sub find_file_type_and_diff_status {
>>  	my $mode = (split(' ', $ls_tree))[0] || "";
>>  
>>  	return ("link", $diff_status) if $mode eq "120000";
>> -	return ("dir", $diff_status) if $mode eq "040000";
>> +	return ("dir", $diff_status) if $mode eq "040000" or -d $path;
> 
> "or" has a lower precedence than "||", so I would do the following:
> 
> 	return ("dir", $diff_status) if $mode eq "040000" || -d $path;
> 
> The general rule I've learned is to use "||" for conditionals and
> "or" for control flow (e.g. do_something() or die("...") ).
> 
> I can take your patch with the above changes (no need to resend),
> but I'd be happier to see the URL field corrected if you want
> to reroll.

I'll try to fix whis url bug, but it will be different patch 'cause I
think, this is different kind of a problem.

On 07/24/2014 12:04 AM, Eric Wong wrote:
> 
> Thanks.
> 

I thanks to you for great review.

--
Best Regards,
b.