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

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Tomáš Ebenlendr <hidden>
Date: 2016-06-15 22:49:44

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

From: Tomáš Ebenlendr <hidden>
Date: 2016-06-15 22:49:44

* 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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:44

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

From: ebik <hidden>
Date: 2016-06-15 22:49:47

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

On Sun, 10 Oct 2010 01:15:35 -0500
Jonathan Nieder [off-list ref] wrote:
Hi Tomáš,

Tomáš Ebenlendr wrote:
quoted
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/
	...
quoted
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
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help