git-svn.perl: Fix glob matching on svn paths

git-svn.perl: Fix glob matching on svn paths

[Tomáš Ebenlendr]

Hello,

I tried to convert our repositories to git. Our repositories have only
branches (no tags, and no branch is so special to be called trunk).
The directory of each individual branch live in the root of the
repository (i.e., not in directory 'branches' as in standard layout).

I init the repository by: git svn init path_to_repo -b *
This triggers first bogus match in match_globs(): the pattern matches an
empty string - the place before first slash in any path.

We have created some branch names just by adding some suffix to another
branch name. Imagine branch "devel" and "devel2". Then there is bogus
match on path '/devel2' as it outputs 'devel'.

Patch in the next email fixes both these problems.

[PATCH] git-svn.perl: Fix glob matching on svn paths

[Tomáš Ebenlendr]

* Git::SVN::GlobSPec::new():
  - Directories or files with empty name are not allowed: The star pattern
    has to match at least one character.
  - The main pattern has to start with full path, i.e. "^/" is prepended.
    Also it has to end with directory boundary, i.e. "(/|$)" is appended.
---
 git-svn.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 18cfb24..01ae6a1 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -6132,7 +6132,7 @@ sub new {
 		if ($part eq "*") {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
-			push(@patterns, "[^/]*");
+			push(@patterns, "[^/]+");
 		} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
 			die $die_msg if $state eq "right";
 			$state = "pattern";
@@ -6155,8 +6155,8 @@ sub new {
 	my $left = join('/', @left);
 	my $right = join('/', @right);
 	$re = join('/', @patterns);
-	$re = join('\/',
-		   grep(length, quotemeta($left), "($re)", quotemeta($right)));
+	$re = '^\/'.join('\/',
+		   grep(length, quotemeta($left), "($re)", quotemeta($right))).'(?:\/|$)';
 	my $left_re = qr/^\/\Q$left\E(\/|$)/;
 	bless { left => $left, right => $right, left_regex => $left_re,
 	        regex => qr/$re/, glob => $glob, depth => $depth }, $class;
-- 
1.7.1

Re: [PATCH] git-svn.perl: Fix glob matching on svn paths

[Jonathan Nieder]

Hi Tomáš,

Tomáš Ebenlendr wrote:

> I tried to convert our repositories to git. Our repositories have only
> branches (no tags, and no branch is so special to be called trunk).
> The directory of each individual branch live in the root of the
> repository (i.e., not in directory 'branches' as in standard layout).

Okay, so I am imagining:

	1.0.x/
	1.1.x/
	1.2.x/
	2.0.x/
	...

> I init the repository by: git svn init path_to_repo -b *
> This triggers first bogus match in match_globs(): the pattern matches an
> empty string - the place before first slash in any path.

A branches refspec of

	*:refs/remotes/*

results in

	$self{left} = ''
	$self{glob} = '*'
	$self{left_regex} = qr'^/(/|$)'
	$self{regex} = qr'([^/]*)'.

Does get_dir_globbed cope correctly?  Will get_dir cope correctly with
the spurious / (from $left/$de) inserted at the beginning of paths?

The regex always matches, even for empty $p, but it is not immediately
obvious to me how that pans out.  Could you describe the symptoms?

> We have created some branch names just by adding some suffix to another
> branch name. Imagine branch "devel" and "devel2". Then there is bogus
> match on path '/devel2' as it outputs 'devel'.

Is this problem reproducible without the other change?  If so, would
it makes sense to split off this fix as a separate patch?

Also, if Eric likes your patches, can he forge your sign-off?  See
Documentation/SubmittingPatches for what this means.

Thanks,
Jonathan

Re: [PATCH] git-svn.perl: Fix glob matching on svn paths

[ebik]

[-- Attachment #1: Type: text/plain, Size: 4800 bytes --]

(once again and "reply-to-all" this time)

On Sun, 10 Oct 2010 01:15:35 -0500
Jonathan Nieder <jrnieder@gmail.com> wrote:

> Hi Tomáš,
> 
> Tomáš Ebenlendr wrote:
> 
> > I tried to convert our repositories to git. Our repositories have
> > only branches (no tags, and no branch is so special to be called
> > trunk). The directory of each individual branch live in the root of
> > the repository (i.e., not in directory 'branches' as in standard
> > layout).
> 
> Okay, so I am imagining:
> 
> 	1.0.x/
> 	1.1.x/
> 	1.2.x/
> 	2.0.x/
> 	...
> 
> > I init the repository by: git svn init path_to_repo -b *
> > This triggers first bogus match in match_globs(): the pattern
> > matches an empty string - the place before first slash in any path.
> 
> A branches refspec of
> 
> 	*:refs/remotes/*
> 
> results in
> 
> 	$self{left} = ''
> 	$self{glob} = '*'
> 	$self{left_regex} = qr'^/(/|$)'
> 	$self{regex} = qr'([^/]*)'.
> 
> Does get_dir_globbed cope correctly?  Will get_dir cope correctly with
> the spurious / (from $left/$de) inserted at the beginning of paths?
> 

Hmm, I don't know. The code is undocummented, thus I don't know what is
any function supposed to return. I just guess. With the patch my
"git svn clone" works, taking 48 hours to convert the repository.
The only problem is that many merges are probably mis-recognized
as cherrypicking, but we can live with that.

Both parts of the patch individually fix the error triggered by
'*:refs/remotes/*', but both parts are about what I think there should
be. The second part also fixes the bug with not recognised branches.

> The regex always matches, even for empty $p, but it is not immediately
> obvious to me how that pans out.  Could you describe the symptoms?
> 
The symptom of '*:refs/remotes/*' bug is following: 'clone' or 'fetch'
fails with following message:

  ref: 'refs/remotes/' ends with a trailing slash, this is not
  permitted by git nor Subversion

This is die() in Git::SVN::refname(). Here is the backtrace for
fetch. $VAR1 being the only argument passed to refname().
Note that the line numbers my be off by small number, as I added
two lines for the backtrace to happen.

$VAR1 = bless( {
                 'index' => '.git/svn/refs/remotes//index',
                 'map_root' => '.git/svn/refs/remotes//.rev_map',
                 'repo_id' => 'svn',
                 'config' => '.git/svn/config',
                 'path' => '',
                 'dir' => '.git/svn/refs/remotes/',
                 'ref_id' => 'refs/remotes/'
               }, 'Git::SVN' );
 at ../git-svn.perl line 2075
        Git::SVN::refname('Git::SVN=HASH(0x93df4b4)') called
at ../git-svn.perl line 1954
Git::SVN::init_remote_config('Git::SVN=HASH(0x93df4b4)',
'file:///afs/ms/u/t/tebe7122/devel/pokussvn', 1) called
at ../git-svn.perl line 2023 Git::SVN::init('Git::SVN',
'file:///afs/ms/u/t/tebe7122/devel/pokussvn', '', 'undef',
'refs/remotes/', 1) called at ../git-svn.perl line 5365
Git::SVN::Ra::match_globs('Git::SVN::Ra=HASH(0x9673f54)',
'HASH(0x910ec7c)', 'HASH(0x9674608)', 'ARRAY(0x8f845c4)', 1) called
at ../git-svn.perl line 5257
Git::SVN::Ra::gs_fetch_loop_common('Git::SVN::Ra=HASH(0x9673f54)', 0,
2, 'ARRAY(0x8f845ac)', 'ARRAY(0x8f845c4)') called at ../git-svn.perl
line 1809 Git::SVN::fetch_all('svn', 'HASH(0x92998f4)') called
at ../git-svn.perl line 992 main::cmd_multi_fetch() called
at ../git-svn.perl line 442 main::cmd_fetch() called at ../git-svn.perl
line 314 eval {...} called at ../git-svn.perl line 312

> > We have created some branch names just by adding some suffix to
> > another branch name. Imagine branch "devel" and "devel2". Then
> > there is bogus match on path '/devel2' as it outputs 'devel'.
> 
> Is this problem reproducible without the other change?  If so, would
> it makes sense to split off this fix as a separate patch?

I cannot reproduce the second bug without '*:/refs/remotes/*',
thus I'm not sure if I'm fixing the origin of the problem.

> 
> Also, if Eric likes your patches, can he forge your sign-off?  See
> Documentation/SubmittingPatches for what this means.
> 

Yes I'm fine with it. Although I'm not yet sure that I fix the problems
at their origin, and I have no time to read the whole source now.
I'll resubmit the patch(es) in any suggested shape, after someone
confirms, that it cannot break other things.

> Thanks,
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
                                 Tomáš 'ebík' Ebenlendr
                                 PF 2010.78487731481


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 230 bytes --]