[PATCH] convert: svn-sink: test if execute bit was actually stored

Maxim Dounin mdounin at mdounin.ru
Tue Dec 25 16:43:18 CST 2007


Hello!

On Wed, 19 Dec 2007, Matt Mackall wrote:

> On Wed, Dec 19, 2007 at 06:13:43PM +0300, Maxim Dounin wrote:
>> Hello!
>>
>> On Sat, 15 Dec 2007, Maxim Dounin wrote:
>>
>>> Hello!
>>>
>>> As of now, the command sequence in tests/test-convert-svn-sink leads
>>> to conversion crash while converting revision where execute bit is set. As a
>>> result - execute bit isn't stored in converted repo. Attached patch makes it
>>> clear.
>>>
>>> As far as I debugged this, actual problem isn't in subversion sink code - but
>>> instead in mercurial conversion source code. It reports extra copy operation
>>> when setting execute bit follows copy for the particular file - because
>>> execute bit is stored in manifest and file itself "thinks" it was copied in
>>> last revision.
>>>
>>> It would be good if somebody with better knowledge of mercurial internals
>>> (BOS?) take a look at this.
>>
>> I've spent some more time investigating this - and it looks like 'hg log
>> -vC' also does wrong thing in this case depite the fact it has much more
>> code to correctly detect copies.
>>
>> Testcase:
>>
>> hg init a
>> cd a
>> echo a > a
>> hg ci -Ama
>> hg cp a b
>> hg ci -A -m 'copy a to b'
>> chmod +x b
>> hg ci -A -m 'set execute bit on b'
>> hg log -vC --template '{rev} {file_copies%filecopy}\n'
>>
>> I can't figure out how to fix this correctly though. Not reporting copies
>> when filelog wasn't changed by particular changelog revision doesn't look
>> like a way to go - it will break test/test-log, notably issue391 test.
>
> The problem is that we're using a heuristic (whether the file is in
> the list of changed files in a changeset) to decide whether to do a
> copy check. But that will list files whose modes have changed as well
> as just their contents.
>
> When we detect a case where a file may have been copied, we can do two
> additional checks:
>
> - does the file revision's linkrev point back to the changeset in
>  question? if yes, then the contents were definitely changed in this
>  revision, and thus the copy happened in this revision.
>
> - if the above quick check fails, we can check the file revision in
>  the parent changeset, and if it is different, we know the copy
>  happened in the child. This test is potentially slow, but should be
>  very infrequent.

Attached patch (against crew) implements the above algorithm in log 
--copies and convert (mercurial as a source) and adds appropriate 
tests.

Maxim Dounin
-------------- next part --------------
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1198577546 -10800
# Node ID 29d55b56ebacde64a0df0280b7061ce6678e76d1
# Parent  1127fe12202a87d76a4d87de4cee5e7c5b4b661a
Fix copies reporting in log and convert.

If copy logged in file revision, we report copy for changeset only
if file revisions linkrev points back to the changeset in question
or both changeset parents contain different file revisions.

This fixes extra copies reported when executable bit was changed for
previously copied file.

diff -r 1127fe12202a -r 29d55b56ebac hgext/convert/hg.py
--- a/hgext/convert/hg.py	Tue Dec 25 22:23:58 2007 +0100
+++ b/hgext/convert/hg.py	Tue Dec 25 13:12:26 2007 +0300
@@ -238,7 +238,26 @@ class mercurial_source(converter_source)
         copies = {}
         for name in files:
             try:
-                copies[name] = ctx.filectx(name).renamed()[0]
+                fctx = ctx.filectx(name)
+                renamed = fctx.renamed()[0]
+                if renamed:
+                    # if linkrev == rev then copy occured in this revision.
+                    # else check parents - copy occured in this revision
+                    # only if both parents contain different file revisions
+                    fnode = ctx.filenode(name)
+                    if fctx.filelog().linkrev(fnode) == ctx.rev:
+                        copies[name] = renamed
+                        continue
+                    def parentsdiffer():
+                        for p in ctx.parents():
+                            try:
+                                if fnode == p.filenode(name):
+                                    return False
+                            except revlog.LookupError:
+                                pass
+                        return True
+                    if parentsdiffer():
+                        copies[name] = renamed
             except TypeError:
                 pass
         return copies
diff -r 1127fe12202a -r 29d55b56ebac mercurial/commands.py
--- a/mercurial/commands.py	Tue Dec 25 22:23:58 2007 +0100
+++ b/mercurial/commands.py	Tue Dec 25 13:12:26 2007 +0300
@@ -1632,14 +1632,25 @@ def log(ui, repo, *pats, **opts):
                     break
         if rev in rcache[fn]:
             return rcache[fn][rev]
+
+        # If linkrev != rev (i.e. rev not found in rcache) check file
+        # version in parent's manifest. File was renamed if both parents
+        # contain different file revisions (and rename was logged in 
+        # current revision).
+
         mr = repo.manifest.rev(man)
         if repo.manifest.parentrevs(mr) != (mr - 1, nullrev):
