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

Durham Goode durham at fb.com
Thu May 19 15:00:48 EDT 2016


This patch looks pretty straight forward and sane to me.


On 5/12/16 9:15 AM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1463058839 25200
> #      Thu May 12 06:13:59 2016 -0700
> # Node ID 05aaad2606ba0f2c6a0bf088d528dacf1436c419
> # Parent  df838803c1d487e4601f96c6cfd85e6ad4f6291f
> strip: invalidate phase cache after stripping changeset (issue5235)
>
> 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
> @@ -194,6 +194,7 @@
>               if not repo.ui.verbose:
>                   repo.ui.popbuffer()
>               f.close()
> +        repo._phasecache.invalidate()
>   
>           for m in updatebm:
>               bm[m] = repo[newbmtarget].node()
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=lIZfwj_e7hmKsNbSd9ENLDYtrbWZUnuHNDwQlvjQWr8&s=p3QJeWzAtFbhXlCbuVLgJsIjVJpMJPhbz6iEfHypPlo&e=



More information about the Mercurial-devel mailing list