[PATCH] strip: invalidate phase cache after stripping changeset (issue5235)

timeless timeless at gmail.com
Wed May 11 09:13:43 EDT 2016


+1 for stable. I hit this while testing my runtests work. It's really annoying.

On Wed, May 11, 2016 at 9:10 AM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
>
>
> On 05/11/2016 03:08 PM, Laurent Charignon wrote:
>>
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon at fb.com>
>> # Date 1462971969 25200
>> #      Wed May 11 06:06:09 2016 -0700
>> # Node ID fd005346d3c86b65c690f8f987e05f44ec4ec02f
>> # Parent  df838803c1d487e4601f96c6cfd85e6ad4f6291f
>> strip: invalidate phase cache after stripping changeset (issue5235)
>
>
> Looks like a small fix, should this be on stable?
>
>
>>
>> When we remove a changeset from the changelog, the phase cache must be
>> invalidated, otherwise it could refer to changesets that are no longer in
>> the
>> repo.
>>
>> To reproduce the failure, I created an extension querying the phase cache
>> after
>> the strip transaction is over.
>>
>> To do that, I stripped two commits with a bookmark on one of them to force
>> another transaction (we open a transaction for moving bookmarks)
>> after the strip transaction.
>>
>> Without the fix in this patch, the test leads to a stacktrace showing the
>> issue:
>>        repair.strip(ui, repo, revs, backup)
>>      File
>> "/Users/lcharignon/facebook-hg-rpms/hg-crew/mercurial/repair.py", line 205,
>> in strip
>>        tr.close()
>>      File
>> "/Users/lcharignon/facebook-hg-rpms/hg-crew/mercurial/transaction.py", line
>> 44, in _active
>>        return func(self, *args, **kwds)
>>      File
>> "/Users/lcharignon/facebook-hg-rpms/hg-crew/mercurial/transaction.py", line
>> 490, in close
>>        self._postclosecallback[cat](self)
>>      File "$TESTTMP/crashstrip2.py", line 4, in test
>>        [repo.changelog.node(r) for r in repo.revs("not public()")]
>>      File
>> "/Users/lcharignon/facebook-hg-rpms/hg-crew/mercurial/changelog.py", line
>> 337, in node
>>        return super(changelog, self).node(rev)
>>      File
>> "/Users/lcharignon/facebook-hg-rpms/hg-crew/mercurial/revlog.py", line 377,
>> in node
>>        return self.index[rev][7]
>>    IndexError: revlog index out of range
>>
>> The situation was encountered in inhibit (evolve's repo) where we would
>> crash
>> following the volatile set invalidation submitted by Augie in
>> e6f490e328635312ee214a12bc7fd3c7d46bf9ce. Before his patch the issue was
>> masked
>> as we were not accessing the phasecache after stripping a revision.
>>
>> This bug uncovered another but in histedit (see explanation in issue5235).
>> I changed the histedit test accordingly to avoid fixing two things at
>> once.
>>
>> diff --git a/mercurial/repair.py b/mercurial/repair.py
>> --- a/mercurial/repair.py
>> +++ b/mercurial/repair.py
>> @@ -165,6 +165,7 @@
>>                 tr.startgroup()
>>               cl.strip(striprev, tr)
>> +            repo._phasecache.invalidate()
>>               mfst.strip(striprev, tr)
>>               for fn in files:
>>                   repo.file(fn).strip(striprev, tr)
>> diff --git a/tests/test-histedit-fold.t b/tests/test-histedit-fold.t
>> --- a/tests/test-histedit-fold.t
>> +++ b/tests/test-histedit-fold.t
>> @@ -468,14 +468,14 @@
>>   #endif
>>     $ hg histedit 6c795aa153cb --config hooks.commit="echo commit $NODE"
>> --commands - 2>&1 << EOF | fixbundle
>>     > pick 199b6bb90248 b
>> +  > pick 6c795aa153cb a
>>     > fold a1a953ffb4b0 c
>> -  > pick 6c795aa153cb a
>>     > EOF
>> -  commit 9599899f62c05f4377548c32bf1c9f1a39634b0c
>> +  commit 16b87e97178dde2af2f3c6f6ddda882292f21d13
>>       $ hg logt
>> -  1:9599899f62c0 a
>> -  0:79b99e9c8e49 b
>> +  1:32458953dcb2 a
>> +  0:16b87e97178d b
>>       $ echo "foo" > amended.txt
>>     $ hg add amended.txt
>> @@ -492,11 +492,11 @@
>>     $ echo foo >> foo
>>     $ hg ci -m foo3
>>     $ hg logt
>> -  4:21679ff7675c foo3
>> -  3:b7389cc4d66e foo2
>> -  2:0e01aeef5fa8 foo1
>> -  1:578c7455730c a
>> -  0:79b99e9c8e49 b
>> +  4:52350bf3559e foo3
>> +  3:e72786ea58ec foo2
>> +  2:60447e897f97 foo1
>> +  1:7fc781383476 a
>> +  0:16b87e97178d b
>>     $ cat > "$TESTTMP/editor.sh" <<EOF
>>     > echo ran editor >> "$TESTTMP/editorlog.txt"
>>     > cat \$1 >> "$TESTTMP/editorlog.txt"
>> @@ -504,15 +504,15 @@
>>     > echo merged foos > \$1
>>     > EOF
>>     $ HGEDITOR="sh \"$TESTTMP/editor.sh\"" hg histedit 1 --commands - 2>&1
>> <<EOF | fixbundle
>> -  > pick 578c7455730c 1 a
>> -  > pick 0e01aeef5fa8 2 foo1
>> -  > fold b7389cc4d66e 3 foo2
>> -  > fold 21679ff7675c 4 foo3
>> +  > pick 7fc781383476 1 a
>> +  > pick 60447e897f97 2 foo1
>> +  > fold e72786ea58ec 3 foo2
>> +  > fold 52350bf3559e 4 foo3
>>     > EOF
>>     $ hg logt
>> -  2:e8bedbda72c1 merged foos
>> -  1:578c7455730c a
>> -  0:79b99e9c8e49 b
>> +  2:be62bdec37b8 merged foos
>> +  1:7fc781383476 a
>> +  0:16b87e97178d b
>>   Editor should have run only once
>>     $ cat $TESTTMP/editorlog.txt
>>     ran editor
>> diff --git a/tests/test-strip.t b/tests/test-strip.t
>> --- a/tests/test-strip.t
>> +++ b/tests/test-strip.t
>> @@ -838,6 +838,41 @@
>>     date:        Thu Jan 01 00:00:00 1970 +0000
>>     summary:     mergeCD
>>     +Check that the phase cache is properly invalidated after a strip with
>> bookmark.
>> +
>> +  $ cat > ../stripstalephasecache.py << EOF
>> +  > from mercurial import extensions, localrepo
>> +  > def transactioncallback(orig, repo, desc, *args, **kwargs):
>> +  >     def test(transaction):
>> +  >         # observe cache inconsistency
>> +  >         try:
>> +  >             [repo.changelog.node(r) for r in repo.revs("not
>> public()")]
>> +  >         except IndexError:
>> +  >             repo.ui.status("Index error!\n")
>> +  >     transaction = orig(repo, desc, *args, **kwargs)
>> +  >     # warm up the phase cache
>> +  >     list(repo.revs("not public()"))
>> +  >     if desc != 'strip':
>> +  >          transaction.addpostclose("phase invalidation test", test)
>> +  >     return transaction
>> +  > def extsetup(ui):
>> +  >     extensions.wrapfunction(localrepo.localrepository, "transaction",
>> +  >                             transactioncallback)
>> +  > EOF
>> +  $ hg up -C 2
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  $ echo k > k
>> +  $ hg add k
>> +  $ hg commit -m commitK
>> +  $ echo l > l
>> +  $ hg add l
>> +  $ hg commit -m commitL
>> +  $ hg book -r tip blah
>> +  $ hg strip ".^" --config
>> extensions.crash=$TESTTMP/stripstalephasecache.py
>> +  0 files updated, 0 files merged, 2 files removed, 0 files unresolved
>> +  saved backup bundle to
>> $TESTTMP/issue4736/.hg/strip-backup/8f0b4384875c-4fa10deb-backup.hg (glob)
>> +  $ hg up -C 1
>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>>     Error during post-close callback of the strip transaction
>>   (They should be gracefully handled and reported)
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list