From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 04 Aug 2008 00:01:40 GMT
Subject: Re: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.
Message-ID: <20080804000140.GA13019@untitled>
In-Reply-To: <1217451235-9609-1-git-send-email-apenwarr@gmail.com>

Avery Pennarun <apenwarr@stasis.open.versabanq.com> wrote:
> Previously, if a config entry looked like this:
> 
>          svn-remote.svn.fetch=:refs/heads/whatever
> 
> git-svn would silently do nothing if you asked it to "git svn fetch", and
> give a strange error if asked to "git svn dcommit".  What it really wants is
> a line that looks like this:
> 
> 	svn-remote.svn.fetch=:refs/remotes/whatever
> 
> So we should simply abort if we get the wrong thing.
> 
> On the other hand, there's actually no good reason for git-svn to enforce
> using the refs/remotes namespace, but the code seems to have hardcoded this
> in several places and I'm not brave enough to try to fix it all right now.

Fully agreed (as I've stated in the past, too).  I just haven't had
time to fix it.

> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>

Thanks Avery,
Acked-by: Eric Wong <normalperson@yhbt.net>

> ---
> 
> I just spent altogether too much time tracking down this problem when
> migrating my git-svn settings from one repo to another.
> 
>  git-svn.perl |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index cf6dbbc..cc35f50 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1420,8 +1420,12 @@ sub read_all_remotes {
>  	    svn.useSvmProps/) };
>  	$use_svm_props = $use_svm_props eq 'true' if $use_svm_props;
>  	foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
> -		if (m!^(.+)\.fetch=\s*(.*)\s*:\s*refs/remotes/(.+)\s*$!) {
> -			my ($remote, $local_ref, $remote_ref) = ($1, $2, $3);
> +		if (m!^(.+)\.fetch=\s*(.*)\s*:\s*(.+)\s*$!) {
> +			my ($remote, $local_ref, $_remote_ref) = ($1, $2, $3);
> +			die("svn-remote.$remote: remote ref '$_remote_ref' "
> +			    . "must start with 'refs/remotes/'\n")
> +				unless $_remote_ref =~ m{^refs/remotes/(.+)};
> +			my $remote_ref = $1;
>  			$local_ref =~ s{^/}{};
>  			$r->{$remote}->{fetch}->{$local_ref} = $remote_ref;
>  			$r->{$remote}->{svm} = {} if $use_svm_props;
> -- 
> 1.6.0.rc0.42.g186458.dirty
> 
>