[PATCH] git-svn: mangle refnames forbidden in git

[PATCH] git-svn: mangle refnames forbidden in git

[Torsten Schmutzler]

git-check-ref-format(1) describes names which
cannot be used as refnames for git.  Some are
legal branchnames in subversion however.
Mangle the not yet handled cases.

Signed-off-by: Torsten Schmutzler <git-ts@theblacksun.eu>
---
 git-svn.perl                          |    8 ++++++++
 t/t9118-git-svn-funky-branch-names.sh |   21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 2c86ea2..57df509 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2086,6 +2086,14 @@ sub refname {
 	# .. becomes %2E%2E
 	$refname =~ s{\.\.}{%2E%2E}g;
 
+	# trailing dots and .lock are not allowed
+	# .$ becomes %2E and .lock becomes %2Elock
+	$refname =~ s{\.(?=$|lock$)}{%2E};
+
+	# the sequence @{ is used to access the reflog
+	# @{ becomes %40{
+	$refname =~ s{\@\{}{%40\{}g;
+
 	return $refname;
 }
 
diff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh
index ac52bff..7d7acc3 100755
--- a/t/t9118-git-svn-funky-branch-names.sh
+++ b/t/t9118-git-svn-funky-branch-names.sh
@@ -21,6 +21,14 @@ test_expect_success 'setup svnrepo' '
 	                      "$svnrepo/pr ject/branches/more fun plugin!" &&
 	svn_cmd cp -m "scary" "$svnrepo/pr ject/branches/fun plugin" \
 	              "$svnrepo/pr ject/branches/$scary_uri" &&
+	svn_cmd cp -m "leading dot" "$svnrepo/pr ject/trunk" \
+			"$svnrepo/pr ject/branches/.leading_dot" &&
+	svn_cmd cp -m "trailing dot" "$svnrepo/pr ject/trunk" \
+			"$svnrepo/pr ject/branches/trailing_dot." &&
+	svn_cmd cp -m "trailing .lock" "$svnrepo/pr ject/trunk" \
+			"$svnrepo/pr ject/branches/trailing_dotlock.lock" &&
+	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
+			"$svnrepo/pr ject/branches/not-a@{0}reflog" &&
 	start_httpd
 	'
 
@@ -30,6 +38,10 @@ test_expect_success 'test clone with funky branch names' '
 		git rev-parse "refs/remotes/fun%20plugin" &&
 		git rev-parse "refs/remotes/more%20fun%20plugin!" &&
 		git rev-parse "refs/remotes/$scary_ref" &&
+		git rev-parse "refs/remotes/%2Eleading_dot" &&
+		git rev-parse "refs/remotes/trailing_dot%2E" &&
+		git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
+		git rev-parse "refs/remotes/not-a%40{0}reflog" &&
 	cd ..
 	'
 
@@ -51,6 +63,15 @@ test_expect_success 'test dcommit to scary branch' '
 	cd ..
 	'
 
+test_expect_success 'test dcommit to trailing_dotlock branch' '
+	cd project &&
+	git reset --hard "refs/remotes/trailing_dotlock%2Elock" &&
+	echo who names branches like this anyway? >> foo &&
+	git commit -m "bar" -- foo &&
+	git svn dcommit &&
+	cd ..
+	'
+
 stop_httpd
 
 test_done
-- 
1.7.1.12.ge45de

Re: [PATCH] git-svn: mangle refnames forbidden in git

[Eric Wong]

Torsten Schmutzler <git-ts@theblacksun.eu> wrote:
> git-check-ref-format(1) describes names which
> cannot be used as refnames for git.  Some are
> legal branchnames in subversion however.
> Mangle the not yet handled cases.
> 
> Signed-off-by: Torsten Schmutzler <git-ts@theblacksun.eu>

Thanks,

Acked-by: Eric Wong <normalperson@yhbt.net>

...and pushed out to git://git.bogomips.org/git-svn

-- 
Eric Wong

Re: [PATCH] git-svn: mangle refnames forbidden in git

[Jonathan Nieder]

t9118-git-svn-funky-branch-names.sh fails for me:

 [...]
 3402a8497588142722deb77f6e4f42b6caaabc85
 refs/remotes/not-a%40{0}reflog
 fatal: ambiguous argument 'refs/remotes/not-a%40{0}reflog': unknown revision or path not in the working tree.
 Use '--' to separate paths from revisions
 not ok - 2 test clone with funky branch names

More precisely, the failure occurs in the

 	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/not-a@{0}reflog" &&

line in that test: I think svn is handling the @ itself here.

 $ svnadmin create /tmp/foo
 $ svn checkout file:///tmp/foo wc
 Checked out revision 0.
 $ echo hello >wc/source
 $ (cd wc && svn add source)
 A         source
 $ (cd wc && svn commit -m one)
 Adding         source
 Transmitting file data .
 Committed revision 1.
 $ svn cp -m reflog file:///tmp/foo/source 'file:///tmp/foo/not-a@{0}reflog'
 Committed revision 2.
 $ (cd wc && svn update)
 A    not-a
 Updated to revision 2.

 $ svn --version
 svn, version 1.6.12 (r955767)
    compiled Jun 21 2010, 20:24:54
 [...]

Ideas?

Re: [PATCH] git-svn: mangle refnames forbidden in git

[Eric Wong]

Jonathan Nieder <jrnieder@gmail.com> wrote:
> t9118-git-svn-funky-branch-names.sh fails for me:
> 
>  [...]
>  3402a8497588142722deb77f6e4f42b6caaabc85
>  refs/remotes/not-a%40{0}reflog
>  fatal: ambiguous argument 'refs/remotes/not-a%40{0}reflog': unknown revision or path not in the working tree.
>  Use '--' to separate paths from revisions
>  not ok - 2 test clone with funky branch names
> 
> More precisely, the failure occurs in the
> 
>  	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
> 			"$svnrepo/pr ject/branches/not-a@{0}reflog" &&
> 
> line in that test: I think svn is handling the @ itself here.
<snip>
>  $ svn --version
>  svn, version 1.6.12 (r955767)
>     compiled Jun 21 2010, 20:24:54
>  [...]
> 
> Ideas?

I can't reproduce it with 1.5.1 here, does URI-escaping the "@{0}"
to "%40%7B0%7D" work?

-- 
Eric Wong

Re: [PATCH] git-svn: mangle refnames forbidden in git

[Jonathan Nieder]

Eric Wong wrote:

> I can't reproduce it with 1.5.1 here, does URI-escaping the "@{0}"
> to "%40%7B0%7D" work?

Yes.

 ...
 $ svn cp -m reflog file:///tmp/foo/source 'file:///tmp/foo/not-a%40%7B0%7Dreflog'
 Committed revision 2.
 $ (cd wc && svn update)
 A    not-a@{0}reflog
 Updated to revision 2.

Does that work with svn 1.5.x?

Re: [PATCH] git-svn: mangle refnames forbidden in git

[Michael J Gruber]

Jonathan Nieder venit, vidit, dixit 08.07.2010 15:03:
> Eric Wong wrote:
> 
>> I can't reproduce it with 1.5.1 here, does URI-escaping the "@{0}"
>> to "%40%7B0%7D" work?
> 
> Yes.
> 
>  ...
>  $ svn cp -m reflog file:///tmp/foo/source 'file:///tmp/foo/not-a%40%7B0%7Dreflog'
>  Committed revision 2.
>  $ (cd wc && svn update)
>  A    not-a@{0}reflog
>  Updated to revision 2.
> 
> Does that work with svn 1.5.x?

It turns out this is not so much about general escaping but about the
treatment of so called svn PEG revisions (not so unsimilar to our @{}).

svn r954995 integrated r952973 (which implemented the eating of PEG
revisions) into the 1.6.x branch. It's considered a bug fix.

This "fix" is in 1.6.12 but not in 1.6.11.

Now, git-svn adjusts its behaviour to the svn version for things like
info. But I don't know if it's worth doing in this case - maybe we
should just change the test not to use the PEG revisions identifier "@".

Michael

[PATCH] t9118: avoid PEG revision identifier in tests

[Michael J Gruber]

@ is SVN's identifier for PEG revisions. But SVN's treatment of PEG
identifiers in copy target URLs changed in r954995/r952973, i.e. between
1.6.11 and 1.6.12. They get eaten now (which is considered the right
way).

Therefore, avoid the @ in the tests with funky branch names.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This version is tested with svn 1.6.9 and 1.6.12.

 t/t9118-git-svn-funky-branch-names.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh
index 7d7acc3..45d5b2d 100755
--- a/t/t9118-git-svn-funky-branch-names.sh
+++ b/t/t9118-git-svn-funky-branch-names.sh
@@ -28,7 +28,7 @@ test_expect_success 'setup svnrepo' '
 	svn_cmd cp -m "trailing .lock" "$svnrepo/pr ject/trunk" \
 			"$svnrepo/pr ject/branches/trailing_dotlock.lock" &&
 	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
-			"$svnrepo/pr ject/branches/not-a@{0}reflog" &&
+			"$svnrepo/pr ject/branches/not-a%40{0}reflog" &&
 	start_httpd
 	'
 
-- 
1.7.2.rc1.212.g850a

Re: [PATCH] t9118: avoid PEG revision identifier in tests

[A Large Angry SCM]

On 07/08/2010 03:16 PM, Michael J Gruber wrote:
> @ is SVN's identifier for PEG revisions. But SVN's treatment of PEG
> identifiers in copy target URLs changed in r954995/r952973, i.e. between
> 1.6.11 and 1.6.12. They get eaten now (which is considered the right
> way).
>
> Therefore, avoid the @ in the tests with funky branch names.
>
> Signed-off-by: Michael J Gruber<git@drmicha.warpmail.net>
> ---
> This version is tested with svn 1.6.9 and 1.6.12.
>
>   t/t9118-git-svn-funky-branch-names.sh |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh
> index 7d7acc3..45d5b2d 100755
> --- a/t/t9118-git-svn-funky-branch-names.sh
> +++ b/t/t9118-git-svn-funky-branch-names.sh
> @@ -28,7 +28,7 @@ test_expect_success 'setup svnrepo' '
>   	svn_cmd cp -m "trailing .lock" "$svnrepo/pr ject/trunk" \
>   			"$svnrepo/pr ject/branches/trailing_dotlock.lock"&&
>   	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
> -			"$svnrepo/pr ject/branches/not-a@{0}reflog"&&
> +			"$svnrepo/pr ject/branches/not-a%40{0}reflog"&&
>   	start_httpd
>   	'
>

With this patch, 'make test' now passes, and make -j4 test' does also.

Re: [PATCH] t9118: avoid PEG revision identifier in tests

[Eric Wong]

Michael J Gruber <git@drmicha.warpmail.net> wrote:
> @ is SVN's identifier for PEG revisions. But SVN's treatment of PEG
> identifiers in copy target URLs changed in r954995/r952973, i.e. between
> 1.6.11 and 1.6.12. They get eaten now (which is considered the right
> way).
> 
> Therefore, avoid the @ in the tests with funky branch names.
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> This version is tested with svn 1.6.9 and 1.6.12.

Thanks Michael, also tested with svn 1.5.1.
Acked and pushed up to git://git.bogomips.org/git-svn

-- 
Eric Wong