{"thread":{"id":"28270","subject":"[PATCH] Teach dcommit --mergeinfo to handle multiple lines","startedAt":"2011-08-31T16:48:39Z","lastAt":"2011-09-01T04:40:35Z","messageCount":7,"participants":["Bryan Jacobs","Eric Wong","Sam Vilain"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174625","messageId":"20110831124839.69c70486@robyn.woti.com","threadId":"28270","inReplyTo":null,"subject":"[PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-08-31T16:48:39Z","receivedAt":"2011-08-31T16:48:39Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"\"svn dcommit --mergeinfo\" replaces the svn:mergeinfo property in an\nupstream SVN repository with the given text. The svn:mergeinfo\nproperty may contain commits originating on multiple branches,\nseparated by newlines.\n\nCause space characters in the mergeinfo to be replaced by newlines,\nallowing a user to create history representing multiple branches being\nmerged into one.\n\nUpdate the corresponding documentation and add a test for the new\nfunctionality.\n\nSigned-off-by: Bryan Jacobs <bjacobs@woti.com>\n---\n Documentation/git-svn.txt    |    5 +++--\n git-svn.perl                 |    3 +++\n t/t9158-git-svn-mergeinfo.sh |   13 +++++++++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\nindex ed5eca1..3ed28df 100644\n--- a/Documentation/git-svn.txt\n+++ b/Documentation/git-svn.txt\n@@ -211,8 +211,9 @@ discouraged.\n \tAdd the given merge information during the dcommit\n \t(e.g. `--mergeinfo=\"/branches/foo:1-10\"`). All svn server versions can\n \tstore this information (as a property), and svn clients starting from\n-\tversion 1.5 can make use of it. 'git svn' currently does not use it\n-\tand does not set it automatically.\n+\tversion 1.5 can make use of it. To specify merge information from multiple\n+\tbranches, use a single space character between the branches\n+\t(`--mergeinfo=\"/branches/foo:1-10 /branches/bar:3,5-6,8\"`)\n \n 'branch'::\n \tCreate a branch in the SVN repository.\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 89f83fd..3ee26a2 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -548,6 +548,9 @@ sub cmd_dcommit {\n \t}\n \tmy $expect_url = $url;\n \tGit::SVN::remove_username($expect_url);\n+\tif (defined($_merge_info)) {\n+\t\t$_merge_info =~ tr{ }{\\n};\n+\t}\n \twhile (1) {\n \t\tmy $d = shift @$linear_refs or last;\n \t\tunless (defined $last_rev) {\ndiff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh\nindex 3ab4390..8c9539e 100755\n--- a/t/t9158-git-svn-mergeinfo.sh\n+++ b/t/t9158-git-svn-mergeinfo.sh\n@@ -38,4 +38,17 @@ test_expect_success 'verify svn:mergeinfo' '\n \ttest \"$mergeinfo\" = \"/branches/foo:1-10\"\n '\n \n+test_expect_success 'change svn:mergeinfo multiline' '\n+\ttouch baz &&\n+\tgit add baz &&\n+\tgit commit -m \"baz\" &&\n+\tgit svn dcommit --mergeinfo=\"/branches/bar:1-10 /branches/other:3-5,8,10-11\"\n+'\n+\n+test_expect_success 'verify svn:mergeinfo multiline' '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk)\n+\ttest \"$mergeinfo\" = \"/branches/bar:1-10\n+/branches/other:3-5,8,10-11\"\n+'\n+\n test_done\n-- \n1.7.6\n"},{"id":"174640","messageId":"20110831202131.GA27307@dcvr.yhbt.net","threadId":"28270","inReplyTo":"20110831124839.69c70486@robyn.woti.com","subject":"Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-08-31T20:21:31Z","receivedAt":"2011-08-31T20:21:31Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Subsystem commit subjects should be prefixed with the approriate\nsubsystem (e.g. \"git-svn: Teach dcommit --mergeinfo ...\"\n\nBryan Jacobs <bjacobs@woti.com> wrote:\n> \"svn dcommit --mergeinfo\" replaces the svn:mergeinfo property in an\n> upstream SVN repository with the given text. The svn:mergeinfo\n> property may contain commits originating on multiple branches,\n> separated by newlines.\n> \n> Cause space characters in the mergeinfo to be replaced by newlines,\n> allowing a user to create history representing multiple branches being\n> merged into one.\n> \n> Update the corresponding documentation and add a test for the new\n> functionality.\n> \n> Signed-off-by: Bryan Jacobs <bjacobs@woti.com>\n\nThis looks reasonable, Cc:-ing Sam since he handled the mergeinfo stuff.\nAfter all this time, I still have no experience using SVN mergeinfo\nanywhere :x\n\n> ---\n>  Documentation/git-svn.txt    |    5 +++--\n>  git-svn.perl                 |    3 +++\n>  t/t9158-git-svn-mergeinfo.sh |   13 +++++++++++++\n>  3 files changed, 19 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\n> index ed5eca1..3ed28df 100644\n> --- a/Documentation/git-svn.txt\n> +++ b/Documentation/git-svn.txt\n> @@ -211,8 +211,9 @@ discouraged.\n>  \tAdd the given merge information during the dcommit\n>  \t(e.g. `--mergeinfo=\"/branches/foo:1-10\"`). All svn server versions can\n>  \tstore this information (as a property), and svn clients starting from\n> -\tversion 1.5 can make use of it. 'git svn' currently does not use it\n> -\tand does not set it automatically.\n> +\tversion 1.5 can make use of it. To specify merge information from multiple\n> +\tbranches, use a single space character between the branches\n> +\t(`--mergeinfo=\"/branches/foo:1-10 /branches/bar:3,5-6,8\"`)\n>  \n>  'branch'::\n>  \tCreate a branch in the SVN repository.\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 89f83fd..3ee26a2 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -548,6 +548,9 @@ sub cmd_dcommit {\n>  \t}\n>  \tmy $expect_url = $url;\n>  \tGit::SVN::remove_username($expect_url);\n> +\tif (defined($_merge_info)) {\n> +\t\t$_merge_info =~ tr{ }{\\n};\n> +\t}\n>  \twhile (1) {\n>  \t\tmy $d = shift @$linear_refs or last;\n>  \t\tunless (defined $last_rev) {\n> diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh\n> index 3ab4390..8c9539e 100755\n> --- a/t/t9158-git-svn-mergeinfo.sh\n> +++ b/t/t9158-git-svn-mergeinfo.sh\n> @@ -38,4 +38,17 @@ test_expect_success 'verify svn:mergeinfo' '\n>  \ttest \"$mergeinfo\" = \"/branches/foo:1-10\"\n>  '\n>  \n> +test_expect_success 'change svn:mergeinfo multiline' '\n> +\ttouch baz &&\n> +\tgit add baz &&\n> +\tgit commit -m \"baz\" &&\n> +\tgit svn dcommit --mergeinfo=\"/branches/bar:1-10 /branches/other:3-5,8,10-11\"\n> +'\n> +\n> +test_expect_success 'verify svn:mergeinfo multiline' '\n> +\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk)\n> +\ttest \"$mergeinfo\" = \"/branches/bar:1-10\n> +/branches/other:3-5,8,10-11\"\n> +'\n> +\n>  test_done\n> -- \n> 1.7.6\n"},{"id":"174642","messageId":"4E5E9CFB.4060600@vilain.net","threadId":"28270","inReplyTo":"20110831202131.GA27307@dcvr.yhbt.net","subject":"Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-08-31T20:43:39Z","receivedAt":"2011-08-31T20:43:39Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 8/31/11 1:21 PM, Eric Wong wrote:\n>> --- a/Documentation/git-svn.txt\n>> +++ b/Documentation/git-svn.txt\n>> @@ -211,8 +211,9 @@ discouraged.\n>>   \tAdd the given merge information during the dcommit\n>>   \t(e.g. `--mergeinfo=\"/branches/foo:1-10\"`). All svn server versions can\n>>   \tstore this information (as a property), and svn clients starting from\n>> -\tversion 1.5 can make use of it. 'git svn' currently does not use it\n>> -\tand does not set it automatically.\n>> +\tversion 1.5 can make use of it. To specify merge information from multiple\n>> +\tbranches, use a single space character between the branches\n>> +\t(`--mergeinfo=\"/branches/foo:1-10 /branches/bar:3,5-6,8\"`)\n\nThis interface seems regrettably stupid.  Like, do I need to consider \nthe existing revisions that are already listed in the property?  Is it \nreally impossible to derive the changes that were merged and generate \nthe list automatically?\n\nBut so long as it makes something previously impossible possible, it is \na good change - my feeling is that it should be called something like \n--mergeinfo-raw or --mergeinfo-set to leave room for a possible \n--mergeinfo-add which knows how the lists work and adds them (which is \nwhat I'd expect a plain --mergeinfo switch to do).\n\nSam\n"},{"id":"174641","messageId":"20110831165109.0ca6373f@robyn.woti.com","threadId":"28270","inReplyTo":"4E5E9CFB.4060600@vilain.net","subject":"Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-08-31T20:51:09Z","receivedAt":"2011-08-31T20:51:09Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Wed, 31 Aug 2011 13:43:39 -0700\nSam Vilain <sam@vilain.net> wrote:\n\n> On 8/31/11 1:21 PM, Eric Wong wrote:\n> >> --- a/Documentation/git-svn.txt\n> >> +++ b/Documentation/git-svn.txt\n> >> @@ -211,8 +211,9 @@ discouraged.\n> >>   \tAdd the given merge information during the dcommit\n> >>   \t(e.g. `--mergeinfo=\"/branches/foo:1-10\"`). All svn\n> >> server versions can store this information (as a property), and\n> >> svn clients starting from\n> >> -\tversion 1.5 can make use of it. 'git svn' currently does\n> >> not use it\n> >> -\tand does not set it automatically.\n> >> +\tversion 1.5 can make use of it. To specify merge\n> >> information from multiple\n> >> +\tbranches, use a single space character between the\n> >> branches\n> >> +\t(`--mergeinfo=\"/branches/foo:1-10 /branches/bar:3,5-6,8\"`)\n> \n> This interface seems regrettably stupid.  Like, do I need to consider \n> the existing revisions that are already listed in the property?  Is\n> it really impossible to derive the changes that were merged and\n> generate the list automatically?\n\nNope, it's possible. I didn't create the original --mergeinfo\ninterface. I was very surprised when I first discovered it clobbered\ninstead of integrating - it's easy to nuke your SVN repo's ability to\nmerge with one careless use of this option. See below.\n\n> But so long as it makes something previously impossible possible, it\n> is a good change - my feeling is that it should be called something\n> like --mergeinfo-raw or --mergeinfo-set to leave room for a possible \n> --mergeinfo-add which knows how the lists work and adds them (which\n> is what I'd expect a plain --mergeinfo switch to do).\n\nI completely agree. I think there should at least be a\n--mergeinfo-update which fetches the current revision, merges that with\nthe provided set using the branch paths as keys (and compacts using\nsvn:mergeinfo rules), and sets the property to the final result.\n\nI actually do this currently with external scripts, which is why I\nwanted to make --mergeinfo capable of delivering my final payload. It\nwould make my life easier if all the logic were part of git-svn instead.\n\nThat said, this change is really small. That change would be larger.\nSo I submitted this first.\n\n> Sam\n"},{"id":"174646","messageId":"4E5EB26D.8030004@vilain.net","threadId":"28270","inReplyTo":"20110831165109.0ca6373f@robyn.woti.com","subject":"Re: [spf:guess,iffy] Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-08-31T22:15:09Z","receivedAt":"2011-08-31T22:15:09Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 8/31/11 1:51 PM, Bryan Jacobs wrote:\n> ...I didn't create the original --mergeinfo interface. I was very \n> surprised when I first discovered it clobbered instead of integrating \n> - it's easy to nuke your SVN repo's ability to merge with one careless \n> use of this option. See below.\n>> But so long as it makes something previously impossible possible, it\n>> is a good change - my feeling is that it should be called something\n>> like --mergeinfo-raw or --mergeinfo-set to leave room for a possible\n>> --mergeinfo-add which knows how the lists work and adds them (which\n>> is what I'd expect a plain --mergeinfo switch to do).\n> I completely agree. I think there should at least be a\n> --mergeinfo-update which fetches the current revision, merges that with\n> the provided set using the branch paths as keys (and compacts using\n> svn:mergeinfo rules), and sets the property to the final result.\n>\n> I actually do this currently with external scripts, which is why I\n> wanted to make --mergeinfo capable of delivering my final payload. It\n> would make my life easier if all the logic were part of git-svn instead.\n>\n> That said, this change is really small. That change would be larger.\n> So I submitted this first.\n>\n\nOk, well I guess this is a useful intermediate feature then.  Feel free \nto copy in any further changes you may come up with in this area to me, \nif you decide to do that.\n\nCheers,\nSam\n"},{"id":"174651","messageId":"20110901013734.GA31207@dcvr.yhbt.net","threadId":"28270","inReplyTo":"4E5EB26D.8030004@vilain.net","subject":"Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-09-01T01:37:35Z","receivedAt":"2011-09-01T01:37:35Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Sam Vilain <sam@vilain.net> wrote:\n> Ok, well I guess this is a useful intermediate feature then.  Feel\n> free to copy in any further changes you may come up with in this\n> area to me, if you decide to do that.\n\nShall I consider this an Acked-by?\n\n-- \nEric Wong\n"},{"id":"174659","messageId":"4E5F0CC3.6020805@vilain.net","threadId":"28270","inReplyTo":"20110901013734.GA31207@dcvr.yhbt.net","subject":"Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-09-01T04:40:35Z","receivedAt":"2011-09-01T04:40:35Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 31/08/11 18:37, Eric Wong wrote:\n> Sam Vilain <sam@vilain.net> wrote:\n>> Ok, well I guess this is a useful intermediate feature then.  Feel\n>> free to copy in any further changes you may come up with in this\n>> area to me, if you decide to do that.\n> Shall I consider this an Acked-by?\n\nAcked-by: Sam Vilain <sam@vilain.net>\n\nCheers,\nSam\n"}]}