[PATCH/RFC] svn test: escape peg revision separator using empty peg rev
This test script uses "svn cp" to create a branch with an @-sign in
its name:
svn cp "pr ject/trunk" "pr ject/branches/not-a@{0}reflog"
That sets up for later tests that fetch the branch and check that git
svn mangles the refname appropriately.
Unfortunately, modern svn versions interpret path arguments with an
@-sign as an example of path@revision syntax (which pegs a path to a
particular revision) and truncate the path or error out with message
"svn: E205000: Syntax error parsing peg revision '{0}reflog'".
When using subversion 1.6.x, escaping the @ sign as %40 avoids trouble
(see 08fd28bb, 2010-07-08). Newer versions are stricter:
$ svn cp "$repo/pr ject/trunk" "$repo/pr ject/branches/not-a%40{reflog}"
svn: E205000: Syntax error parsing peg revision '%7B0%7Dreflog'
The recommended method for escaping a literal @ sign in a path passed
to subversion is to add an empty peg revision at the end of the path
("branches/not-a@{0}reflog@"). Do that.
Pre-1.6.12 versions of Subversion probably treat the trailing @ as
another literal @-sign (svn issue 3651). Luckily ever since
v1.8.0-rc0~155^2~7 (t9118: workaround inconsistency between SVN
versions, 2012-07-28) the test can survive that.
Tested with Debian Subversion 1.6.12dfsg-6 and 1.7.5-1 and r1395837
of Subversion trunk (1.8.x).
Signed-off-by: Jonathan Nieder
---
Michael G Schwern wrote:
> On 2012.7.28 7:16 AM, Jonathan Nieder wrote:
>> Michael G. Schwern wrote:
>>> - git rev-parse "refs/remotes/not-a%40{0}reflog"
>>> + git rev-parse "refs/remotes/$non_reflog"
>>
>> Doesn't this defeat the point of the testcase (checking that git-svn
>> is able to avoid creating git refs containing @{, following the rules
>> from git-check-ref-format(1))?
>
> Unless I messed up, entirely possible as I'm not a shell programmer, the test
> is still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be
> 'not-a%40{0}reflog' as before.
Here's a patch to make the test useful again for SVN 1.7. Sensible?
t/t9118-git-svn-funky-branch-names.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9118-git-svn-funky-branch-names.sh
b/t/t9118-git-svn-funky-branch-names.sh
index 193d3cab..15f93b4c 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%40{0}reflog" &&
+ "$svnrepo/pr ject/branches/not-a@{0}reflog@" &&
start_httpd
'
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41:
> This test script uses "svn cp" to create a branch with an @-sign in
> its name:
>
> svn cp "pr ject/trunk" "pr ject/branches/not-a@{0}reflog"
>
> That sets up for later tests that fetch the branch and check that git
> svn mangles the refname appropriately.
>
> Unfortunately, modern svn versions interpret path arguments with an
> @-sign as an example of path@revision syntax (which pegs a path to a
> particular revision) and truncate the path or error out with message
> "svn: E205000: Syntax error parsing peg revision '{0}reflog'".
>
> When using subversion 1.6.x, escaping the @ sign as %40 avoids trouble
> (see 08fd28bb, 2010-07-08). Newer versions are stricter:
>
> $ svn cp "$repo/pr ject/trunk" "$repo/pr ject/branches/not-a%40{reflog}"
> svn: E205000: Syntax error parsing peg revision '%7B0%7Dreflog'
>
> The recommended method for escaping a literal @ sign in a path passed
> to subversion is to add an empty peg revision at the end of the path
> ("branches/not-a@{0}reflog@"). Do that.
>
> Pre-1.6.12 versions of Subversion probably treat the trailing @ as
> another literal @-sign (svn issue 3651). Luckily ever since
> v1.8.0-rc0~155^2~7 (t9118: workaround inconsistency between SVN
> versions, 2012-07-28) the test can survive that.
>
> Tested with Debian Subversion 1.6.12dfsg-6 and 1.7.5-1 and r1395837
> of Subversion trunk (1.8.x).
>
> Signed-off-by: Jonathan Nieder
Tested with Subversion 1.6.18.
> ---
> Michael G Schwern wrote:
>> On 2012.7.28 7:16 AM, Jonathan Nieder wrote:
>>> Michael G. Schwern wrote:
>
- git rev-parse "refs/remotes/not-a%40{0}reflog"
+ git rev-parse "refs/remotes/$non_reflog"
>>>
>>> Doesn't this defeat the point of the testcase (checking that git-svn
>>> is able to avoid creating git refs containing @{, following the rules
>>> from git-check-ref-format(1))?
>>
>> Unless I messed up, entirely possible as I'm not a shell programmer, the test
>> is still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be
>> 'not-a%40{0}reflog' as before.
>
> Here's a patch to make the test useful again for SVN 1.7. Sensible?
>
> t/t9118-git-svn-funky-branch-names.sh |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9118-git-svn-funky-branch-names.sh
> b/t/t9118-git-svn-funky-branch-names.sh
> index 193d3cab..15f93b4c 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%40{0}reflog" &&
> + "$svnrepo/pr ject/branches/not-a@{0}reflog@" &&
> start_httpd
> '
I haven't checked other svn versions but this approach looks perfectly
sensible. It makes us test branch names which can't even be created
easily with current svn. Does svn really deserve this much attention? ;)
Seriously, our tests prepare us well for an svn remote helper...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Michael J Gruber wrote: > Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41: >> Signed-off-by: Jonathan Nieder > > Tested with Subversion 1.6.18. [...] > I haven't checked other svn versions but this approach looks perfectly > sensible. It makes us test branch names which can't even be created > easily with current svn. Does svn really deserve this much attention? ;) Thanks for the quick and thorough feedback. I'm glad to hear it seems sane. ;-) > Seriously, our tests prepare us well for an svn remote helper... That might be a good reason to make a mock implementation of the existing git-svn interface on top of git-remote-svn. Sounds fun but hard. Ciao, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Jonathan Nieder wrote: > Michael J Gruber wrote: > > Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41: > > >> Signed-off-by: Jonathan Nieder > > > > Tested with Subversion 1.6.18. Thanks both. Also pushed to "master" on git://bogomips.org/git-svn.git (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f) Jonathan Nieder (2): git svn: work around SVN 1.7 mishandling of svn:special changes svn test: escape peg revision separator using empty peg rev -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Eric Wong wrote:
> Thanks both. Also pushed to "master" on git://bogomips.org/git-svn.git
> (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)
>
> Jonathan Nieder (2):
> git svn: work around SVN 1.7 mishandling of svn:special changes
> svn test: escape peg revision separator using empty peg rev
Thanks. Here's the $deletions nit as a patch on top.
-- >8 --
Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D
This shouldn't make a difference because the $deletions hash is
only used when adding a directory (see 379862ec, 2012-02-20) but
it's nice to be consistent to make reading smoother anyway. No
functional change intended.
Signed-off-by: Jonathan Nieder
---
perl/Git/SVN/Editor.pm |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 3bbc20a0..178920c8 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -358,12 +358,12 @@ sub T {
mode_a => $m->{mode_a}, mode_b => '00',
sha1_a => $m->{sha1_a}, sha1_b => '0' x 40,
chg => 'D', file_b => $m->{file_b}
- });
+ }, $deletions);
$self->A({
mode_a => '00', mode_b => $m->{mode_b},
sha1_a => '0' x 40, sha1_b => $m->{sha1_b},
chg => 'A', file_b => $m->{file_b}
- });
+ }, $deletions);
return;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Jonathan Nieder wrote: > Eric Wong wrote: > > > Thanks both. Also pushed to "master" on git://bogomips.org/git-svn.git > > (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f) > > > > Jonathan Nieder (2): > > git svn: work around SVN 1.7 mishandling of svn:special changes > > svn test: escape peg revision separator using empty peg rev > > Thanks. Here's the $deletions nit as a patch on top. > > -- >8 -- > Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D For future reference, it'd be slightly easier for me to apply if you included the From: (and Date:) headers so I don't have to yank+paste them myself :> > This shouldn't make a difference because the $deletions hash is > only used when adding a directory (see 379862ec, 2012-02-20) but > it's nice to be consistent to make reading smoother anyway. No > functional change intended. > > Signed-off-by: Jonathan Nieder Signed-off-by: Eric Wong And pushed to master on git://bogomips.org/git-svn.git (commit a9608896587718549e82c5bae11740f2c0eac4c6) Jonathan Nieder (3): git svn: work around SVN 1.7 mishandling of svn:special changes svn test: escape peg revision separator using empty peg rev Git::SVN::Editor::T: pass $deletions to ->A and ->D -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Eric Wong wrote: > Jonathan Nieder wrote: >> -- >8 -- >> Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D > > For future reference, it'd be slightly easier for me to apply if you > included the From: (and Date:) headers so I don't have to yank+paste > them myself :> Ah, I assumed you were using "git am --scissors". Will do next time. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Jonathan Nieder wrote: > Eric Wong wrote: > > Jonathan Nieder wrote: > > >> -- >8 -- > >> Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D > > > > For future reference, it'd be slightly easier for me to apply if you > > included the From: (and Date:) headers so I don't have to yank+paste > > them myself :> > > Ah, I assumed you were using "git am --scissors". Will do next time. I missed the addition of --scissors. Will use it in the future :> -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Eric Wong writes: > Jonathan Nieder wrote: >> Michael J Gruber wrote: >> > Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41: >> >> >> Signed-off-by: Jonathan Nieder >> > >> > Tested with Subversion 1.6.18. > > Thanks both. Also pushed to "master" on git://bogomips.org/git-svn.git > (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f) > > Jonathan Nieder (2): > git svn: work around SVN 1.7 mishandling of svn:special changes > svn test: escape peg revision separator using empty peg rev Thanks; pulled. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
