* [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
@ 2012-03-14 16:09 Avishay Lavie
2012-03-14 16:15 ` Bryan Jacobs
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Avishay Lavie @ 2012-03-14 16:09 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Bryan Jacobs
[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,
--
1.7.8.msysgit.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
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
2 siblings, 0 replies; 6+ messages in thread
From: Bryan Jacobs @ 2012-03-14 16:15 UTC (permalink / raw)
To: Avishay Lavie; +Cc: git, Eric Wong
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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
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
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Rast @ 2012-03-14 16:23 UTC (permalink / raw)
To: Avishay Lavie; +Cc: git, Eric Wong, Bryan Jacobs
Avishay Lavie <avishay.lavie@gmail.com> writes:
> For more details, see my original report on the issue at [1].
> [1] http://thread.gmane.org/gmane.comp.version-control.git/191932
You could replace this, which is annoying to look up and tedious to
verify, with a test (perhaps in t/t9151-svn-mergeinfo.sh) that checks
that your reported scenario acts correctly.
(I'm afraid I can't say anything about the problem itself, as I do not
have a use for the mergeinfo feature...)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
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
2012-03-15 22:07 ` Bryan Jacobs
2012-03-15 22:28 ` [spf:guess] " Sam Vilain
2 siblings, 2 replies; 6+ messages in thread
From: Eric Wong @ 2012-03-15 22:02 UTC (permalink / raw)
To: Avishay Lavie; +Cc: git, Bryan Jacobs, Sam Vilain
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
2012-03-15 22:02 ` Eric Wong
@ 2012-03-15 22:07 ` Bryan Jacobs
2012-03-15 22:28 ` [spf:guess] " Sam Vilain
1 sibling, 0 replies; 6+ messages in thread
From: Bryan Jacobs @ 2012-03-15 22:07 UTC (permalink / raw)
To: Eric Wong; +Cc: Avishay Lavie, git, Sam Vilain
On Thu, 15 Mar 2012 22:02:42 +0000
Eric Wong <normalperson@yhbt.net> wrote:
>
> 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).
The test is demonstrating the behavior the patch fixes.
A merge is made to "svnb4", and then the test checks that the mergeinfo
contains "svnb4:5-6,10-12". So really this test could be adapted to
prove that the patch is functioning as intended :-).
Bryan Jacobs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [spf:guess] Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured
2012-03-15 22:02 ` Eric Wong
2012-03-15 22:07 ` Bryan Jacobs
@ 2012-03-15 22:28 ` Sam Vilain
1 sibling, 0 replies; 6+ messages in thread
From: Sam Vilain @ 2012-03-15 22:28 UTC (permalink / raw)
To: Eric Wong; +Cc: Avishay Lavie, git, Bryan Jacobs
On 3/15/12 3:02 PM, Eric Wong wrote:
> 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
Can I suggest:
git svn dcommit: avoid self-referential mergeinfo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-15 22:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2012-03-15 22:07 ` Bryan Jacobs
2012-03-15 22:28 ` [spf:guess] " Sam Vilain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).