Re: [PATCH] git-svn: make rebuild respect rewriteRoot option

[Eric Wong <normalperson@yhbt.net>]

Jan Krüger <jk@jk.gs> wrote:
> Suppose someone fetches git-svn-ified commits from another repo and then
> attempts to use 'git-svn init --rewrite-root=foo bar'. Using git svn rebase
> after that will fail badly:
> 
>  * For each commit tried by working_head_info, rebuild is called indirectly.
>  * rebuild will iterate over all commits and skip all of them because the
>    URL does not match. Because of that no rev_map file is generated at all.
>  * Thus, rebuild will run once for every commit. This takes ages.
>  * In the end there still isn't any rev_map file and thus working_head_info
>    fails.
> 
> Addressing this behaviour fixes an apparently not too uncommon problem with
> providing git-svn mirrors of Subversion repositories. Some repositories are
> accessed using different URLs depending on whether the user has push
> privileges or not. In the latter case, an anonymous URL is often used that
> differs from the push URL. Providing a mirror that is usable in both cases
> becomes a lot more possible with this change.
> 
> Signed-off-by: Jan Krüger <jk@jk.gs>
> ---
> Since this patch focuses on a case that is discouraged by the git-svn
> manpage (use rewriteRoot even though commits already exist), a bit of
> discussion might be helpful. On the up side, I think it doesn't affect any
> other cases.
> 
> The specific problem situation looks like this:
> 
> % git fetch git://.../clone-of-some-svn-repo (...)
> % git svn init -s --rewrite-root=<url contained in fetched commits> <url \
>       we want to use>
> % git svn rebase
> [Boom]
> 
> This patch passes all existing git-svn tests; feel free to suggest
> additional test(s) to cover this sort of situation.

Seems to make sense to me...  If you have a test case for this
it'd be nicer :)

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

>  git-svn.perl |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index a54979d..4c9c59b 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2577,8 +2577,8 @@ sub rebuild {
>  	my ($log, $ctx) =
>  	    command_output_pipe(qw/rev-list --pretty=raw --no-color --reverse/,
>  	                        $self->refname, '--');
> -	my $full_url = $self->full_url;
> -	remove_username($full_url);
> +	my $metadata_url = $self->metadata_url;
> +	remove_username($metadata_url);
>  	my $svn_uuid = $self->ra_uuid;
>  	my $c;
>  	while (<$log>) {
> @@ -2596,7 +2596,7 @@ sub rebuild {
>  		# if we merged or otherwise started elsewhere, this is
>  		# how we break out of it
>  		if (($uuid ne $svn_uuid) ||
> -		    ($full_url && $url && ($url ne $full_url))) {
> +		    ($metadata_url && $url && ($url ne $metadata_url))) {
>  			next;
>  		}
>