Re: [bug][bisected] git-svn with root branches
- From
- Eric Wong <normalperson@yhbt.net>
- Date
- Oct 22, 2009, 18:55 UTC
- Message-ID
- <20091022185533.GA11771@dcvr.yhbt.net>
- In-Reply-To
- <c376da900910220824g2948dc2sa1156bda59b49405@mail.gmail.com>
Adam Brewster <adambrewster@gmail.com> wrote:
Show 91 quoted lines
> > > The offending config is:
> > > [svn-remote "svn"]
> > > url = http://svn.collab.net/repos/svn
> > > branches = /*:refs/remotes/*
> > >
> >
> > Reverting the left hand side of these two regexps from Adam's commit
> > seems to fix the problem.
> >
> > diff --git a/git-svn.perl b/git-svn.perl
> > index eb4b75a..56af221 100755
> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -1765,7 +1765,7 @@ sub read_all_remotes {
> > my $use_svm_props = eval { command_oneline(qw/config --bool
> > svn.useSvmProps/) };
> > $use_svm_props = $use_svm_props eq 'true' if $use_svm_props;
> > - my $svn_refspec = qr{\s*/?(.*?)\s*:\s*(.+?)\s*};
> > + my $svn_refspec = qr{\s*(.*?)\s*:\s*(.+?)\s*};
> > foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
> > if (m!^(.+)\.fetch=$svn_refspec$!) {
> > my ($remote, $local_ref, $remote_ref) = ($1, $2,
> > $3);
> > @@ -1979,7 +1979,7 @@ sub find_ref {
> > my ($ref_id) = @_;
> > foreach (command(qw/config -l/)) {
> > next unless m!^svn-remote\.(.+)\.fetch=
> > - \s*/?(.*?)\s*:\s*(.+?)\s*$!x;
> > + \s*(.*?)\s*:\s*(.+?)\s*$!x;
> > my ($repo_id, $path, $ref) = ($1, $2, $3);
> > if ($ref eq $ref_id) {
> > $path = '' if ($path =~ m#^\./?#);
> > ---
> >
> > I'm not sure why Adam decided the leading slash needed to be removed for
> > the git refspec. That said, the globbing/branching code still makes me
> > want to hide under a rock and I'm generally afraid to touch it.
> > I'll wait for Adam to chime in since he's braver than I am :)
> >
> >
> How's this for brave: I'm not sure why I did that either.
>
> Looking at it again it seems correct for the leading / to be ignored because
> it has no meaning. What's the difference between the above config and
> "branches = *:refs/remotes/*" (besides the fact that one works and one
> doesn't)? In both cases I think I've asked for all of the top-level
> directories to be branches. But that has nothing to do with the rest of the
> patch, so it shouldn't have been included.
>
> Given all of that I would be more inclined to fix this with something like
> the following:
>
> diff --git a/git-svn.perl b/git-svn.perl
> index eb4b75a..cd8a93b 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1768,7 +1768,7 @@ sub read_all_remotes {
> my $svn_refspec = qr{\s*/?(.*?)\s*:\s*(.+?)\s*};
> foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
> if (m!^(.+)\.fetch=$svn_refspec$!) {
> - my ($remote, $local_ref, $remote_ref) = ($1, $2, $3);
> + my ($remote, $local_ref, $remote_ref) = ($1, "/$2", $3);
> die("svn-remote.$remote: remote ref '$remote_ref' "
> . "must start with 'refs/'\n")
> unless $remote_ref =~ m{^refs/};
> @@ -1780,7 +1780,7 @@ sub read_all_remotes {
> $r->{$1}->{url} = $2;
> } elsif (m!^(.+)\.(branches|tags)=$svn_refspec$!) {
> my ($remote, $t, $local_ref, $remote_ref) =
> - ($1, $2, $3, $4);
> + ($1, $2, "/$3", $4);
> die("svn-remote.$remote: remote ref '$remote_ref' ($t) "
> . "must start with 'refs/'\n")
> unless $remote_ref =~ m{^refs/};
> @@ -1980,7 +1980,7 @@ sub find_ref {
> foreach (command(qw/config -l/)) {
> next unless m!^svn-remote\.(.+)\.fetch=
> \s*/?(.*?)\s*:\s*(.+?)\s*$!x;
> - my ($repo_id, $path, $ref) = ($1, $2, $3);
> + my ($repo_id, $path, $ref) = ($1, "/$2", $3);
> if ($ref eq $ref_id) {
> $path = '' if ($path =~ m#^\./?#);
> return ($repo_id, $path);
> --
> 1.6.5.1.63.ga9d7.dirty
>
> That is, continue disregard the / on the actual input because it means
> nothing, and add a / in places where the code will crash if it's not there.
> Even better would be to find out _why_ $path and $local_ref need a leading /
> and fix it that way, but that's more work that I don't have time for right
> now.Thanks for the feedback Adam,
Yeah, I'm confused by that myself :x /me kicks himself again
> On the other hand it doesn't really matter because git svn init won't accept > -b '' (to create the configuration I proposed above), so I have no practical > objection to Eric's solution.
Meanwhile, your patch looks good to me. Let me know if you want write a commit message for it or if you want me to do it.
I actually tried editing my $GIT_CONFIG in $EDITOR to have a bare '*' as the ref glob:
[svn-remote "svn"] url = http://svn.collab.net/repos/svn branches = *:refs/remotes/*
Since I've tried it, I suspect others may try editing $GIT_CONFIG in the same way as well.
-- Eric Wong