public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Eric Wong <normalperson@yhbt•net>
To: Avishay Lavie <avishay.lavie@gmail•com>
Cc: git@vger•kernel.org, Bryan Jacobs <bjacobs@woti•com>,
	Sam Vilain <sam@vilain•net>
Subject: Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
Date: Thu, 15 Mar 2012 22:02:42 +0000	[thread overview]
Message-ID: <20120315220242.GA9348@dcvr.yhbt.net> (raw)
In-Reply-To: <CAHkK2bpq1J2SW2P1tkFnjw5dWEr=uQrfrTUaS2J-swuKsP4kig@mail.gmail.com>

Avishay Lavie <avishay.lavie@gmail•com> wrote:
> [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when
> svn.pushmergeinfo flag is configured

Subject line is too long, ~50 chars is the limit.
See git-commit(1) / Documentation/SubmittingPatches

> 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.

Noted :)

Sam Vilain should be Cc:-ed on mergeinfo-related stuff.  I don't know my
way around mergeinfo stuff at all.

This test breaks t9161-git-svn-mergeinfo-push.sh:

  not ok - 12 check reintegration mergeinfo
  #
  #               mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4)
  #               test "$mergeinfo" = "/branches/svnb1:2-4,7-9,13-18
  #       /branches/svnb2:3,8,16-17
  #       /branches/svnb3:4,9
  #       /branches/svnb4:5-6,10-12
  #       /branches/svnb5:6,11"

Be sure tests run successfully before submitting patches (or ask
for help fixing tests).

Lastly, formatting: some lines are too long (80 columns max) and
there's trailing whitespace.

>  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,
> -- 
> 1.7.8.msysgit.0

  parent reply	other threads:[~2012-03-15 22:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-14 16:09 [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured Avishay Lavie
2012-03-14 16:15 ` Bryan Jacobs
2012-03-14 16:23 ` Thomas Rast
2012-03-15 22:02 ` Eric Wong [this message]
2012-03-15 22:07   ` Bryan Jacobs
2012-03-15 22:28   ` [spf:guess] " Sam Vilain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120315220242.GA9348@dcvr.yhbt.net \
    --to=normalperson@yhbt$(echo .)net \
    --cc=avishay.lavie@gmail$(echo .)com \
    --cc=bjacobs@woti$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=sam@vilain$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox