{"thread":{"id":"29947","subject":"[PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","startedAt":"2012-03-14T16:09:04Z","lastAt":"2012-03-15T22:28:56Z","messageCount":6,"participants":["Avishay Lavie","Thomas Rast","Bryan Jacobs","Eric Wong","Sam Vilain"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"186954","messageId":"CAHkK2bpq1J2SW2P1tkFnjw5dWEr=uQrfrTUaS2J-swuKsP4kig@mail.gmail.com","threadId":"29947","inReplyTo":null,"subject":"[PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Avishay Lavie","fromEmail":"avishay.lavie@gmail.com","sentAt":"2012-03-14T16:09:04Z","receivedAt":"2012-03-14T16:09:04Z","isPatch":true,"sender":{"key":"avishay.lavie@gmail.com","avatar":"https://avatars.githubusercontent.com/u/557935?v=4"},"body":"[PATCH] git svn dcommit: avoid self-referential mergeinfo lines when\nsvn.pushmergeinfo flag is configured\n\nWhen svn.pushmergeinfo is configured, git svn dcommit tries to\nautomatically populate svn:mergeinfo properties by merging the parent\nbranch's mergeinfo into the committed one on each merge commit. This\nprocess can add self-referential mergeinfo lines, i.e. ones that\nreference the same branch being committed into (e.g. when\nreintegrating a branch to trunk after previously having merged trunk\ninto it), which are then mishandled by SVN and cause errors in mixed\nSVN/Git environments.\nFor more details, see my original report on the issue at [1].\n\nThis commit adds a step to git svn dcommit that filters out any\nmergeinfo lines referencing the target branch from the mergeinfo, thus\navoiding the problem.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/191932\n\nSigned-off-by: Avishay Lavie <avishay.lavie@gmail.com>\n---\nThis is my first time sending a patch to the group, so if I'm doing\nsomething wrong, please let me know.\n\n git-svn.perl |   15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex eeb83d3..1ed409d 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -752,6 +752,19 @@ sub populate_merge_info {\n \treturn undef;\n }\n\n+sub remove_self_referential_merge_info {\n+\treturn $_merge_info unless defined $_merge_info;\n+\n+\tmy ($_merge_info, $branchurl, $gs) = @_;\n+\tmy $rooturl = $gs->repos_root;\n+\t\n+\tunless ($branchurl =~ /^\\Q$rooturl\\E(.*)/) {\n+\t\tfatal \"URL to commit to is not under SVN root $rooturl!\";\n+\t}\n+\tmy $branchpath = $1;\n+\treturn join(\"\\n\", grep { $_ !~ m/^$branchpath\\:/ } split(/\\n/, $_merge_info));\n+}\n+\n sub cmd_dcommit {\n \tmy $head = shift;\n \tcommand_noisy(qw/update-index --refresh/);\n@@ -902,6 +915,8 @@ sub cmd_dcommit {\n \t\t\t\t                             $uuid,\n \t\t\t\t                             $linear_refs,\n \t\t\t\t                             $rewritten_parent);\n+\n+\t\t\t\t$_merge_info = remove_self_referential_merge_info($_merge_info, $url, $gs);\n \t\t\t}\n\n \t\t\tmy %ed_opts = ( r => $last_rev,\n-- \n1.7.8.msysgit.0\n"},{"id":"186956","messageId":"20120314121556.44e4054a@robyn.woti.com","threadId":"29947","inReplyTo":"CAHkK2bpq1J2SW2P1tkFnjw5dWEr=uQrfrTUaS2J-swuKsP4kig@mail.gmail.com","subject":"Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2012-03-14T16:15:56Z","receivedAt":"2012-03-14T16:15:56Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"This patch looks proper to me.\n\nIt's arguable that the real bug is SVN removing the\nredundant-but-harmless mergeinfo lines, but that's neither here nor\nthere. git-svn should be as similar to the real SVN client as possible.\n\nAt my workplace we have not experienced the interoperability bug\ndescribed in the linked thread, and we work with both SVN and git-svn.\nBut it may be our SVN client does not exhibit the good/bad behavior, or\nperhaps nobody has tried the workflow which causes it to be a problem.\n\nBryan Jacobs\n\nOn Wed, 14 Mar 2012 18:09:04 +0200\nAvishay Lavie <avishay.lavie@gmail.com> wrote:\n\n> [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when\n> svn.pushmergeinfo flag is configured\n> \n> When svn.pushmergeinfo is configured, git svn dcommit tries to\n> automatically populate svn:mergeinfo properties by merging the parent\n> branch's mergeinfo into the committed one on each merge commit. This\n> process can add self-referential mergeinfo lines, i.e. ones that\n> reference the same branch being committed into (e.g. when\n> reintegrating a branch to trunk after previously having merged trunk\n> into it), which are then mishandled by SVN and cause errors in mixed\n> SVN/Git environments.\n> For more details, see my original report on the issue at [1].\n> \n> This commit adds a step to git svn dcommit that filters out any\n> mergeinfo lines referencing the target branch from the mergeinfo, thus\n> avoiding the problem.\n> \n> [1] http://thread.gmane.org/gmane.comp.version-control.git/191932\n> \n> Signed-off-by: Avishay Lavie <avishay.lavie@gmail.com>\n> ---\n> This is my first time sending a patch to the group, so if I'm doing\n> something wrong, please let me know.\n> \n>  git-svn.perl |   15 +++++++++++++++\n>  1 files changed, 15 insertions(+), 0 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index eeb83d3..1ed409d 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -752,6 +752,19 @@ sub populate_merge_info {\n>  \treturn undef;\n>  }\n> \n> +sub remove_self_referential_merge_info {\n> +\treturn $_merge_info unless defined $_merge_info;\n> +\n> +\tmy ($_merge_info, $branchurl, $gs) = @_;\n> +\tmy $rooturl = $gs->repos_root;\n> +\t\n> +\tunless ($branchurl =~ /^\\Q$rooturl\\E(.*)/) {\n> +\t\tfatal \"URL to commit to is not under SVN root\n> $rooturl!\";\n> +\t}\n> +\tmy $branchpath = $1;\n> +\treturn join(\"\\n\", grep { $_ !~ m/^$branchpath\\:/ }\n> split(/\\n/, $_merge_info)); +}\n> +\n>  sub cmd_dcommit {\n>  \tmy $head = shift;\n>  \tcommand_noisy(qw/update-index --refresh/);\n> @@ -902,6 +915,8 @@ sub cmd_dcommit {\n>  \t\t\t\t                             $uuid,\n>  \t\t\t\t                             $linear_refs,\n>  \t\t\t\t                             $rewritten_parent);\n> +\n> +\t\t\t\t$_merge_info =\n> remove_self_referential_merge_info($_merge_info, $url, $gs); }\n> \n>  \t\t\tmy %ed_opts = ( r => $last_rev,\n"},{"id":"186955","messageId":"87r4wvcft3.fsf@thomas.inf.ethz.ch","threadId":"29947","inReplyTo":"CAHkK2bpq1J2SW2P1tkFnjw5dWEr=uQrfrTUaS2J-swuKsP4kig@mail.gmail.com","subject":"Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-03-14T16:23:20Z","receivedAt":"2012-03-14T16:23:20Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Avishay Lavie <avishay.lavie@gmail.com> writes:\n\n> For more details, see my original report on the issue at [1].\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/191932\n\nYou could replace this, which is annoying to look up and tedious to\nverify, with a test (perhaps in t/t9151-svn-mergeinfo.sh) that checks\nthat your reported scenario acts correctly.\n\n(I'm afraid I can't say anything about the problem itself, as I do not\nhave a use for the mergeinfo feature...)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"187060","messageId":"20120315220242.GA9348@dcvr.yhbt.net","threadId":"29947","inReplyTo":"CAHkK2bpq1J2SW2P1tkFnjw5dWEr=uQrfrTUaS2J-swuKsP4kig@mail.gmail.com","subject":"Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-03-15T22:02:42Z","receivedAt":"2012-03-15T22:02:42Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Avishay Lavie <avishay.lavie@gmail.com> wrote:\n> [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when\n> svn.pushmergeinfo flag is configured\n\nSubject line is too long, ~50 chars is the limit.\nSee git-commit(1) / Documentation/SubmittingPatches\n\n> When svn.pushmergeinfo is configured, git svn dcommit tries to\n> automatically populate svn:mergeinfo properties by merging the parent\n> branch's mergeinfo into the committed one on each merge commit. This\n> process can add self-referential mergeinfo lines, i.e. ones that\n> reference the same branch being committed into (e.g. when\n> reintegrating a branch to trunk after previously having merged trunk\n> into it), which are then mishandled by SVN and cause errors in mixed\n> SVN/Git environments.\n> For more details, see my original report on the issue at [1].\n> \n> This commit adds a step to git svn dcommit that filters out any\n> mergeinfo lines referencing the target branch from the mergeinfo, thus\n> avoiding the problem.\n> \n> [1] http://thread.gmane.org/gmane.comp.version-control.git/191932\n> \n> Signed-off-by: Avishay Lavie <avishay.lavie@gmail.com>\n> ---\n> This is my first time sending a patch to the group, so if I'm doing\n> something wrong, please let me know.\n\nNoted :)\n\nSam Vilain should be Cc:-ed on mergeinfo-related stuff.  I don't know my\nway around mergeinfo stuff at all.\n\nThis test breaks t9161-git-svn-mergeinfo-push.sh:\n\n  not ok - 12 check reintegration mergeinfo\n  #\n  #               mergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb4)\n  #               test \"$mergeinfo\" = \"/branches/svnb1:2-4,7-9,13-18\n  #       /branches/svnb2:3,8,16-17\n  #       /branches/svnb3:4,9\n  #       /branches/svnb4:5-6,10-12\n  #       /branches/svnb5:6,11\"\n\nBe sure tests run successfully before submitting patches (or ask\nfor help fixing tests).\n\nLastly, formatting: some lines are too long (80 columns max) and\nthere's trailing whitespace.\n\n>  git-svn.perl |   15 +++++++++++++++\n>  1 files changed, 15 insertions(+), 0 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index eeb83d3..1ed409d 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -752,6 +752,19 @@ sub populate_merge_info {\n>  \treturn undef;\n>  }\n> \n> +sub remove_self_referential_merge_info {\n> +\treturn $_merge_info unless defined $_merge_info;\n> +\n> +\tmy ($_merge_info, $branchurl, $gs) = @_;\n> +\tmy $rooturl = $gs->repos_root;\n> +\t\n> +\tunless ($branchurl =~ /^\\Q$rooturl\\E(.*)/) {\n> +\t\tfatal \"URL to commit to is not under SVN root $rooturl!\";\n> +\t}\n> +\tmy $branchpath = $1;\n> +\treturn join(\"\\n\", grep { $_ !~ m/^$branchpath\\:/ } split(/\\n/, $_merge_info));\n> +}\n> +\n>  sub cmd_dcommit {\n>  \tmy $head = shift;\n>  \tcommand_noisy(qw/update-index --refresh/);\n> @@ -902,6 +915,8 @@ sub cmd_dcommit {\n>  \t\t\t\t                             $uuid,\n>  \t\t\t\t                             $linear_refs,\n>  \t\t\t\t                             $rewritten_parent);\n> +\n> +\t\t\t\t$_merge_info = remove_self_referential_merge_info($_merge_info, $url, $gs);\n>  \t\t\t}\n> \n>  \t\t\tmy %ed_opts = ( r => $last_rev,\n> -- \n> 1.7.8.msysgit.0\n"},{"id":"187061","messageId":"20120315180703.7c9c0629@robyn.woti.com","threadId":"29947","inReplyTo":"20120315220242.GA9348@dcvr.yhbt.net","subject":"Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2012-03-15T22:07:03Z","receivedAt":"2012-03-15T22:07:03Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Thu, 15 Mar 2012 22:02:42 +0000\nEric Wong <normalperson@yhbt.net> wrote:\n\n> \n> This test breaks t9161-git-svn-mergeinfo-push.sh:\n> \n>   not ok - 12 check reintegration mergeinfo\n>   #\n>   #               mergeinfo=$(svn_cmd propget svn:mergeinfo\n> \"$svnrepo\"/branches/svnb4) #               test \"$mergeinfo\" =\n> \"/branches/svnb1:2-4,7-9,13-18 #       /branches/svnb2:3,8,16-17\n>   #       /branches/svnb3:4,9\n>   #       /branches/svnb4:5-6,10-12\n>   #       /branches/svnb5:6,11\"\n> \n> Be sure tests run successfully before submitting patches (or ask\n> for help fixing tests).\n\nThe test is demonstrating the behavior the patch fixes.\n\nA merge is made to \"svnb4\", and then the test checks that the mergeinfo\ncontains \"svnb4:5-6,10-12\". So really this test could be adapted to\nprove that the patch is functioning as intended :-).\n\nBryan Jacobs\n"},{"id":"187065","messageId":"4F626D28.70100@vilain.net","threadId":"29947","inReplyTo":"20120315220242.GA9348@dcvr.yhbt.net","subject":"Re: [spf:guess] Re: [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when svn.pushmergeinfo is configured","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2012-03-15T22:28:56Z","receivedAt":"2012-03-15T22:28:56Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 3/15/12 3:02 PM, Eric Wong wrote:\n> Avishay Lavie<avishay.lavie@gmail.com>  wrote:\n>> [PATCH] git svn dcommit: avoid self-referential mergeinfo lines when\n>> svn.pushmergeinfo flag is configured\n>\n> Subject line is too long, ~50 chars is the limit.\n> See git-commit(1) / Documentation/SubmittingPatches\n\nCan I suggest:\n\ngit svn dcommit: avoid self-referential mergeinfo\n"}]}