{"thread":{"id":"24741","subject":"[PATCH] git-svn: fix fetch with deleted tag","startedAt":"2010-08-14T14:07:11Z","lastAt":"2010-08-15T06:55:00Z","messageCount":5,"participants":["David D. Kilzer","Ævar Arnfjörð Bjarmason","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"148088","messageId":"1281794831-33347-1-git-send-email-ddkilzer@kilzer.net","threadId":"24741","inReplyTo":null,"subject":"[PATCH] git-svn: fix fetch with deleted tag","fromName":"David D. Kilzer","fromEmail":"ddkilzer@kilzer.net","sentAt":"2010-08-14T14:07:11Z","receivedAt":"2010-08-14T14:07:11Z","isPatch":true,"sender":{"key":"ddkilzer@kilzer.net","avatar":"https://avatars.githubusercontent.com/u/263571?v=4"},"body":"Currently git-svn assumes that two tags created from the same\nrevision will have the same repo url, so it uses a ref to the\ntag without checking that its url matches the current url.\n\nThis causes issues when fetching an svn repo where a tag was\ncreated, deleted, and then recreated under the following\ncircumstances:\n\n- Both tags were copied from the same revision.\n- Both tags had the same name.\n- Both tags had different repository paths.\n- [Optional] Both tags have a file with the same name but\n different content.\n\nWhen all four conditions are met, a checksum mismatch error\noccurs because the content of two files with the same path\ndiffer (see t/t9155--git-svn-fetch-deleted-tag.sh):\n\n Checksum mismatch: ChangeLog 065854....\n expected: ce771b....\n got: 9563fd....\n\nWhen only the first three conditions are met, no error occurs\nbut the tag in git matches the first (deleted) tag instead of\nthe last (most recent) tag (see\nt/t9156-git-svn-fetch-deleted-tag-2.sh).\n\nThe fix is to verify that the repo url for the ref matches the\ncurrent url. If the urls do not match, then a \"tail\" is grown\non the tag name by appending a dash and rechecking the new ref's\nrepo url until either a matching repo url is found or a new tag\nis created.\n\nAlso fix a regular expression used to remove the revision from\nthe end of a tag or branch name. The regex did not account for\nany \"tail\" (dashes) that may have been added to the end of the\ntag name (which first appeared in a00439ac). If not fixed, tags\nwith names like \"tags/mytag@5--@2\" may be created.\n---\nOriginally reported in: [BUG/TEST] git-svn: fetch fails with deleted tag\n\n\n\n\n git-svn.perl | 16 +++++++++--\n t/t9155-git-svn-fetch-deleted-tag.sh | 43 ++++++++++++++++++++++++++++++\n t/t9156-git-svn-fetch-deleted-tag-2.sh | 45 ++++++++++++++++++++++++++++++++\n 3 files changed, 101 insertions(+), 3 deletions(-)\n create mode 100755 t/t9155-git-svn-fetch-deleted-tag.sh\n create mode 100755 t/t9156-git-svn-fetch-deleted-tag-2.sh\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 8d2ef3d..c06d9d0 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2957,18 +2957,28 @@ sub other_gs {\n \tmy $gs = Git::SVN->find_by_url($new_url, $url, $branch_from);\n \tunless ($gs) {\n \t\tmy $ref_id = $old_ref_id;\n-\t\t$ref_id =~ s/\\@\\d+$//;\n+\t\t$ref_id =~ s/\\@\\d+-*$//;\n \t\t$ref_id .= \"\\@$r\";\n \t\t# just grow a tail if we're not unique enough :x\n \t\t$ref_id .= '-' while find_ref($ref_id);\n-\t\tprint STDERR \"Initializing parent: $ref_id\\n\" unless $::_q > 1;\n \t\tmy ($u, $p, $repo_id) = ($new_url, '', $ref_id);\n \t\tif ($u =~ s#^\\Q$url\\E(/|$)##) {\n \t\t\t$p = $u;\n \t\t\t$u = $url;\n \t\t\t$repo_id = $self->{repo_id};\n \t\t}\n-\t\t$gs = Git::SVN->init($u, $p, $repo_id, $ref_id, 1);\n+\t\twhile (1) {\n+\t\t\t# It is possible to tag two different subdirectories\n+\t\t\t# at the same revision. If the url for an existing\n+\t\t\t# ref does not match, we must create a new ref.\n+\t\t\t$gs = Git::SVN->init($u, $p, $repo_id, $ref_id, 1);\n+\t\t\tmy (undef, $max_commit) = $gs->rev_map_max(1);\n+\t\t\tlast if (!$max_commit);\n+\t\t\tmy ($url, undef, undef) = ::cmt_metadata($max_commit);\n+\t\t\tlast if ($url eq $gs->full_url);\n+\t\t\t$ref_id .= '-';\n+\t\t}\n+\t\tprint STDERR \"Initializing parent: $ref_id\\n\" unless $::_q > 1;\n \t}\n \t$gs\n }\ndiff --git a/t/t9155-git-svn-fetch-deleted-tag.sh b/t/t9155-git-svn-fetch-deleted-tag.sh\nnew file mode 100755\nindex 0000000..4b50d7f\n--- /dev/null\n+++ b/t/t9155-git-svn-fetch-deleted-tag.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description='git svn fetch deleted tag'\n+\n+. ./lib-git-svn.sh\n+\n+test_expect_success 'setup svn repo' '\n+\tmkdir -p import/trunk/subdir &&\n+\tmkdir -p import/branches &&\n+\tmkdir -p import/tags &&\n+\techo \"base\" > import/trunk/subdir/file &&\n+\tsvn_cmd import -m \"import for git svn\" import \"$svnrepo\" &&\n+\trm -rf import &&\n+\n+\tsvn_cmd mkdir --parents -m \"create mybranch directory\" \"$svnrepo/branches/mybranch\" &&\n+\tsvn_cmd cp -m \"create branch mybranch\" \"$svnrepo/trunk\" \"$svnrepo/branches/mybranch/trunk\" &&\n+\n+\tsvn_cmd co \"$svnrepo/trunk\" svn_project &&\n+\tcd svn_project &&\n+\n+\techo \"trunk change\" >> subdir/file &&\n+\tsvn_cmd ci -m \"trunk change\" subdir/file &&\n+\n+\tsvn_cmd switch \"$svnrepo/branches/mybranch/trunk\" &&\n+\techo \"branch change\" >> subdir/file &&\n+\tsvn_cmd ci -m \"branch change\" subdir/file &&\n+\n+\tcd .. &&\n+\tsvn_cmd cp -m \"create mytag attempt 1\" -r5 \"$svnrepo/trunk/subdir\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd rm -m \"delete mytag attempt 1\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd cp -m \"create mytag attempt 2\" -r5 \"$svnrepo/branches/mybranch/trunk/subdir\" \"$svnrepo/tags/mytag\"\n+'\n+\n+test_expect_success 'fetch deleted tags from same revision with checksum error' '\n+\tgit svn init --stdlayout \"$svnrepo\" git_project &&\n+\tcd git_project &&\n+\tgit svn fetch &&\n+\n+\tgit diff --exit-code mybranch:trunk/subdir/file tags/mytag:file &&\n+\tgit diff --exit-code master:subdir/file tags/mytag^:file\n+'\n+\n+test_done\ndiff --git a/t/t9156-git-svn-fetch-deleted-tag-2.sh b/t/t9156-git-svn-fetch-deleted-tag-2.sh\nnew file mode 100755\nindex 0000000..ad8589c\n--- /dev/null\n+++ b/t/t9156-git-svn-fetch-deleted-tag-2.sh\n@@ -0,0 +1,45 @@\n+#!/bin/sh\n+\n+test_description='git svn fetch deleted tag 2'\n+\n+. ./lib-git-svn.sh\n+\n+test_expect_success 'setup svn repo' '\n+\tmkdir -p import/branches &&\n+\tmkdir -p import/tags &&\n+\tmkdir -p import/trunk/subdir1 &&\n+\tmkdir -p import/trunk/subdir2 &&\n+\tmkdir -p import/trunk/subdir3 &&\n+\techo \"file1\" > import/trunk/subdir1/file &&\n+\techo \"file2\" > import/trunk/subdir2/file &&\n+\techo \"file3\" > import/trunk/subdir3/file &&\n+\tsvn_cmd import -m \"import for git svn\" import \"$svnrepo\" &&\n+\trm -rf import &&\n+\n+\tsvn_cmd co \"$svnrepo/trunk\" svn_project &&\n+\tcd svn_project &&\n+\n+\techo \"change1\" >> subdir1/file &&\n+\techo \"change2\" >> subdir2/file &&\n+\techo \"change3\" >> subdir3/file &&\n+\tsvn_cmd ci -m \"change\" . &&\n+\n+\tcd .. &&\n+\tsvn_cmd cp -m \"create mytag 1\" -r2 \"$svnrepo/trunk/subdir1\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd rm -m \"delete mytag 1\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd cp -m \"create mytag 2\" -r2 \"$svnrepo/trunk/subdir2\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd rm -m \"delete mytag 2\" \"$svnrepo/tags/mytag\" &&\n+\tsvn_cmd cp -m \"create mytag 3\" -r2 \"$svnrepo/trunk/subdir3\" \"$svnrepo/tags/mytag\"\n+'\n+\n+test_expect_success 'fetch deleted tags from same revision with no checksum error' '\n+\tgit svn init --stdlayout \"$svnrepo\" git_project &&\n+\tcd git_project &&\n+\tgit svn fetch &&\n+\n+\tgit diff --exit-code master:subdir3/file tags/mytag:file &&\n+\tgit diff --exit-code master:subdir2/file tags/mytag^:file &&\n+\tgit diff --exit-code master:subdir1/file tags/mytag^^:file\n+'\n+\n+test_done\n-- \n1.7.2.1.49.g98551\n"},{"id":"148090","messageId":"AANLkTinpLUyQP=6XktduWAmSHg3CgcT3Y7cMJ9FQ4by_@mail.gmail.com","threadId":"24741","inReplyTo":"1281794831-33347-1-git-send-email-ddkilzer@kilzer.net","subject":"Re: [PATCH] git-svn: fix fetch with deleted tag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-14T14:52:48Z","receivedAt":"2010-08-14T14:52:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Aug 14, 2010 at 14:07, David D. Kilzer wrote:\n\n> +                       my ($url, undef, undef) = ::cmt_metadata($max_commit);\n\nThis can just be:\n\n my ($url) = ::cmt_metadata($max_commit);\n\nPerl will throw the extra arguments away for you.\n\n> +test_expect_success 'setup svn repo' '\n> +       mkdir -p import/trunk/subdir &&\n> +       mkdir -p import/branches &&\n> +       mkdir -p import/tags &&\n> +       echo \"base\" > import/trunk/subdir/file &&\n\nJunio usually prefers the \">foo\" style to \"> foo\".\n\n> +       cd svn_project &&\n> +\n> +       echo \"trunk change\" >> subdir/file &&\n> +       svn_cmd ci -m \"trunk change\" subdir/file &&\n> +\n> +       svn_cmd switch \"$svnrepo/branches/mybranch/trunk\" &&\n> +       echo \"branch change\" >> subdir/file &&\n> +       svn_cmd ci -m \"branch change\" subdir/file &&\n> +\n> +       cd .. &&\n\nIf you use a subshell here it'll cd back for you.\n\n> +++ b/t/t9156-git-svn-fetch-deleted-tag-2.sh\n> @@ -0,0 +1,45 @@\n> +#!/bin/sh\n> +\n> +test_description='git svn fetch deleted tag 2'\n\nAny reason not to include both of these in the same file? Just to\navoid having to manually reset the repository?\n\n\n"},{"id":"148098","messageId":"84607.29034.qm@web30003.mail.mud.yahoo.com","threadId":"24741","inReplyTo":"AANLkTinpLUyQP=6XktduWAmSHg3CgcT3Y7cMJ9FQ4by_@mail.gmail.com","subject":"Re: [PATCH] git-svn: fix fetch with deleted tag","fromName":"David D. Kilzer","fromEmail":"ddkilzer@kilzer.net","sentAt":"2010-08-14T18:49:01Z","receivedAt":"2010-08-14T18:49:01Z","isPatch":true,"sender":{"key":"ddkilzer@kilzer.net","avatar":"https://avatars.githubusercontent.com/u/263571?v=4"},"body":"On Sat, August 14, 2010 at 7:52:48 AM, Ævar Arnfjörð Bjarmason wrote:\n\n\n> On Sat, Aug 14, 2010 at 14:07, David D. Kilzer wrote:\n> > +++ b/t/t9156-git-svn-fetch-deleted-tag-2.sh\n> > @@ -0,0 +1,45 @@\n> > +#!/bin/sh\n> > +\n> > +test_description='git svn fetch deleted tag 2'\n> \n> Any reason not to include both of these in the same file? Just to\n> avoid having to manually reset the repository?\n\n\nIt was easier to run the tests individually when working on them, and I was \nhesitant to combine the setup steps from each test since it wouldn't be as clear \nwhich steps were for which test in the future. I realize this may be slower \nwhen running the tests, but it makes them easier to maintain, especially when \none hasn't looked at the tests in a while.\n\nIs there a nice way to reset the repository between steps?\n\nThanks for the feedback! I've already applied your other suggestions.\n\nDave\n"},{"id":"148101","messageId":"AANLkTi=FKDa4sTTd1b=9yxsWafY1jEEdPrHZ88+uPozi@mail.gmail.com","threadId":"24741","inReplyTo":"84607.29034.qm@web30003.mail.mud.yahoo.com","subject":"Re: [PATCH] git-svn: fix fetch with deleted tag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-14T19:08:42Z","receivedAt":"2010-08-14T19:08:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Aug 14, 2010 at 18:49, David D. Kilzer wrote:\n> On Sat, August 14, 2010 at 7:52:48 AM, Ævar Arnfjörð Bjarmason wrote:\n>> On Sat, Aug 14, 2010 at 14:07, David D. Kilzer wrote:\n>> >  +++ b/t/t9156-git-svn-fetch-deleted-tag-2.sh\n>> >  @@ -0,0 +1,45 @@\n>> > +#!/bin/sh\n>> > +\n>> > +test_description='git  svn fetch deleted tag 2'\n>>\n>> Any reason not to include both of these in the  same file? Just to\n>> avoid having to manually reset the  repository?\n>\n> It was easier to run the tests individually when working on them, and I was\n> hesitant to combine the setup steps from each test since it wouldn't be as clear\n> which steps were for which test in the future.\n\nIf you think it's easier to maintain like this then by all means keep\nit as it is. I was just wondering why it was like this, that's all.\n\n> I realize this may be slower when running the tests, but it makes\n> them easier to maintain, especially when one hasn't looked at the\n> tests in a while.\n\nThe SLOOOOW part of running the git svn tests is definitely *not* the\ntiny bit of shellscript required to execute each *.sh file :)\n\n> Is there a nice way to reset the repository between steps?\n\nI don't know if this applies in this case but if you need fresh repos\nfor each tests you can usually do:\n\n test_expect_success 'test #1' '\n (test_create_repo one &&\n cd one &&\n \t...)\n '\n\n test_expect_success 'test #2' '\n (test_create_repo two &&\n cd two &&\n \t...)\n '\n\nBut I haven't looked at the svn_* functions you're using, so perhaps\nthat's not possible here.\n\n> Thanks for the feedback!  I've already applied your other suggestions.\n\nCool, good to know that it was helpful.\n"},{"id":"148124","messageId":"20100815065500.GA29542@dcvr.yhbt.net","threadId":"24741","inReplyTo":"84607.29034.qm@web30003.mail.mud.yahoo.com","subject":"Re: [PATCH] git-svn: fix fetch with deleted tag","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-08-15T06:55:00Z","receivedAt":"2010-08-15T06:55:00Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"\"David D. Kilzer\" wrote:\n> Thanks for the feedback! I've already applied your other suggestions.\n\nThanks David and Ævar!\n\nDavid: Everything looks alright to me with Ævar's suggestions, so I'll\nack whenever you have the final patch ready.\n\n\nOn a related note, if anybody has the time/patience to do some grunt\nwork: converting all existing and fragile \"cd\" usage to use subshells\nwould be much appreciated and help set better examples for introducing\nnew code.\n\n-- \nEric Wong\n"}]}