git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [bug][bisected] git-svn with root branches

From
EWEric 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
Previous: Eric WongNext: Eric Wong
Message 6 of 7 in “[bug][bisected] git-svn with root branches”
  1. Daniel CorderoOct 21, 2009
  2. Eric WongOct 22, 2009
  3. git svn: fix fetch where glob is on the top-level URLEric Wong, Oct 23, 2009
  4. Eric WongOct 23, 2009
  5. Eric WongOct 23, 2009
  6. Eric WongOct 22, 2009
  7. Eric WongOct 23, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.