Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured

[Bryan Jacobs <bjacobs@woti.com>]

This patch looks proper to me.

It's arguable that the real bug is SVN removing the
redundant-but-harmless mergeinfo lines, but that's neither here nor
there. git-svn should be as similar to the real SVN client as possible.

At my workplace we have not experienced the interoperability bug
described in the linked thread, and we work with both SVN and git-svn.
But it may be our SVN client does not exhibit the good/bad behavior, or
perhaps nobody has tried the workflow which causes it to be a problem.

Bryan Jacobs

On Wed, 14 Mar 2012 18:09:04 +0200
Avishay Lavie <avishay.lavie@gmail.com> wrote:

> [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when
> svn.pushmergeinfo flag is configured
> 
> When svn.pushmergeinfo is configured, git svn dcommit tries to
> automatically populate svn:mergeinfo properties by merging the parent
> branch's mergeinfo into the committed one on each merge commit. This
> process can add self-referential mergeinfo lines, i.e. ones that
> reference the same branch being committed into (e.g. when
> reintegrating a branch to trunk after previously having merged trunk
> into it), which are then mishandled by SVN and cause errors in mixed
> SVN/Git environments.
> For more details, see my original report on the issue at [1].
> 
> This commit adds a step to git svn dcommit that filters out any
> mergeinfo lines referencing the target branch from the mergeinfo, thus
> avoiding the problem.
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/191932
> 
> Signed-off-by: Avishay Lavie <avishay.lavie@gmail.com>
> ---
> This is my first time sending a patch to the group, so if I'm doing
> something wrong, please let me know.
> 
>  git-svn.perl |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index eeb83d3..1ed409d 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -752,6 +752,19 @@ sub populate_merge_info {
>  	return undef;
>  }
> 
> +sub remove_self_referential_merge_info {
> +	return $_merge_info unless defined $_merge_info;
> +
> +	my ($_merge_info, $branchurl, $gs) = @_;
> +	my $rooturl = $gs->repos_root;
> +	
> +	unless ($branchurl =~ /^\Q$rooturl\E(.*)/) {
> +		fatal "URL to commit to is not under SVN root
> $rooturl!";
> +	}
> +	my $branchpath = $1;
> +	return join("\n", grep { $_ !~ m/^$branchpath\:/ }
> split(/\n/, $_merge_info)); +}
> +
>  sub cmd_dcommit {
>  	my $head = shift;
>  	command_noisy(qw/update-index --refresh/);
> @@ -902,6 +915,8 @@ sub cmd_dcommit {
>  				                             $uuid,
>  				                             $linear_refs,
>  				                             $rewritten_parent);
> +
> +				$_merge_info =
> remove_self_referential_merge_info($_merge_info, $url, $gs); }
> 
>  			my %ed_opts = ( r => $last_rev,