Bug 3462 - convert crashes due to integrity error
Summary: convert crashes due to integrity error
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: convert (show other bugs)
Version: earlier
Hardware: All All
: normal bug
Assignee: Idan Kamara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-17 18:07 UTC by Bryan O'Sullivan
Modified: 2017-11-01 18:05 UTC (History)
5 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan O'Sullivan 2012-05-17 18:07 UTC
hg clone https://code.google.com/p/catroid/ catroid
echo 'exclude "Paintroid"' > rem
echo 'exclude "catroweb"' > rem
hg convert --traceback catroid wtf
Comment 1 Bryan O'Sullivan 2012-05-17 18:20 UTC
Typo: second ">" should be ">>".
Comment 2 Bryan O'Sullivan 2012-05-17 19:15 UTC
I thought this was specific to 2.2, but it isn't. I was able to reproduce the crash with 2.1.2 on my first try, although it took until closer to the end of the convert run.

The trigger is in this block of code:

http://selenic.com/repo/hg/file/9acb5cd19162/hgext/convert/hg.py#l175

When the rollback occurs, there seems to be a race in which the changelog may not be dropped and reread by the filecache code as it should be. As a result, it contains phantom data for the rolled-back rev, and the next commit fails with an integrity error.
Comment 3 Bryan O'Sullivan 2012-05-17 19:16 UTC
Downgrading, since not a regression.
Comment 4 Bryan O'Sullivan 2012-05-17 22:42 UTC
Bisected to this commit:

changeset:   16116:ce0ad184f489
branch:      stable
user:        Idan Kamara <idankk86@gmail.com>
date:        Thu Feb 16 01:21:34 2012 +0200
summary:     localrepo: clear _filecache on rollback (issue3261)
Comment 5 Idan Kamara 2012-05-18 17:57 UTC
Thanks for looking into it. Hopefully I'll have time tomorrow to dig deeper.
Comment 6 Idan Kamara 2012-05-20 13:02 UTC
Can't seem to reproduce (using 2.2.1).
Comment 7 Matt Mackall 2012-05-20 15:22 UTC
(In reply to comment #6)

Bryan's example isn't quite complete, did you test it with "rem" as a filemap?
Comment 8 Bryan O'Sullivan 2012-05-20 22:40 UTC
Yes, two mistakes in the recipe. Ouch. Here's one with both corrected:

hg clone https://code.google.com/p/catroid/ catroid
echo 'exclude "Paintroid"' > rem
echo 'exclude "catroweb"' >> rem
hg convert --filemap rem --traceback catroid wtf
Comment 9 Idan Kamara 2012-05-21 17:01 UTC
(In reply to comment #2)

I haven't had time to fully understand what's going on, but looking at what you found and the faulty commit it seems that what's done there to force reloading isn't enough.

This seems to fix it but some rebase tests are failing.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1314,7 +1314,7 @@
         # head, refresh the tag cache, then immediately add a new head.
         # But I think doing it this way is necessary for the "instant
         # tag cache retrieval" case to work.
-        self.invalidatecaches()
+        self.invalidate()
 
         # Discard all cache entries to force reloading everything.
         self._filecache.clear()
Comment 10 Bryan O'Sullivan 2012-06-01 17:58 UTC
Idan committed a fix here:

http://selenic.com/repo/hg/rev/9a99224a6409

However, the default branch is broken as of

http://selenic.com/repo/hg/rev/8abee656e14c

which merges Idan's fix. Specifically, the following tests fail:

test-mq-caches.t
test-mq-header-date.t
test-mq-header-from.t
test-mq-qrefresh-replace-log-message.t
test-mq-qrefresh.t
test-mq-subrepo.t
test-mq-symlinks.t
test-mq.t

If I back out Idan's change, the tests on the default branch are happy again, but I have not tried that catroid tree to see if it gets broken once more by backing out 9a99224a6409.
Comment 11 Matt Mackall 2012-07-29 14:50 UTC
Presumed fixed in 2.2.3 by:

9a99224a6409 localrepo: clear _filecache earlier to really force reloading