changeset 588:2723152c8111

stupid: fix getcopies() logic getcopies() assumed that copies where happening withing the current branch. This is wrong when a branch replaces another, and used to generate wrong copy records when copy sources existed in parent revision but were coming from an unrelated revision.
author Patrick Mezard <pmezard@gmail.com>
date Tue, 02 Mar 2010 17:06:06 +0100
parents c06f59441f8e
children f360e1629f5d
files hgsubversion/stupid.py tests/fixtures/replace_branch_with_branch.sh tests/fixtures/replace_branch_with_branch.svndump tests/test_fetch_branches.py
diffstat 4 files changed, 109 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/hgsubversion/stupid.py
+++ b/hgsubversion/stupid.py
@@ -254,7 +254,7 @@ def diff_branchrev(ui, svn, meta, branch
 
     return list(touched_files), filectxfn
 
-def makecopyfinder(r, branchpath, rootdir):
+def makecopyfinder(meta, r, branchpath):
     """Return a function detecting copies.
 
     Returned copyfinder(path) returns None if no copy information can
@@ -264,21 +264,35 @@ def makecopyfinder(r, branchpath, rootdi
     "sourcepath" and "source" are the same, while file copies dectected from
     directory copies return the copied source directory in "source".
     """
+    # cache changeset contexts and map them to source svn revisions
+    ctxs = {}
+    def getctx(branch, svnrev):
+        if svnrev in ctxs:
+            return ctxs[svnrev]
+        changeid = meta.get_parent_revision(svnrev + 1, branch, True)
+        ctx = None
+        if changeid != revlog.nullid:
+            ctx = meta.repo.changectx(changeid)
+        ctxs[svnrev] = ctx
+        return ctx
+
     # filter copy information for current branch
     branchpath = (branchpath and branchpath + '/') or ''
-    fullbranchpath = rootdir + branchpath
     copies = []
     for path, e in r.paths.iteritems():
         if not e.copyfrom_path:
             continue
         if not path.startswith(branchpath):
             continue
-        if not e.copyfrom_path.startswith(fullbranchpath):
-            # ignore cross branch copies
+        # compute converted source path and revision
+        frompath, frombranch = meta.split_branch_path(e.copyfrom_path)[:2]
+        if frompath is None:
+            continue
+        fromctx = getctx(frombranch, e.copyfrom_rev)
+        if fromctx is None:
             continue
-        dest = path[len(branchpath):]
-        source = e.copyfrom_path[len(fullbranchpath):]
-        copies.append((dest, (source, e.copyfrom_rev)))
+        destpath = path[len(branchpath):]
+        copies.append((destpath, (frompath, fromctx)))
 
     copies.sort(reverse=True)
     exactcopies = dict(copies)
@@ -287,12 +301,12 @@ def makecopyfinder(r, branchpath, rootdi
         if path in exactcopies:
             return exactcopies[path], exactcopies[path][0]
         # look for parent directory copy, longest first
-        for dest, (source, sourcerev) in copies:
+        for dest, (source, sourcectx) in copies:
             dest = dest + '/'
             if not path.startswith(dest):
                 continue
             sourcepath = source + '/' + path[len(dest):]
-            return (source, sourcerev), sourcepath
+            return (source, sourcectx), sourcepath
         return None
 
     return finder
@@ -309,7 +323,7 @@ def getcopies(svn, meta, branch, branchp
     # one event for single file copy). We assume that copy events match
     # copy sources in revision info.
     svncopies = {}
-    finder = makecopyfinder(r, branchpath, svn.subdir)
+    finder = makecopyfinder(meta, r, branchpath)
     for f in files:
         copy = finder(f)
         if copy:
@@ -317,24 +331,9 @@ def getcopies(svn, meta, branch, branchp
     if not svncopies:
         return {}
 
-    # cache changeset contexts and map them to source svn revisions
-    ctxs = {}
-    def getctx(svnrev):
-        if svnrev in ctxs:
-            return ctxs[svnrev]
-        changeid = meta.get_parent_revision(svnrev + 1, branch)
-        ctx = None
-        if changeid != revlog.nullid:
-            ctx = meta.repo.changectx(changeid)
-        ctxs[svnrev] = ctx
-        return ctx
-
     # check svn copies really make sense in mercurial
     hgcopies = {}
-    for (sourcepath, rev), copies in svncopies.iteritems():
-        sourcectx = getctx(rev)
-        if sourcectx is None:
-            continue
+    for (sourcepath, sourcectx), copies in svncopies.iteritems():
         for k, v in copies:
             if not util.issamefile(sourcectx, parentctx, v):
                 continue
--- a/tests/fixtures/replace_branch_with_branch.sh
+++ b/tests/fixtures/replace_branch_with_branch.sh
@@ -24,7 +24,10 @@ echo b > branches/branch1/b
 echo d > branches/branch1/d
 mkdir branches/branch1/dir
 echo e > branches/branch1/dir/e
-svn add branches/branch1/b branches/branch1/d branches/branch1/dir
+echo f > branches/branch1/f
+echo g > branches/branch1/g
+svn add branches/branch1/b branches/branch1/d branches/branch1/dir \
+    branches/branch1/f branches/branch1/g
 svn ci -m 'add b to branch1'
 svn cp trunk branches/branch2
 svn ci -m 'branch2'
@@ -32,7 +35,8 @@ svn up
 echo c > branches/branch2/c
 mkdir branches/branch2/dir
 echo e2 > branches/branch2/dir/e
-svn add branches/branch2/c branches/branch2/dir
+echo f2 > branches/branch2/f
+svn add branches/branch2/c branches/branch2/dir branches/branch2/f
 svn ci -m 'add c to branch2'
 svn up
 
@@ -44,6 +48,8 @@ rcopy branches/branch2 branches/branch1
 rcopy branches/branch1/d branches/branch1/a
 rcopy branches/branch1/dir branches/branch1/dir
 rcopy branches/branch1/dir branches/branch1/dir2
+rcopy branches/branch1/f branches/branch1/f
+rcopy branches/branch1/g branches/branch1/g
 EOF
 
 python $RSVN --message=blah --username=evil `pwd`/repo < clobber.rsvn
--- a/tests/fixtures/replace_branch_with_branch.svndump
+++ b/tests/fixtures/replace_branch_with_branch.svndump
@@ -1,6 +1,6 @@
 SVN-fs-dump-format-version: 2
 
-UUID: a88ce668-5208-4e23-8d73-bebfb2ebee53
+UUID: 6c4a9db2-8b38-4988-bc82-df1f3bb0afcd
 
 Revision-number: 0
 Prop-content-length: 56
@@ -9,7 +9,7 @@ Content-length: 56
 K 8
 svn:date
 V 27
-2010-03-01T20:45:07.143960Z
+2010-03-01T23:13:19.505202Z
 PROPS-END
 
 Revision-number: 1
@@ -27,7 +27,7 @@ pmezard
 K 8
 svn:date
 V 27
-2010-03-01T20:45:08.063083Z
+2010-03-01T23:13:20.066689Z
 PROPS-END
 
 Node-path: branches
@@ -76,7 +76,7 @@ pmezard
 K 8
 svn:date
 V 27
-2010-03-01T20:45:11.044475Z
+2010-03-01T23:13:23.056920Z
 PROPS-END
 
 Node-path: branches/branch1
@@ -101,7 +101,7 @@ pmezard
 K 8
 svn:date
 V 27
-2010-03-01T20:45:13.068264Z
+2010-03-01T23:13:25.099593Z
 PROPS-END
 
 Node-path: branches/branch1/b
@@ -152,6 +152,32 @@ PROPS-END
 e
 
 
+Node-path: branches/branch1/f
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 2
+Text-content-md5: 9a8ad92c50cae39aa2c5604fd0ab6d8c
+Text-content-sha1: a9fcd54b25e7e863d72cd47c08af46e61b74b561
+Content-length: 12
+
+PROPS-END
+f
+
+
+Node-path: branches/branch1/g
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 2
+Text-content-md5: f5302386464f953ed581edac03556e55
+Text-content-sha1: a5938ace3f424be1a26904781cdb06d55b614e6b
+Content-length: 12
+
+PROPS-END
+g
+
+
 Revision-number: 4
 Prop-content-length: 108
 Content-length: 108
@@ -167,7 +193,7 @@ pmezard
 K 8
 svn:date
 V 27
-2010-03-01T20:45:15.045695Z
+2010-03-01T23:13:27.051018Z
 PROPS-END
 
 Node-path: branches/branch2
@@ -192,7 +218,7 @@ pmezard
 K 8
 svn:date
 V 27
-2010-03-01T20:45:17.065669Z
+2010-03-01T23:13:29.108780Z
 PROPS-END
 
 Node-path: branches/branch2/c
@@ -230,6 +256,19 @@ PROPS-END
 e2
 
 
+Node-path: branches/branch2/f
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 3
+Text-content-md5: 575c5638d60271457e54ab7d07309502
+Text-content-sha1: 1c49a440c352f3473efa9512255033b94dc7def0
+Content-length: 13
+
+PROPS-END
+f2
+
+
 Revision-number: 6
 Prop-content-length: 102
 Content-length: 102
@@ -245,7 +284,7 @@ evil
 K 8
 svn:date
 V 27
-2010-03-01T20:45:19.243252Z
+2010-03-01T23:13:31.520483Z
 PROPS-END
 
 Node-path: branches/branch1
@@ -296,3 +335,27 @@ Node-copyfrom-rev: 5
 Node-copyfrom-path: branches/branch1/dir
 
 
+Node-path: branches/branch1/f
+Node-kind: file
+Node-action: delete
+
+Node-path: branches/branch1/f
+Node-kind: file
+Node-action: add
+Node-copyfrom-rev: 5
+Node-copyfrom-path: branches/branch1/f
+Text-copy-source-md5: 9a8ad92c50cae39aa2c5604fd0ab6d8c
+Text-copy-source-sha1: a9fcd54b25e7e863d72cd47c08af46e61b74b561
+
+
+
+
+Node-path: branches/branch1/g
+Node-kind: file
+Node-action: add
+Node-copyfrom-rev: 5
+Node-copyfrom-path: branches/branch1/g
+Text-copy-source-md5: f5302386464f953ed581edac03556e55
+Text-copy-source-sha1: a5938ace3f424be1a26904781cdb06d55b614e6b
+
+
--- a/tests/test_fetch_branches.py
+++ b/tests/test_fetch_branches.py
@@ -141,13 +141,17 @@ class TestFetchBranches(test_util.TestBa
         self.assertEqual('branch1', ctx.branch())
         # r5 is where the replacement takes place
         ctx = repo[5]
-        self.assertEqual(set(['a', 'c', 'dir/e', 'dir2/e']), set(ctx))
+        self.assertEqual(set(['a', 'c', 'dir/e', 'dir2/e', 'f', 'g']), set(ctx))
         self.assertEqual('0', ctx.extra().get('close', '0'))
         self.assertEqual('branch1', ctx.branch())
         self.assertEqual('c\n', ctx['c'].data())
         self.assertEqual('d\n', ctx['a'].data())
         self.assertEqual('e\n', ctx['dir/e'].data())
         self.assertEqual('e\n', ctx['dir2/e'].data())
+        self.assertEqual('f\n', ctx['f'].data())
+        self.assertEqual('g\n', ctx['g'].data())
+        for f in ctx:
+            self.assertTrue(not ctx[f].renamed())
 
     def test_replace_branch_with_branch_stupid(self, stupid=False):
         self.test_replace_branch_with_branch(True)