[PATCH] Teach dcommit --mergeinfo to handle multiple lines

[PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Bryan Jacobs]

"svn dcommit --mergeinfo" replaces the svn:mergeinfo property in an
upstream SVN repository with the given text. The svn:mergeinfo
property may contain commits originating on multiple branches,
separated by newlines.

Cause space characters in the mergeinfo to be replaced by newlines,
allowing a user to create history representing multiple branches being
merged into one.

Update the corresponding documentation and add a test for the new
functionality.

Signed-off-by: Bryan Jacobs <bjacobs@woti.com>
---
 Documentation/git-svn.txt    |    5 +++--
 git-svn.perl                 |    3 +++
 t/t9158-git-svn-mergeinfo.sh |   13 +++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index ed5eca1..3ed28df 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -211,8 +211,9 @@ discouraged.
 	Add the given merge information during the dcommit
 	(e.g. `--mergeinfo="/branches/foo:1-10"`). All svn server versions can
 	store this information (as a property), and svn clients starting from
-	version 1.5 can make use of it. 'git svn' currently does not use it
-	and does not set it automatically.
+	version 1.5 can make use of it. To specify merge information from multiple
+	branches, use a single space character between the branches
+	(`--mergeinfo="/branches/foo:1-10 /branches/bar:3,5-6,8"`)
 
 'branch'::
 	Create a branch in the SVN repository.
diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..3ee26a2 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -548,6 +548,9 @@ sub cmd_dcommit {
 	}
 	my $expect_url = $url;
 	Git::SVN::remove_username($expect_url);