-            return ncache[fn].get(repo.manifest.find(man, fn)[0])
-        if not dcache or dcache[0] != man:
-            dcache[:] = [man, repo.manifest.readdelta(man)]
-        if fn in dcache[1]:
-            return ncache[fn].get(dcache[1][fn])
-        return None
+            fnode = repo.manifest.find(man, fn)[0]
+        else:
+            if not dcache or dcache[0] != man:
+                dcache[:] = [man, repo.manifest.readdelta(man)]
+            fnode = dcache[1].get(fn)
+        if fnode not in ncache[fn]:
+            return None
+        for p in repo.manifest.parents(man):
+            if fnode == repo.manifest.find(p, fn)[0]:
+                return None
+        return ncache[fn].get(fnode)
 
     df = False
     if opts["date"]:
diff -r 1127fe12202a -r 29d55b56ebac tests/test-convert-hg-source
--- a/tests/test-convert-hg-source	Tue Dec 25 22:23:58 2007 +0100
+++ b/tests/test-convert-hg-source	Tue Dec 25 13:12:26 2007 +0300
@@ -29,6 +29,9 @@ hg merge 2
 hg merge 2
 hg ci -m 'merge remote copy' -d '4 0'
 
+chmod +x baz
+hg ci -m 'mark baz executable' -d '5 0'
+
 cd ..
 hg convert --datesort orig new 2>&1 | grep -v 'subversion python bindings could not be loaded'
 cd new
diff -r 1127fe12202a -r 29d55b56ebac tests/test-convert-hg-source.out
--- a/tests/test-convert-hg-source.out	Tue Dec 25 22:23:58 2007 +0100
+++ b/tests/test-convert-hg-source.out	Tue Dec 25 13:12:26 2007 +0300
@@ -9,11 +9,12 @@ scanning source...
 scanning source...
 sorting...
 converting...
-4 add foo bar
-3 change foo
-2 make bar and baz copies of foo
-1 merge local copy
-0 merge remote copy
+5 add foo bar
+4 change foo
+3 make bar and baz copies of foo
+2 merge local copy
+1 merge remote copy
+0 mark baz executable
 comparing with ../orig
 searching for changes
 no changes found
diff -r 1127fe12202a -r 29d55b56ebac tests/test-convert-svn-sink.out
--- a/tests/test-convert-svn-sink.out	Tue Dec 25 22:23:58 2007 +0100
+++ b/tests/test-convert-svn-sink.out	Tue Dec 25 13:12:26 2007 +0300
@@ -167,31 +167,29 @@ d1
 d1
 % executable
 5:f205b3636d77
-svn: Path 'b' does not exist
 assuming destination a-hg
 initializing svn wc 'a-hg-wc'
 scanning source...
 sorting...
 converting...
 0 make a file executable
-abort: svn exited with status 1
-At revision 5.
-                5        5 test         .
- M              5        4 test         c
-                5        1 test         d1
-                5        1 test         d1/d2
-                5        1 test         d1/d2/b
+At revision 6.
+                6        6 test         .
+                6        6 test         c
+                6        1 test         d1
+                6        1 test         d1/d2
+                6        1 test         d1/d2/b
 <?xml version="1.0"?>
 <log>
 <logentry
-   revision="5">
+   revision="6">
 <author>test</author>
 <date/>
 <paths>
 <path
-   action="D">/b</path>
+   action="M">/c</path>
 </paths>
-<msg>remove a file</msg>
+<msg>make a file executable</msg>
 </logentry>
 </log>
 executable
diff -r 1127fe12202a -r 29d55b56ebac tests/test-log
--- a/tests/test-log	Tue Dec 25 22:23:58 2007 +0100
+++ b/tests/test-log	Tue Dec 25 13:12:26 2007 +0300
@@ -40,6 +40,11 @@ echo foo > foo
 echo foo > foo
 hg ci -Ame2 -d '6 0'
 hg log -vC --template '{rev} {file_copies%filecopy}\n' -r 5
+
+echo % log copies, execute bit set
+chmod +x e
+hg ci -me3 -d '7 0'
+hg log -vC --template '{rev} {file_copies%filecopy}\n' -r 6
 
 echo '% log -p d'
 hg log -pv d
diff -r 1127fe12202a -r 29d55b56ebac tests/test-log.out
--- a/tests/test-log.out	Tue Dec 25 22:23:58 2007 +0100
+++ b/tests/test-log.out	Tue Dec 25 13:12:26 2007 +0300
@@ -86,6 +86,8 @@ 1 files updated, 0 files merged, 1 files
 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
 adding foo
 5 e (dir/b)
+% log copies, execute bit set
+6 
 % log -p d
 changeset:   3:16b60bf3f99a
 user:        test


More information about the Mercurial-devel mailing list