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

From: ebik <ebik@ucw.cz>
Date: Oct 14, 2010, 09:34 UTC
Message-ID: <20101014113459.47bfa959@Chewbacca.cub>
In-Reply-To: <20101010061534.GB23100@burratino>

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

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

Show 36 quoted lines

> 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

Show 6 quoted lines

> > 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.

Show 7 quoted lines

> 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