+	if (defined($_merge_info)) {
+		$_merge_info =~ tr{ }{\n};
+	}
 	while (1) {
 		my $d = shift @$linear_refs or last;
 		unless (defined $last_rev) {
diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh
index 3ab4390..8c9539e 100755
--- a/t/t9158-git-svn-mergeinfo.sh
+++ b/t/t9158-git-svn-mergeinfo.sh
@@ -38,4 +38,17 @@ test_expect_success 'verify svn:mergeinfo' '
 	test "$mergeinfo" = "/branches/foo:1-10"
 '
 
+test_expect_success 'change svn:mergeinfo multiline' '
+	touch baz &&
+	git add baz &&
+	git commit -m "baz" &&
+	git svn dcommit --mergeinfo="/branches/bar:1-10 /branches/other:3-5,8,10-11"
+'
+
+test_expect_success 'verify svn:mergeinfo multiline' '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk)
+	test "$mergeinfo" = "/branches/bar:1-10
+/branches/other:3-5,8,10-11"
+'
+
 test_done
-- 
1.7.6

Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Eric Wong]

Subsystem commit subjects should be prefixed with the approriate
subsystem (e.g. "git-svn: Teach dcommit --mergeinfo ..."

Bryan Jacobs <bjacobs@woti.com> wrote:
> "svn dcommit --mergeinfo" replaces the svn:mergeinfo property in an
> upstream SVN repository with the given text. The svn:mergeinfo
> property may contain commits originating on multiple branches,
> separated by newlines.
> 
> Cause space characters in the mergeinfo to be replaced by newlines,
> allowing a user to create history representing multiple branches being
> merged into one.
> 
> Update the corresponding documentation and add a test for the new
> functionality.
> 
> Signed-off-by: Bryan Jacobs <bjacobs@woti.com>

This looks reasonable, Cc:-ing Sam since he handled the mergeinfo stuff.
After all this time, I still have no experience using SVN mergeinfo
anywhere :x

> ---
>  Documentation/git-svn.txt    |    5 +++--
>  git-svn.perl                 |    3 +++
>  t/t9158-git-svn-mergeinfo.sh |   13 +++++++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index ed5eca1..3ed28df 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -211,8 +211,9 @@ discouraged.
>  	Add the given merge information during the dcommit
>  	(e.g. `--mergeinfo="/branches/foo:1-10"`). All svn server versions can
>  	store this information (as a property), and svn clients starting from
> -	version 1.5 can make use of it. 'git svn' currently does not use it
> -	and does not set it automatically.
> +	version 1.5 can make use of it. To specify merge information from multiple
> +	branches, use a single space character between the branches
> +	(`--mergeinfo="/branches/foo:1-10 /branches/bar:3,5-6,8"`)
>  
>  'branch'::
>  	Create a branch in the SVN repository.
> diff --git a/git-svn.perl b/git-svn.perl
> index 89f83fd..3ee26a2 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -548,6 +548,9 @@ sub cmd_dcommit {
>  	}
>  	my $expect_url = $url;
>  	Git::SVN::remove_username($expect_url);
> +	if (defined($_merge_info)) {
> +		$_merge_info =~ tr{ }{\n};
> +	}
>  	while (1) {
>  		my $d = shift @$linear_refs or last;
>  		unless (defined $last_rev) {
> diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh
> index 3ab4390..8c9539e 100755
> --- a/t/t9158-git-svn-mergeinfo.sh
> +++ b/t/t9158-git-svn-mergeinfo.sh
> @@ -38,4 +38,17 @@ test_expect_success 'verify svn:mergeinfo' '
>  	test "$mergeinfo" = "/branches/foo:1-10"
>  '
>  
> +test_expect_success 'change svn:mergeinfo multiline' '
> +	touch baz &&
> +	git add baz &&
> +	git commit -m "baz" &&
> +	git svn dcommit --mergeinfo="/branches/bar:1-10 /branches/other:3-5,8,10-11"
> +'
> +
> +test_expect_success 'verify svn:mergeinfo multiline' '
> +	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk)
> +	test "$mergeinfo" = "/branches/bar:1-10
> +/branches/other:3-5,8,10-11"
> +'
> +
>  test_done
> -- 
> 1.7.6

Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Sam Vilain]

On 8/31/11 1:21 PM, Eric Wong wrote:
>> --- a/Documentation/git-svn.txt
>> +++ b/Documentation/git-svn.txt
>> @@ -211,8 +211,9 @@ discouraged.
>>   	Add the given merge information during the dcommit
>>   	(e.g. `--mergeinfo="/branches/foo:1-10"`). All svn server versions can
>>   	store this information (as a property), and svn clients starting from
>> -	version 1.5 can make use of it. 'git svn' currently does not use it
>> -	and does not set it automatically.
>> +	version 1.5 can make use of it. To specify merge information from multiple
>> +	branches, use a single space character between the branches
>> +	(`--mergeinfo="/branches/foo:1-10 /branches/bar:3,5-6,8"`)

This interface seems regrettably stupid.  Like, do I need to consider 
the existing revisions that are already listed in the property?  Is it 
really impossible to derive the changes that were merged and generate 
the list automatically?

But so long as it makes something previously impossible possible, it is 
a good change - my feeling is that it should be called something like 
--mergeinfo-raw or --mergeinfo-set to leave room for a possible 
--mergeinfo-add which knows how the lists work and adds them (which is 
what I'd expect a plain --mergeinfo switch to do).

Sam

Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Bryan Jacobs]

On Wed, 31 Aug 2011 13:43:39 -0700
Sam Vilain <sam@vilain.net> wrote:

> On 8/31/11 1:21 PM, Eric Wong wrote:
> >> --- a/Documentation/git-svn.txt
> >> +++ b/Documentation/git-svn.txt
> >> @@ -211,8 +211,9 @@ discouraged.
> >>   	Add the given merge information during the dcommit
> >>   	(e.g. `--mergeinfo="/branches/foo:1-10"`). All svn
> >> server versions can store this information (as a property), and
> >> svn clients starting from
> >> -	version 1.5 can make use of it. 'git svn' currently does
> >> not use it
> >> -	and does not set it automatically.
> >> +	version 1.5 can make use of it. To specify merge
> >> information from multiple
> >> +	branches, use a single space character between the
> >> branches
> >> +	(`--mergeinfo="/branches/foo:1-10 /branches/bar:3,5-6,8"`)
> 
> This interface seems regrettably stupid.  Like, do I need to consider 
> the existing revisions that are already listed in the property?  Is
> it really impossible to derive the changes that were merged and
> generate the list automatically?

Nope, it's possible. I didn't create the original --mergeinfo
interface. I was very surprised when I first discovered it clobbered
instead of integrating - it's easy to nuke your SVN repo's ability to
merge with one careless use of this option. See below.

> But so long as it makes something previously impossible possible, it
> is a good change - my feeling is that it should be called something
> like --mergeinfo-raw or --mergeinfo-set to leave room for a possible 
> --mergeinfo-add which knows how the lists work and adds them (which
> is what I'd expect a plain --mergeinfo switch to do).

I completely agree. I think there should at least be a
--mergeinfo-update which fetches the current revision, merges that with
the provided set using the branch paths as keys (and compacts using
svn:mergeinfo rules), and sets the property to the final result.

I actually do this currently with external scripts, which is why I
wanted to make --mergeinfo capable of delivering my final payload. It
would make my life easier if all the logic were part of git-svn instead.

That said, this change is really small. That change would be larger.
So I submitted this first.

> Sam

Re: [spf:guess,iffy] Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Sam Vilain]

On 8/31/11 1:51 PM, Bryan Jacobs wrote:
> ...I didn't create the original --mergeinfo interface. I was very 
> surprised when I first discovered it clobbered instead of integrating 
> - it's easy to nuke your SVN repo's ability to merge with one careless 
> use of this option. See below.
>> But so long as it makes something previously impossible possible, it
>> is a good change - my feeling is that it should be called something
>> like --mergeinfo-raw or --mergeinfo-set to leave room for a possible
>> --mergeinfo-add which knows how the lists work and adds them (which
>> is what I'd expect a plain --mergeinfo switch to do).
> I completely agree. I think there should at least be a
> --mergeinfo-update which fetches the current revision, merges that with
> the provided set using the branch paths as keys (and compacts using
> svn:mergeinfo rules), and sets the property to the final result.
>
> I actually do this currently with external scripts, which is why I
> wanted to make --mergeinfo capable of delivering my final payload. It
> would make my life easier if all the logic were part of git-svn instead.
>
> That said, this change is really small. That change would be larger.
> So I submitted this first.
>

Ok, well I guess this is a useful intermediate feature then.  Feel free 
to copy in any further changes you may come up with in this area to me, 
if you decide to do that.

Cheers,
Sam

Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Eric Wong]

Sam Vilain <sam@vilain.net> wrote:
> Ok, well I guess this is a useful intermediate feature then.  Feel
> free to copy in any further changes you may come up with in this
> area to me, if you decide to do that.

Shall I consider this an Acked-by?

-- 
Eric Wong

Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines

[Sam Vilain]

On 31/08/11 18:37, Eric Wong wrote:
> Sam Vilain <sam@vilain.net> wrote:
>> Ok, well I guess this is a useful intermediate feature then.  Feel
>> free to copy in any further changes you may come up with in this
>> area to me, if you decide to do that.
> Shall I consider this an Acked-by?

Acked-by: Sam Vilain <sam@vilain.net>

Cheers,
Sam