{"thread":{"id":"22797","subject":"[PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","startedAt":"2010-02-24T18:09:01Z","lastAt":"2010-03-13T20:41:35Z","messageCount":7,"participants":["Tuomas Suutari","Eric Wong","Sam Vilain","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"135590","messageId":"1267034942-31581-1-git-send-email-tuomas.suutari@gmail.com","threadId":"22797","inReplyTo":null,"subject":"[PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Tuomas Suutari","fromEmail":"tuomas.suutari@gmail.com","sentAt":"2010-02-24T18:09:01Z","receivedAt":"2010-02-24T18:09:01Z","isPatch":true,"sender":{"key":"tuomas.suutari@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2368817?v=4"},"body":"Detecting of merges from svn:mergeinfo or svk merge tickets failed\nwith rewrite-root option. This fixes it.\n\nSigned-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n---\nHi again,\n\nnow I found another problem while importing SVN repo with git-svn.\n\nTo speed-up the import, I copied the SVN repo with rsync to localhost\nand used file:// URL for the import, but because I want to be able to\ntrack the history with svn+ssh:// later, I used the rewrite-root\noption. That seemed to break the merge detecting.\n\nWith this patch the merge detecting works also with rewrite-root\noption, but since there are no comments why the $self->rewrite_root\nwas used in the first place, I have no idea, if this is the right\nthing to do.\n\n git-svn.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 265852f..1cbddca 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2993,7 +2993,7 @@ sub find_extra_svk_parents {\n \tfor my $ticket ( @tickets ) {\n \t\tmy ($uuid, $path, $rev) = split /:/, $ticket;\n \t\tif ( $uuid eq $self->ra_uuid ) {\n-\t\t\tmy $url = $self->rewrite_root || $self->{url};\n+\t\t\tmy $url = $self->{url};\n \t\t\tmy $repos_root = $url;\n \t\t\tmy $branch_from = $path;\n \t\t\t$branch_from =~ s{^/}{};\n@@ -3201,7 +3201,7 @@ sub find_extra_svn_parents {\n \t# are now marked as merge, we can add the tip as a parent.\n \tmy @merges = split \"\\n\", $mergeinfo;\n \tmy @merge_tips;\n-\tmy $url = $self->rewrite_root || $self->{url};\n+\tmy $url = $self->{url};\n \tmy $uuid = $self->ra_uuid;\n \tmy %ranges;\n \tfor my $merge ( @merges ) {\n-- \n1.7.0.2.ged48\n"},{"id":"135591","messageId":"1267034942-31581-2-git-send-email-tuomas.suutari@gmail.com","threadId":"22797","inReplyTo":"1267034942-31581-1-git-send-email-tuomas.suutari@gmail.com","subject":"[PATCH 2/2] t9150,t9151: Add rewrite-root option to init","fromName":"Tuomas Suutari","fromEmail":"tuomas.suutari@gmail.com","sentAt":"2010-02-24T18:09:02Z","receivedAt":"2010-02-24T18:09:02Z","isPatch":true,"sender":{"key":"tuomas.suutari@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2368817?v=4"},"body":"The rewrite-root option seems to be a bit problematic with merge\ndetecting, so it's better to have a merge detecting test with it\nturned on.\n\nSigned-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n---\n\nThis is mainly for supporting the first patch. Try applying this\nbefore the first patch to see the problem it addresses.\n\nAfter this is applied, then there are no merge detecting tests without\nrewrite-root. Could this be a problem?\n\n t/t9150-svk-mergetickets.sh |    1 +\n t/t9151-svn-mergeinfo.sh    |    1 +\n 2 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9150-svk-mergetickets.sh b/t/t9150-svk-mergetickets.sh\nindex 5358142..24c2421 100755\n--- a/t/t9150-svk-mergetickets.sh\n+++ b/t/t9150-svk-mergetickets.sh\n@@ -11,6 +11,7 @@ test_expect_success 'load svk depot' \"\n \tsvnadmin load -q '$rawsvnrepo' \\\n \t  < '$TEST_DIRECTORY/t9150/svk-merge.dump' &&\n \tgit svn init --minimize-url -R svkmerge \\\n+\t  --rewrite-root=http://svn.example.org \\\n \t  -T trunk -b branches '$svnrepo' &&\n \tgit svn fetch --all\n \t\"\ndiff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh\nindex 3569c62..c415775 100755\n--- a/t/t9151-svn-mergeinfo.sh\n+++ b/t/t9151-svn-mergeinfo.sh\n@@ -11,6 +11,7 @@ test_expect_success 'load svn dump' \"\n \tsvnadmin load -q '$rawsvnrepo' \\\n \t  < '$TEST_DIRECTORY/t9151/svn-mergeinfo.dump' &&\n \tgit svn init --minimize-url -R svnmerge \\\n+\t  --rewrite-root=http://svn.example.org \\\n \t  -T trunk -b branches '$svnrepo' &&\n \tgit svn fetch --all\n \t\"\n-- \n1.7.0.2.ged48\n"},{"id":"135743","messageId":"20100226094317.GA22579@dcvr.yhbt.net","threadId":"22797","inReplyTo":"1267034942-31581-1-git-send-email-tuomas.suutari@gmail.com","subject":"Re: [PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-02-26T09:43:17Z","receivedAt":"2010-02-26T09:43:17Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Tuomas Suutari <tuomas.suutari@gmail.com> wrote:\n> Detecting of merges from svn:mergeinfo or svk merge tickets failed\n> with rewrite-root option. This fixes it.\n> \n> Signed-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n> ---\n> Hi again,\n> \n> now I found another problem while importing SVN repo with git-svn.\n> \n> To speed-up the import, I copied the SVN repo with rsync to localhost\n> and used file:// URL for the import, but because I want to be able to\n> track the history with svn+ssh:// later, I used the rewrite-root\n> option. That seemed to break the merge detecting.\n> \n> With this patch the merge detecting works also with rewrite-root\n> option, but since there are no comments why the $self->rewrite_root\n> was used in the first place, I have no idea, if this is the right\n> thing to do.\n\nHi Tuomas,\n\nI'm not sure why rewrite_root is used here, either.  Ignoring it\nseems correct but I'll wait for Sam to chime in.\n\n>  git-svn.perl |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index 265852f..1cbddca 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -2993,7 +2993,7 @@ sub find_extra_svk_parents {\n>  \tfor my $ticket ( @tickets ) {\n>  \t\tmy ($uuid, $path, $rev) = split /:/, $ticket;\n>  \t\tif ( $uuid eq $self->ra_uuid ) {\n> -\t\t\tmy $url = $self->rewrite_root || $self->{url};\n> +\t\t\tmy $url = $self->{url};\n>  \t\t\tmy $repos_root = $url;\n>  \t\t\tmy $branch_from = $path;\n>  \t\t\t$branch_from =~ s{^/}{};\n> @@ -3201,7 +3201,7 @@ sub find_extra_svn_parents {\n>  \t# are now marked as merge, we can add the tip as a parent.\n>  \tmy @merges = split \"\\n\", $mergeinfo;\n>  \tmy @merge_tips;\n> -\tmy $url = $self->rewrite_root || $self->{url};\n> +\tmy $url = $self->{url};\n>  \tmy $uuid = $self->ra_uuid;\n>  \tmy %ranges;\n>  \tfor my $merge ( @merges ) {\n> -- \n> 1.7.0.2.ged48\n"},{"id":"136709","messageId":"201003130854.24847.tuomas.suutari@gmail.com","threadId":"22797","inReplyTo":"20100226094317.GA22579@dcvr.yhbt.net","subject":"Re: [PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Tuomas Suutari","fromEmail":"tuomas.suutari@gmail.com","sentAt":"2010-03-13T06:54:24Z","receivedAt":"2010-03-13T06:54:24Z","isPatch":true,"sender":{"key":"tuomas.suutari@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2368817?v=4"},"body":"Hi Sam,\n\nCould you please provide your feedback on this?\n\nThanks!\n\nOn Fri 2010-02-26 11:43:17 Eric Wong wrote:\n> Tuomas Suutari <tuomas.suutari@gmail.com> wrote:\n> > Detecting of merges from svn:mergeinfo or svk merge tickets failed\n> > with rewrite-root option. This fixes it.\n> >\n> > Signed-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n> > ---\n> > Hi again,\n> >\n> > now I found another problem while importing SVN repo with git-svn.\n> >\n> > To speed-up the import, I copied the SVN repo with rsync to localhost\n> > and used file:// URL for the import, but because I want to be able to\n> > track the history with svn+ssh:// later, I used the rewrite-root\n> > option. That seemed to break the merge detecting.\n> >\n> > With this patch the merge detecting works also with rewrite-root\n> > option, but since there are no comments why the $self->rewrite_root\n> > was used in the first place, I have no idea, if this is the right\n> > thing to do.\n> \n> Hi Tuomas,\n> \n> I'm not sure why rewrite_root is used here, either.  Ignoring it\n> seems correct but I'll wait for Sam to chime in.\n> \n> >  git-svn.perl |    4 ++--\n> >  1 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/git-svn.perl b/git-svn.perl\n> > index 265852f..1cbddca 100755\n> > --- a/git-svn.perl\n> > +++ b/git-svn.perl\n> > @@ -2993,7 +2993,7 @@ sub find_extra_svk_parents {\n> >  \tfor my $ticket ( @tickets ) {\n> >  \t\tmy ($uuid, $path, $rev) = split /:/, $ticket;\n> >  \t\tif ( $uuid eq $self->ra_uuid ) {\n> > -\t\t\tmy $url = $self->rewrite_root || $self->{url};\n> > +\t\t\tmy $url = $self->{url};\n> >  \t\t\tmy $repos_root = $url;\n> >  \t\t\tmy $branch_from = $path;\n> >  \t\t\t$branch_from =~ s{^/}{};\n> > @@ -3201,7 +3201,7 @@ sub find_extra_svn_parents {\n> >  \t# are now marked as merge, we can add the tip as a parent.\n> >  \tmy @merges = split \"\\n\", $mergeinfo;\n> >  \tmy @merge_tips;\n> > -\tmy $url = $self->rewrite_root || $self->{url};\n> > +\tmy $url = $self->{url};\n> >  \tmy $uuid = $self->ra_uuid;\n> >  \tmy %ranges;\n> >  \tfor my $merge ( @merges ) {\n> \n"},{"id":"136714","messageId":"1268469883.7841.40.camel@denix","threadId":"22797","inReplyTo":"201003130854.24847.tuomas.suutari@gmail.com","subject":"Re: [PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2010-03-13T08:44:43Z","receivedAt":"2010-03-13T08:44:43Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"I may have been cargo culting nearby similar code blocks; treat it as\nsuspicious.\n\nThe idea with the rewrite_root support is for SVK and/or svnsync; these\nfirst copy revisions to a local repository, which you then point git-svn\nat.  I wrote a long piece on this, on this (quite dated now) guide to\nconverting from SVK to git: of particular relevance is this section:\nhttp://utsl.gen.nz/talks/git-svn/intro.html#howto-fetch-convert\n\nSo.  I predict that if this is not done correctly, then any SVK or\nsvnsync mirrors, when imported via git-svn, will not correctly detect\nstandard SVN merge attributes present in the original repository.  This\nshould be relatively easy to construct a test case for, using a similar\nstyle to the t9110, t9111, t9150 and t9151 tests.  ie, make a script\nwhich constructs a repository in the desired format, and then test that\nit imports as it should.\n\nThat's a lot of work to go through however for a class of users which\nmay not even exist.  So, in the meantime it may be more appropriate to\ngo with the simpler approach which does not try to handle this case\ncorrectly, until someone wants to do the above work.\n\nDoes that help?\nSam \n\n\nOn Sat, 2010-03-13 at 08:54 +0200, Tuomas Suutari wrote:\n> Hi Sam,\n> \n> Could you please provide your feedback on this?\n> \n> Thanks!\n> \n> On Fri 2010-02-26 11:43:17 Eric Wong wrote:\n> > Tuomas Suutari <tuomas.suutari@gmail.com> wrote:\n> > > Detecting of merges from svn:mergeinfo or svk merge tickets failed\n> > > with rewrite-root option. This fixes it.\n> > >\n> > > Signed-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n> > > ---\n> > > Hi again,\n> > >\n> > > now I found another problem while importing SVN repo with git-svn.\n> > >\n> > > To speed-up the import, I copied the SVN repo with rsync to localhost\n> > > and used file:// URL for the import, but because I want to be able to\n> > > track the history with svn+ssh:// later, I used the rewrite-root\n> > > option. That seemed to break the merge detecting.\n> > >\n> > > With this patch the merge detecting works also with rewrite-root\n> > > option, but since there are no comments why the $self->rewrite_root\n> > > was used in the first place, I have no idea, if this is the right\n> > > thing to do.\n> > \n> > Hi Tuomas,\n> > \n> > I'm not sure why rewrite_root is used here, either.  Ignoring it\n> > seems correct but I'll wait for Sam to chime in.\n> > \n> > >  git-svn.perl |    4 ++--\n> > >  1 files changed, 2 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/git-svn.perl b/git-svn.perl\n> > > index 265852f..1cbddca 100755\n> > > --- a/git-svn.perl\n> > > +++ b/git-svn.perl\n> > > @@ -2993,7 +2993,7 @@ sub find_extra_svk_parents {\n> > >  \tfor my $ticket ( @tickets ) {\n> > >  \t\tmy ($uuid, $path, $rev) = split /:/, $ticket;\n> > >  \t\tif ( $uuid eq $self->ra_uuid ) {\n> > > -\t\t\tmy $url = $self->rewrite_root || $self->{url};\n> > > +\t\t\tmy $url = $self->{url};\n> > >  \t\t\tmy $repos_root = $url;\n> > >  \t\t\tmy $branch_from = $path;\n> > >  \t\t\t$branch_from =~ s{^/}{};\n> > > @@ -3201,7 +3201,7 @@ sub find_extra_svn_parents {\n> > >  \t# are now marked as merge, we can add the tip as a parent.\n> > >  \tmy @merges = split \"\\n\", $mergeinfo;\n> > >  \tmy @merge_tips;\n> > > -\tmy $url = $self->rewrite_root || $self->{url};\n> > > +\tmy $url = $self->{url};\n> > >  \tmy $uuid = $self->ra_uuid;\n> > >  \tmy %ranges;\n> > >  \tfor my $merge ( @merges ) {\n> > \n"},{"id":"136718","messageId":"20100313094355.GA10052@dcvr.yhbt.net","threadId":"22797","inReplyTo":"1267034942-31581-1-git-send-email-tuomas.suutari@gmail.com","subject":"Re: [PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-03-13T09:43:56Z","receivedAt":"2010-03-13T09:43:56Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Tuomas Suutari <tuomas.suutari@gmail.com> wrote:\n> Detecting of merges from svn:mergeinfo or svk merge tickets failed\n> with rewrite-root option. This fixes it.\n> \n> Signed-off-by: Tuomas Suutari <tuomas.suutari@gmail.com>\n\n> With this patch the merge detecting works also with rewrite-root\n> option, but since there are no comments why the $self->rewrite_root\n> was used in the first place, I have no idea, if this is the right\n> thing to do.\n\nThanks Tuomas, and Sam for the clarification,\n\nThis series acked and pushed out to git://git.bogomips.org/git-svn\n\n-- \nEric Wong\n"},{"id":"136726","messageId":"7vaauc9cow.fsf@alter.siamese.dyndns.org","threadId":"22797","inReplyTo":"20100313094355.GA10052@dcvr.yhbt.net","subject":"Re: [PATCH 1/2] git-svn: Fix merge detecting with rewrite-root","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-13T20:41:35Z","receivedAt":"2010-03-13T20:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; pulled.\n"}]}