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

2014-08-03 Thread Andrej Manduch
Hi Eric,

Nice touch, It works like charm. However unfortunatelly now I think you
introduced new bug :)

On 08/03/2014 04:45 AM, Eric Wong wrote:
 Hi Andrej, I could not help thinking your patch was obscuring
 another bug.  I think I have an alternative to your patch which
 fixes both our bugs.  Can you give this a shot?  Thanks.
 
 --- 8 
 Subject: [PATCH] git svn: info: correctly handle absolute path args
 
 Calling git svn info $(pwd) would hit:
   Reading from filehandle failed at ...
 errors due to improper prefixing and canonicalization.
 
 Strip the toplevel path from absolute filesystem paths to ensure
 downstream canonicalization routines are only exposed to paths
 tracked in git (or SVN).
 
 Noticed-by: Andrej Manduch amand...@gmail.com
 Signed-off-by: Eric Wong normalper...@yhbt.net
 ---
  git-svn.perl| 21 +++--
  t/t9119-git-svn-info.sh | 10 ++
  2 files changed, 25 insertions(+), 6 deletions(-)
 
 diff --git a/git-svn.perl b/git-svn.perl
 index 1f41ee1..1f9582b 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1477,10 +1477,19 @@ sub cmd_commit_diff {
   }
  }
  
 -
  sub cmd_info {
 - my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
 - my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
 + my $path_arg = defined($_[0]) ? $_[0] : '.';
 + my $path = $path_arg;
 + if ($path =~ m!\A/!) {
 + my $toplevel = eval {
 + my @cmd = qw/rev-parse --show-toplevel/;
 + command_oneline(\@cmd, STDERR = 0);
 + };
 + $path =~ s!\A\Q$toplevel\E/?!!;
I have problem with this line ^^^

Suppose your $toplevel is /sometning and you type in command line
something like that: git svn info /somethingsrc and as you see this
should end up with error. However $path =~ s!\A\Q$toplevel\E/?!!;
will just cut /sometning from /somethingsrc and and up with same
answer as for svn git info src which is not equivalent query.

Second scenario is something which worries me more: If your query look
like this: git svn info /something//src it will just end up with error
because it will set $path to /src witch is outside of repository.

Second scenario can be fixed with this:

diff --git a/git-svn.perl b/git-svn.perl
index a69f0fc..00f9d01 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1483,7 +1483,7 @@ sub cmd_info {
my @cmd = qw/rev-parse --show-toplevel/;
command_oneline(\@cmd, STDERR = 0);
};
-   $path =~ s!\A\Q$toplevel\E/?!!;
+   $path =~ s!\A\Q$toplevel\E/*!!;
$path = canonicalize_path($path);
} else {
$path = canonicalize_path($cmd_dir_prefix . $path);


However I'm not sure if this will work on windows (where slashes are in
different orientation).


On 08/03/2014 04:45 AM, Eric Wong wrote:
 + $path = canonicalize_path($path);
 + } else {
 + $path = canonicalize_path($cmd_dir_prefix . $path);
 + }
   if (exists $_[1]) {
   die Too many arguments specified\n;
   }
 @@ -1501,14 +1510,14 @@ sub cmd_info {
   # canonicalize_path() will return  to make libsvn 1.5.x happy,
   $path = . if $path eq ;
  
 - my $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );
 + my $full_url = canonicalize_url( add_path_to_url( $url, $path ) );
  
   if ($_url) {
   print $full_url\n;
   return;
   }
  
 - my $result = Path: $path\n;
 + my $result = Path: $path_arg\n;
   $result .= Name:  . basename($path) . \n if $file_type ne dir;
   $result .= URL: $full_url\n;
  
 @@ -1539,7 +1548,7 @@ sub cmd_info {
   }
  
   my ($lc_author, $lc_rev, $lc_date_utc);
 - my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $fullpath);
 + my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $path);
   my $log = command_output_pipe(@args);
   my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
   while ($log) {
 diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
 index ff19695..4f6e669 100755
 --- a/t/t9119-git-svn-info.sh
 +++ b/t/t9119-git-svn-info.sh
 @@ -74,6 +74,16 @@ test_expect_success 'info .' 
   test_cmp_info expected.info-dot actual.info-dot
   
  
 +test_expect_success 'info $(pwd)' '
 + (cd svnwc; svn info $(pwd)) expected.info-pwd 
 + (cd gitwc; git svn info $(pwd)) actual.info-pwd 
 + grep -v ^Path: expected.info-pwd expected.info-np 
 + grep -v ^Path: actual.info-pwd actual.info-np 
 + test_cmp_info expected.info-np actual.info-np 
 + test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
 +  $(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
 + '
 +
  test_expect_success 'info --url .' '
   test $(cd gitwc; git svn info --url .) = $quoted_svnrepo
   '
 
--
To unsubscribe from this list: 

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

2014-08-03 Thread Andrej Manduch
On 08/03/2014 02:22 PM, Andrej Manduch wrote:
 Hi Eric,
 
 Nice touch, It works like charm. However unfortunatelly now I think you
 introduced new bug :)
 
 On 08/03/2014 04:45 AM, Eric Wong wrote:
 Hi Andrej, I could not help thinking your patch was obscuring
 another bug.  I think I have an alternative to your patch which
 fixes both our bugs.  Can you give this a shot?  Thanks.

 --- 8 
 Subject: [PATCH] git svn: info: correctly handle absolute path args

 Calling git svn info $(pwd) would hit:
   Reading from filehandle failed at ...
 errors due to improper prefixing and canonicalization.

 Strip the toplevel path from absolute filesystem paths to ensure
 downstream canonicalization routines are only exposed to paths
 tracked in git (or SVN).

 Noticed-by: Andrej Manduch amand...@gmail.com
 Signed-off-by: Eric Wong normalper...@yhbt.net
 ---
  git-svn.perl| 21 +++--
  t/t9119-git-svn-info.sh | 10 ++
  2 files changed, 25 insertions(+), 6 deletions(-)

 diff --git a/git-svn.perl b/git-svn.perl
 index 1f41ee1..1f9582b 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1477,10 +1477,19 @@ sub cmd_commit_diff {
  }
  }
  
 -
  sub cmd_info {
 -my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
 -my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
 +my $path_arg = defined($_[0]) ? $_[0] : '.';
 +my $path = $path_arg;
 +if ($path =~ m!\A/!) {
 +my $toplevel = eval {
 +my @cmd = qw/rev-parse --show-toplevel/;
 +command_oneline(\@cmd, STDERR = 0);
 +};
 +$path =~ s!\A\Q$toplevel\E/?!!;
 I have problem with this line ^^^
 
 Suppose your $toplevel is /sometning and you type in command line
 something like that: git svn info /somethingsrc and as you see this
 should end up with error. However $path =~ s!\A\Q$toplevel\E/?!!;
 will just cut /sometning from /somethingsrc and and up with same
 answer as for svn git info src which is not equivalent query.
 
 Second scenario is something which worries me more: If your query look
 like this: git svn info /something//src it will just end up with error
 because it will set $path to /src witch is outside of repository.
 
 Second scenario can be fixed with this:
 
 diff --git a/git-svn.perl b/git-svn.perl
 index a69f0fc..00f9d01 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1483,7 +1483,7 @@ sub cmd_info {
   my @cmd = qw/rev-parse --show-toplevel/;
   command_oneline(\@cmd, STDERR = 0);
   };
 - $path =~ s!\A\Q$toplevel\E/?!!;
 + $path =~ s!\A\Q$toplevel\E/*!!;
   $path = canonicalize_path($path);
   } else {
   $path = canonicalize_path($cmd_dir_prefix . $path);
 

Actualy this will be even better:


Signed-off-by: Andrej Manduch amand...@gmail.com
---
 git-svn.perl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-svn.perl b/git-svn.perl
index a69f0fc..58df866 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1483,6 +1483,7 @@ sub cmd_info {
my @cmd = qw/rev-parse --show-toplevel/;
command_oneline(\@cmd, STDERR = 0);
};
+   $path = canonicalize_path($path);
$path =~ s!\A\Q$toplevel\E/?!!;
$path = canonicalize_path($path);
} else {
-- 
2.0.0.GIT

Because this will have not problem with really weird query like: git
svn info /media/../media/something//src

 
 However I'm not sure if this will work on windows (where slashes are in
 different orientation).
 
 
 On 08/03/2014 04:45 AM, Eric Wong wrote:
 +$path = canonicalize_path($path);
 +} else {
 +$path = canonicalize_path($cmd_dir_prefix . $path);
 +}
  if (exists $_[1]) {
  die Too many arguments specified\n;
  }
 @@ -1501,14 +1510,14 @@ sub cmd_info {
  # canonicalize_path() will return  to make libsvn 1.5.x happy,
  $path = . if $path eq ;
  
 -my $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );
 +my $full_url = canonicalize_url( add_path_to_url( $url, $path ) );
  
  if ($_url) {
  print $full_url\n;
  return;
  }
  
 -my $result = Path: $path\n;
 +my $result = Path: $path_arg\n;
  $result .= Name:  . basename($path) . \n if $file_type ne dir;
  $result .= URL: $full_url\n;
  
 @@ -1539,7 +1548,7 @@ sub cmd_info {
  }
  
  my ($lc_author, $lc_rev, $lc_date_utc);
 -my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $fullpath);
 +my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $path);
  my $log = command_output_pipe(@args);
  my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
  while ($log) {
 diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
 index ff19695..4f6e669 100755
 --- a/t/t9119-git-svn-info.sh
 

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

2014-08-03 Thread Eric Wong
Andrej Manduch amand...@gmail.com wrote:
 On 08/03/2014 02:22 PM, Andrej Manduch wrote:
  Nice touch, It works like charm. However unfortunatelly now I think you
  introduced new bug :)

Good catch!

  On 08/03/2014 04:45 AM, Eric Wong wrote:
   sub cmd_info {
  -  my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
  -  my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
  +  my $path_arg = defined($_[0]) ? $_[0] : '.';
  +  my $path = $path_arg;
  +  if ($path =~ m!\A/!) {
  +  my $toplevel = eval {
  +  my @cmd = qw/rev-parse --show-toplevel/;
  +  command_oneline(\@cmd, STDERR = 0);
  +  };
  +  $path =~ s!\A\Q$toplevel\E/?!!;
  I have problem with this line ^^^
  
  Suppose your $toplevel is /sometning and you type in command line
  something like that: git svn info /somethingsrc and as you see this
  should end up with error. However $path =~ s!\A\Q$toplevel\E/?!!;
  will just cut /sometning from /somethingsrc and and up with same
  answer as for svn git info src which is not equivalent query.
  
  Second scenario is something which worries me more: If your query look
  like this: git svn info /something//src it will just end up with error
  because it will set $path to /src witch is outside of repository.
  
  Second scenario can be fixed with this:
  
 
 Actualy this will be even better:

Thanks Andrej.  I'll queue that on top of mine.
Can you turn that into a proper commit message with Subject?
Thanks.
(The English-generating part of my brain is too tired)

 Signed-off-by: Andrej Manduch amand...@gmail.com
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1483,6 +1483,7 @@ sub cmd_info {
   my @cmd = qw/rev-parse --show-toplevel/;
   command_oneline(\@cmd, STDERR = 0);
   };
 + $path = canonicalize_path($path);
   $path =~ s!\A\Q$toplevel\E/?!!;
   $path = canonicalize_path($path);
   } else {

 Because this will have not problem with really weird query like: git
 svn info /media/../media/something//src

I've also started working on the following test cases,
will squash:

diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index 4f6e669..f16f323 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -84,6 +84,26 @@ test_expect_success 'info $(pwd)' '
 $(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
'
 
+test_expect_success 'info $(pwd)/../___wc' '
+   (cd svnwc; svn info $(pwd)/../svnwc) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)/../gitwc) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
+test_expect_success 'info $(pwd)/../___wc//file' '
+   (cd svnwc; svn info $(pwd)/../svnwc//file) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)/../gitwc//file) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
 test_expect_success 'info --url .' '
test $(cd gitwc; git svn info --url .) = $quoted_svnrepo
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-02 Thread Eric Wong
Andrej Manduch amand...@gmail.com wrote:
 On 07/24/2014 12:04 AM, Eric Wong wrote:
  Andrej Manduch amand...@gmail.com wrote:
  * this fixes 'git svn info `pwd`' buggy behaviour
  
  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:

 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.

Hi Andrej, I could not help thinking your patch was obscuring
another bug.  I think I have an alternative to your patch which
fixes both our bugs.  Can you give this a shot?  Thanks.

--- 8 
Subject: [PATCH] git svn: info: correctly handle absolute path args

Calling git svn info $(pwd) would hit:
  Reading from filehandle failed at ...
errors due to improper prefixing and canonicalization.

Strip the toplevel path from absolute filesystem paths to ensure
downstream canonicalization routines are only exposed to paths
tracked in git (or SVN).

Noticed-by: Andrej Manduch amand...@gmail.com
Signed-off-by: Eric Wong normalper...@yhbt.net
---
 git-svn.perl| 21 +++--
 t/t9119-git-svn-info.sh | 10 ++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 1f41ee1..1f9582b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1477,10 +1477,19 @@ sub cmd_commit_diff {
}
 }
 
-
 sub cmd_info {
-   my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
-   my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
+   my $path_arg = defined($_[0]) ? $_[0] : '.';
+   my $path = $path_arg;
+   if ($path =~ m!\A/!) {
+   my $toplevel = eval {
+   my @cmd = qw/rev-parse --show-toplevel/;
+   command_oneline(\@cmd, STDERR = 0);
+   };
+   $path =~ s!\A\Q$toplevel\E/?!!;
+   $path = canonicalize_path($path);
+   } else {
+   $path = canonicalize_path($cmd_dir_prefix . $path);
+   }
if (exists $_[1]) {
die Too many arguments specified\n;
}
@@ -1501,14 +1510,14 @@ sub cmd_info {
# canonicalize_path() will return  to make libsvn 1.5.x happy,
$path = . if $path eq ;
 
-   my $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );
+   my $full_url = canonicalize_url( add_path_to_url( $url, $path ) );
 
if ($_url) {
print $full_url\n;
return;
}
 
-   my $result = Path: $path\n;
+   my $result = Path: $path_arg\n;
$result .= Name:  . basename($path) . \n if $file_type ne dir;
$result .= URL: $full_url\n;
 
@@ -1539,7 +1548,7 @@ sub cmd_info {
}
 
my ($lc_author, $lc_rev, $lc_date_utc);
-   my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $fullpath);
+   my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $path);
my $log = command_output_pipe(@args);
my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
while ($log) {
diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index ff19695..4f6e669 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -74,6 +74,16 @@ test_expect_success 'info .' 
test_cmp_info expected.info-dot actual.info-dot

 
+test_expect_success 'info $(pwd)' '
+   (cd svnwc; svn info $(pwd)) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
 test_expect_success 'info --url .' '
test $(cd gitwc; git svn info --url .) = $quoted_svnrepo
'
-- 
EW

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-26 Thread Andrej Manduch
Hi,

On 07/24/2014 12:04 AM, Eric Wong wrote:
 Andrej Manduch amand...@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 amand...@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 + (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 12;
 -return (dir, $diff_status) if $mode eq 04;
 +return (dir, $diff_status) if $mode eq 04 or -d $path;
 
 or has a lower precedence than ||, so I would do the following:
 
   return (dir, $diff_status) if $mode eq 04 || -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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-23 Thread Eric Wong
Andrej Manduch amand...@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 amand...@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 + (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* :).

 --- 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 12;
 - return (dir, $diff_status) if $mode eq 04;
 + return (dir, $diff_status) if $mode eq 04 or -d $path;

or has a lower precedence than ||, so I would do the following:

return (dir, $diff_status) if $mode eq 04 || -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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-17 Thread Andrej Manduch
I'm sorry, i've made mistake in this one.  Please ingnore it. Correct
patch is on the way.

kind regards,
b.

On Fri, Jul 18, 2014 at 6:05 AM, Andrej Manduch amand...@gmail.com wrote:
 * this fixes 'git svn info `pwd`' buggy behaviour

 Signed-off-by: Andrej Manduch amand...@gmail.com
 ---
  git-svn.perl | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/git-svn.perl b/git-svn.perl
 index 0a32372..c3d893e 100755
 --- 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 12;
 -   return (dir, $diff_status) if $mode eq 04;
 +   return (dir, $diff_status) if $mode eq 04 od -d $path;
 return (file, $diff_status);
  }

 --
 2.0.0.GIT

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html