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

[Andrej Manduch <amanduch@gmail.com>]

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 <amanduch@gmail.com>
>> Signed-off-by: Eric Wong <normalperson@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 <amanduch@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
>> +++ 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"
>>  	'
>>