[PATCH v4] git-svn: add support for prefixed globs in config

[PATCH v4] git-svn: add support for prefixed globs in config

[Victor Leschuk]

Introduce prefixed globs for branches and tags in git-svn.
Globs like 'release_*' allow users to avoid long lines in config like:

branches = branches/{release_20,release_21,release_22,...}

Signed-off-by: Victor Leschuk <vleschuk@accesssoftek.com>
---
Changes from v3:
	* Wrapped all test preparations in separate test-cases

 Documentation/git-svn.txt        |   5 ++
 perl/Git/SVN/GlobSpec.pm         |   9 ++-
 t/t9168-git-svn-prefixed-glob.sh | 142 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 151 insertions(+), 5 deletions(-)
 create mode 100755 t/t9168-git-svn-prefixed-glob.sh

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 0c0f60b..529cffe 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -1034,6 +1034,7 @@ listed below are allowed:
 	url = http://server.org/svn
 	fetch = trunk/project-a:refs/remotes/project-a/trunk
 	branches = branches/*/project-a:refs/remotes/project-a/branches/*
+	branches = branches/release_*:refs/remotes/project-a/branches/release_*
 	tags = tags/*/project-a:refs/remotes/project-a/tags/*
 ------------------------------------------------------------------------
 
@@ -1044,6 +1045,10 @@ independent path component (surrounded by '/' or EOL).   This
 type of configuration is not automatically created by 'init' and
 should be manually entered with a text-editor or using 'git config'.
 
+Also note that prefixed globs (e.g. 'release_*') match everything after prefix
+but do not match exact prefix. For example:
+'release_*' will match 'release_1' or 'release_v1' but will not match 'release_'.
+
 It is also possible to fetch a subset of branches or tags by using a
 comma-separated list of names within braces. For example:
 
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index c95f5d7..a136090 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -11,16 +11,15 @@ sub new {
 	my $die_msg = "Only one set of wildcard directories " .
 				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
 	for my $part (split(m|/|, $glob)) {
-		if ($part =~ /\*/ && $part ne "*") {
-			die "Invalid pattern in '$glob': $part\n";
-		} elsif ($pattern_ok && $part =~ /[{}]/ &&
+		if ($pattern_ok && $part =~ /[{}]/ &&
 			 $part !~ /^\{[^{}]+\}/) {
 			die "Invalid pattern in '$glob': $part\n";
 		}
-		if ($part eq "*") {
+		if ($part =~ /(\w*)\*/) {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
-			push(@patterns, "[^/]*");
+			my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+			push(@patterns, $pat);
 		} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
diff --git a/t/t9168-git-svn-prefixed-glob.sh b/t/t9168-git-svn-prefixed-glob.sh
new file mode 100755
index 0000000..1b08e45
--- /dev/null
+++ b/t/t9168-git-svn-prefixed-glob.sh
@@ -0,0 +1,142 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+test_expect_success 'prepare test refspec prefixed globbing' '
+	cat >expect.end <<EOF
+the end
+hi
+start a new branch
+initial
+EOF
+	'
+
+test_expect_success 'test refspec prefixed globbing' '
+	mkdir -p trunk/src/a trunk/src/b trunk/doc &&
+	echo "hello world" >trunk/src/a/readme &&
+	echo "goodbye world" >trunk/src/b/readme &&
+	svn_cmd import -m "initial" trunk "$svnrepo"/trunk &&
+	svn_cmd co "$svnrepo" tmp &&
+	(
+		cd tmp &&
+		mkdir branches tags &&
+		svn_cmd add branches tags &&
+		svn_cmd cp trunk branches/b_start &&
+		svn_cmd commit -m "start a new branch" &&
+		svn_cmd up &&
+		echo "hi" >>branches/b_start/src/b/readme &&
+		poke branches/b_start/src/b/readme &&
+		echo "hey" >>branches/b_start/src/a/readme &&
+		poke branches/b_start/src/a/readme &&
+		svn_cmd commit -m "hi" &&
+		svn_cmd up &&
+		svn_cmd cp branches/b_start tags/t_end &&
+		echo "bye" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		echo "aye" >>tags/t_end/src/a/readme &&
+		poke tags/t_end/src/a/readme &&
+		svn_cmd commit -m "the end" &&
+		echo "byebye" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "nothing to see here"
+	) &&
+	git config --add svn-remote.svn.url "$svnrepo" &&
+	git config --add svn-remote.svn.fetch \
+	                 "trunk/src/a:refs/remotes/trunk" &&
+	git config --add svn-remote.svn.branches \
+	                 "branches/b_*/src/a:refs/remotes/branches/b_*" &&
+	git config --add svn-remote.svn.tags\
+	                 "tags/t_*/src/a:refs/remotes/tags/t_*" &&
+	git svn multi-fetch &&
+	git log --pretty=oneline refs/remotes/tags/t_end | \
+	    sed -e "s/^.\{41\}//" >output.end &&
+	test_cmp expect.end output.end &&
+	test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
+		"$(git rev-parse refs/remotes/branches/b_start)" &&
+	test "$(git rev-parse refs/remotes/branches/b_start~2)" = \
+		"$(git rev-parse refs/remotes/trunk)" &&
+	test_must_fail git rev-parse refs/remotes/tags/t_end@3
+	'
+
+test_expect_success 'prepare test left-hand-side only prefixed globbing' '
+	echo try to try >expect.two &&
+	echo nothing to see here >>expect.two &&
+	cat expect.end >>expect.two
+	'
+
+test_expect_success 'test left-hand-side only prefixed globbing' '
+	git config --add svn-remote.two.url "$svnrepo" &&
+	git config --add svn-remote.two.fetch trunk:refs/remotes/two/trunk &&
+	git config --add svn-remote.two.branches \
+	                 "branches/b_*:refs/remotes/two/branches/*" &&
+	git config --add svn-remote.two.tags \
+	                 "tags/t_*:refs/remotes/two/tags/*" &&
+	(
+		cd tmp &&
+		echo "try try" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "try to try"
+	) &&
+	git svn fetch two &&
+	test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
+	test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+	test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
+	     $(git rev-parse refs/remotes/two/trunk) &&
+	test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
+	     $(git rev-parse refs/remotes/two/branches/b_start) &&
+	git log --pretty=oneline refs/remotes/two/tags/t_end | \
+	    sed -e "s/^.\{41\}//" >output.two &&
+	test_cmp expect.two output.two
+	'
+
+test_expect_success 'test prefixed globs do not match just prefix' '
+	git config --add svn-remote.three.url "$svnrepo" &&
+	git config --add svn-remote.three.fetch \
+	                 trunk:refs/remotes/three/trunk &&
+	git config --add svn-remote.three.branches \
+	                 "branches/b_*:refs/remotes/three/branches/*" &&
+	git config --add svn-remote.three.tags \
+	                 "tags/t_*:refs/remotes/three/tags/*" &&
+	(
+		cd tmp &&
+		svn_cmd cp trunk branches/b_ &&
+		echo "You should never see me" >>branches/b_/src/a/readme &&
+		poke branches/b_/src/a/readme &&
+		svn_cmd commit -m "Never seen branch commit" &&
+		svn_cmd up &&
+		svn_cmd cp branches/b_ tags/t_ &&
+		echo "You should never see mee too" >>tags/t_/src/a/readme &&
+		poke tags/t_/src/a/readme &&
+		svn_cmd commit -m "Never seen tag commit" &&
+		svn_cmd up
+	) &&
+	git svn fetch three &&
+	test_path_is_missing refs/remotes/three/branches/b_ &&
+	test_path_is_missing refs/remotes/three/tags/t_
+	'
+
+test_expect_success 'prepare test dissallow prefixed multi-globs' "
+	echo \"Only one set of wildcard directories\" \
+	     \"(e.g. '*' or '*/*/*') is supported: 'branches/b_*/t/*'\" >expect.four &&
+	echo \"\" >>expect.four
+	"
+
+test_expect_success 'test disallow prefixed multi-globs' '
+	git config --add svn-remote.four.url "$svnrepo" &&
+	git config --add svn-remote.four.fetch \
+	                 trunk:refs/remotes/four/trunk &&
+	git config --add svn-remote.four.branches \
+	                 "branches/b_*/t/*:refs/remotes/four/branches/*" &&
+	git config --add svn-remote.four.tags \
+	                 "tags/t_*/*:refs/remotes/four/tags/*" &&
+	(
+		cd tmp &&
+		echo "try try" >>tags/t_end/src/b/readme &&
+		poke tags/t_end/src/b/readme &&
+		svn_cmd commit -m "try to try"
+	) &&
+	test_must_fail git svn fetch four 2>stderr.four &&
+	test_cmp expect.four stderr.four
+	'
+
+test_done
-- 
2.7.0.rc0.21.gb793f61

Re: [PATCH v4] git-svn: add support for prefixed globs in config

[Junio C Hamano]

Victor Leschuk <vleschuk@gmail.com> writes:

> Introduce prefixed globs for branches and tags in git-svn.
> Globs like 'release_*' allow users to avoid long lines in config like:
>
> branches = branches/{release_20,release_21,release_22,...}
>
> Signed-off-by: Victor Leschuk <vleschuk@accesssoftek.com>
> ---
> Changes from v3:
> 	* Wrapped all test preparations in separate test-cases
>
>  Documentation/git-svn.txt        |   5 ++
>  perl/Git/SVN/GlobSpec.pm         |   9 ++-
>  t/t9168-git-svn-prefixed-glob.sh | 142 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 151 insertions(+), 5 deletions(-)
>  create mode 100755 t/t9168-git-svn-prefixed-glob.sh
>
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 0c0f60b..529cffe 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -1034,6 +1034,7 @@ listed below are allowed:
>  	url = http://server.org/svn
>  	fetch = trunk/project-a:refs/remotes/project-a/trunk
>  	branches = branches/*/project-a:refs/remotes/project-a/branches/*
> +	branches = branches/release_*:refs/remotes/project-a/branches/release_*

Hmph, if you are going this route, I wonder if there is a reason to
limit yourself only to "prefix".  Would allowing something like this:

	branches = branches/pre*post:refs/remotes/project-a/branches/*

i.e., loosening the rule to allow at most one asterisk anywhere on
the left hand side of the colon, and require the same number of
asterisks as the left hand side has on the right hand side of the
colon, be too lax and hurt the users?

Re: [PATCH v4] git-svn: add support for prefixed globs in config

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Victor Leschuk <vleschuk@gmail.com> writes:
> > +	branches = branches/release_*:refs/remotes/project-a/branches/release_*
> 
> Hmph, if you are going this route, I wonder if there is a reason to
> limit yourself only to "prefix".  Would allowing something like this:
> 
> 	branches = branches/pre*post:refs/remotes/project-a/branches/*
> 
> i.e., loosening the rule to allow at most one asterisk anywhere on
> the left hand side of the colon, and require the same number of
> asterisks as the left hand side has on the right hand side of the
> colon, be too lax and hurt the users?

Seems doable.  Not sure about the consequences, yet...

I also think the $1 truthiness check was unnecessary and even
problematic if we need to encounter a "0" as a path component.
And using the path component will need to be quoted as we do below
with the brace case (showing with diff -U6)

Perhaps this? (untested)

diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index a136090..7961a78 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -12,16 +12,17 @@ sub new {
 				"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
 	for my $part (split(m|/|, $glob)) {
 		if ($pattern_ok && $part =~ /[{}]/ &&
 			 $part !~ /^\{[^{}]+\}/) {
 			die "Invalid pattern in '$glob': $part\n";
 		}
-		if ($part =~ /(\w*)\*/) {
+		if ($part =~ /(\w*)\*(\w*)/) {
+			my ($l, $r) = ($1, $2);
 			die $die_msg if $state eq "right";
 			$state = "pattern";
-			my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+			my $pat = quotemeta($l) . '[^/]*'. quotemeta($r);
 			push(@patterns, $pat);
 		} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
 			my $p = quotemeta($1);
 			$p =~ s/\\,/|/g;
-- 
EW

RE: [PATCH v4] git-svn: add support for prefixed globs in config

[Victor Leschuk]

Hello Eric and Junio,

Thanks a lot for the comments and sorry for the delay with the response.

I think that's reasonable to widen the applicability of globs, will do it and prepare next patch.


-                       my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+                       my $pat = quotemeta($l) . '[^/]*'. quotemeta($r);

Hm, that differs from the behavior I suggested at first time: in this case the 'test*' pattern will match all of the: "test", "test0", "test1". In my case it wouldn't match "test"...

However I think that's ok.