{"thread":{"id":"21598","subject":"[PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch","startedAt":"2009-11-12T20:18:48Z","lastAt":"2009-11-13T01:03:19Z","messageCount":3,"participants":["Toby Allsopp","Sam Vilain"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"127478","messageId":"871vk35o86.fsf@navakl084.mitacad.com","threadId":"21598","inReplyTo":null,"subject":"[PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch","fromName":"Toby Allsopp","fromEmail":"toby.allsopp@navman.co.nz","sentAt":"2009-11-12T20:18:48Z","receivedAt":"2009-11-12T20:18:48Z","isPatch":true,"sender":{"key":"toby.allsopp@navman.co.nz","avatar":null},"body":"When recording the revisions that it has merged, SVN sets the top\nrevision to be the latest revision in the repository, which is not\nnecessarily a revision on the branch that is being merged from.  When\nit is not on the branch, git-svn fails to add the extra parent to\nrepresent the merge because it relies on finding the commit on the\nbranch that corresponds to the top of the SVN merge range.\n\nIn order to correctly handle this case, we look for the maximum\nrevision less than or equal to the top of the SVN merge range that is\nactually on the branch being merged from.\n\nSigned-off-by: Toby Allsopp <toby.allsopp@navman.co.nz>\n---\n git-svn.perl             |    7 +++++--\n t/t9151-svn-mergeinfo.sh |    2 +-\n 2 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 6a3b501..27fbe30 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2950,8 +2950,11 @@ sub find_extra_svn_parents {\n \t\t\tmy $bottom_commit =\n \t\t\t\t$gs->rev_map_get($bottom, $self->ra_uuid) ||\n \t\t\t\t$gs->rev_map_get($bottom+1, $self->ra_uuid);\n-\t\t\tmy $top_commit =\n-\t\t\t\t$gs->rev_map_get($top, $self->ra_uuid);\n+\t\t\tmy $top_commit;\n+\t\t\tfor (; !$top_commit && $top >= $bottom; --$top) {\n+\t\t\t\t$top_commit =\n+\t\t\t\t\t$gs->rev_map_get($top, $self->ra_uuid);\n+\t\t\t}\n \n \t\t\tunless ($top_commit and $bottom_commit) {\n \t\t\t\twarn \"W:unknown path/rev in svn:mergeinfo \"\ndiff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh\nindex 0d42c84..f57daf4 100755\n--- a/t/t9151-svn-mergeinfo.sh\n+++ b/t/t9151-svn-mergeinfo.sh\n@@ -19,7 +19,7 @@ test_expect_success 'represent svn merges without intervening commits' \"\n \t[ `git cat-file commit HEAD^1 | grep parent | wc -l` -eq 2 ]\n \t\"\n \n-test_expect_failure 'represent svn merges with intervening commits' \"\n+test_expect_success 'represent svn merges with intervening commits' \"\n \t[ `git cat-file commit HEAD | grep parent | wc -l` -eq 2 ]\n \t\"\n \n-- \n1.6.5.2.155.gbb47.dirty\n"},{"id":"127481","messageId":"4AFCAC9C.9020305@vilain.net","threadId":"21598","inReplyTo":"871vk35o86.fsf@navakl084.mitacad.com","subject":"Re: [PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-11-13T00:47:24Z","receivedAt":"2009-11-13T00:47:24Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Toby Allsopp wrote:\n> When recording the revisions that it has merged, SVN sets the top\n> revision to be the latest revision in the repository, which is not\n> necessarily a revision on the branch that is being merged from.  When\n> it is not on the branch, git-svn fails to add the extra parent to\n> represent the merge because it relies on finding the commit on the\n> branch that corresponds to the top of the SVN merge range.\n\nI thought, \"that sounds like he's repeating himself, wait a sec...\"\n\n> -test_expect_failure 'represent svn merges with intervening commits' \"\n> +test_expect_success 'represent svn merges with intervening commits' \"\n>  \t[ `git cat-file commit HEAD | grep parent | wc -l` -eq 2 ]\n>  \t\"\n\nSo you made a failing test and then added the implementation for it?\nInteresting strategy :).  I'd probably not repeat the same sentence\ntwice though.\n\nThanks for contributing this.  There might be other bugs too, especially\nwhen upstream has a more complicated merge hierarchy ... apparently svn\ntends to get it wrong, so checking for all commits might not work in\nthat case.\n\nIt would be nice if \"dcommit\" could make these commits, too...\n\nSam\n"},{"id":"127486","messageId":"87ws1v44o8.fsf@navakl084.mitacad.com","threadId":"21598","inReplyTo":"4AFCAC9C.9020305@vilain.net","subject":"Re: [PATCH 2/2] git-svn: handle SVN merges from revisions past the tip of the branch","fromName":"Toby Allsopp","fromEmail":"toby.allsopp@navman.co.nz","sentAt":"2009-11-13T01:03:19Z","receivedAt":"2009-11-13T01:03:19Z","isPatch":true,"sender":{"key":"toby.allsopp@navman.co.nz","avatar":null},"body":"On Fri, Nov 13 2009, Sam Vilain wrote:\n\n> Toby Allsopp wrote:\n> > When recording the revisions that it has merged, SVN sets the top\n> > revision to be the latest revision in the repository, which is not\n> > necessarily a revision on the branch that is being merged from.  When\n> > it is not on the branch, git-svn fails to add the extra parent to\n> > represent the merge because it relies on finding the commit on the\n> > branch that corresponds to the top of the SVN merge range.\n>\n> I thought, \"that sounds like he's repeating himself, wait a sec...\"\n\nHmm, it makes perfect sense to me :-)  Does the explanation in 1/2 make\nmore sense?\n\nThe first sentence describes what Subversion does, the second what\ngit-svn does in response.\n\n> Thanks for contributing this.  There might be other bugs too, especially\n> when upstream has a more complicated merge hierarchy ... apparently svn\n> tends to get it wrong, so checking for all commits might not work in\n> that case.\n\nOh yes, SVN gets the merges wrong in an alarming number of cases, it's\nreally shocking.  I only stay sane at work because I tell myself that\nSVN is making the case for git for me :-)\n\n> It would be nice if \"dcommit\" could make these commits, too...\n\nYes.\n\nToby.\n"}]}