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

From: Eric Wong <normalperson@yhbt.net>
Date: Jul 23, 2014, 22:04 UTC
Message-ID: <20140723220414.GA14145@dcvr.yhbt.net>
In-Reply-To: <1405657201-32035-1-git-send-email-amanduch@gmail.com>

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* :).

Show 8 quoted lines

> --- 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.

Thanks.