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

(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