Submitter | Pierre-Yves David |
---|---|
Date | April 15, 2013, 3:15 p.m. |
Message ID | <715a39973a7af426651c.1366038921@crater1.logilab.fr> |
Download | mbox | patch |
Permalink | /patch/1316/ |
State | Accepted |
Commit | 31bcc5112191ecb72a964cb270363b9b68da2973 |
Headers | show |
Comments
On Mon, Apr 15, 2013 at 6:15 PM, <pierre-yves.david@logilab.fr> wrote: > > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@logilab.fr> > # Date 1366038658 -7200 > # Mon Apr 15 17:10:58 2013 +0200 > # Node ID 715a39973a7af426651cc2522cff04067d8d44ae > # Parent 6891e361bec6831ada94b7c2b49950769aba7be9 > destroyed: invalidate phraserevs cache in all case (issue3858) The real to test code ratio for this fix is disturbing. Can't this be tested more trivially?
On Mon, Apr 15, 2013 at 06:47:53PM +0300, Idan Kamara wrote: > On Mon, Apr 15, 2013 at 6:15 PM, <pierre-yves.david@logilab.fr> wrote: > > > > # HG changeset patch > > # User Pierre-Yves David <pierre-yves.david@logilab.fr> > > # Date 1366038658 -7200 > > # Mon Apr 15 17:10:58 2013 +0200 > > # Node ID 715a39973a7af426651cc2522cff04067d8d44ae > > # Parent 6891e361bec6831ada94b7c2b49950769aba7be9 > > destroyed: invalidate phraserevs cache in all case (issue3858) > > The real to test code ratio for this fix is disturbing. Can't this be > tested more trivially? Not much. it need a branchy history with specify phase setup. We could a tree a bit more compact but I think a wider testcase is useful given the lack of test in this area.
Den 2013-04-15 19:47:53 skrev Idan Kamara <idankk86@gmail.com>: > On Mon, Apr 15, 2013 at 6:15 PM, <pierre-yves.david@logilab.fr> wrote: >> >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@logilab.fr> >> # Date 1366038658 -7200 >> # Mon Apr 15 17:10:58 2013 +0200 >> # Node ID 715a39973a7af426651cc2522cff04067d8d44ae >> # Parent 6891e361bec6831ada94b7c2b49950769aba7be9 >> destroyed: invalidate phraserevs cache in all case (issue3858) > > The real to test code ratio for this fix is disturbing. Can't this be > tested more trivially? If it could, I suppose it wouldn't have taken a month of Matt's, Bryan's and Pierre-Yves' combined efforts to merely reproduce the issue (see bugzilla comments like "I've seen this happen at Facebook, too" and "It might be fixed, but I'm not sure" :)
On Mon, Apr 15, 2013 at 11:04 AM, Nikolaj Sjujskij <sterkrig@myopera.com>wrote: > If it could, I suppose it wouldn't have taken a month of Matt's, Bryan's > and Pierre-Yves' combined efforts to merely reproduce the issue (see > bugzilla comments like "I've seen this happen at Facebook, too" and "It > might be fixed, but I'm not sure" :) > Well. I'm not sure that "did nothing about this for a month" qualifies as "effort"? Although if it does, I have a long list of things I'd like to take credit for.
On 15 avr. 2013, at 20:04, Nikolaj Sjujskij wrote: > Den 2013-04-15 19:47:53 skrev Idan Kamara <idankk86@gmail.com>: > >> On Mon, Apr 15, 2013 at 6:15 PM, <pierre-yves.david@logilab.fr> wrote: >>> >>> # HG changeset patch >>> # User Pierre-Yves David <pierre-yves.david@logilab.fr> >>> # Date 1366038658 -7200 >>> # Mon Apr 15 17:10:58 2013 +0200 >>> # Node ID 715a39973a7af426651cc2522cff04067d8d44ae >>> # Parent 6891e361bec6831ada94b7c2b49950769aba7be9 >>> destroyed: invalidate phraserevs cache in all case (issue3858) >> >> The real to test code ratio for this fix is disturbing. Can't this be >> tested more trivially? > If it could, I suppose it wouldn't have taken a month of Matt's, Bryan's and Pierre-Yves' combined efforts to merely reproduce the issue It took me two month to get a day for looking at it ^^ that is all. The precise bug report with example and data saved the day.
On Mon, 2013-04-15 at 17:15 +0200, pierre-yves.david@logilab.fr wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@logilab.fr> > # Date 1366038658 -7200 > # Mon Apr 15 17:10:58 2013 +0200 > # Node ID 715a39973a7af426651cc2522cff04067d8d44ae > # Parent 6891e361bec6831ada94b7c2b49950769aba7be9 > destroyed: invalidate phraserevs cache in all case (issue3858) Queued for default, thanks.
Patch
diff --git a/mercurial/phases.py b/mercurial/phases.py --- a/mercurial/phases.py +++ b/mercurial/phases.py @@ -264,11 +264,19 @@ class phasecache(object): % (short(mnode), phase)) nodes.symmetric_difference_update(missing) filtered = True if filtered: self.dirty = True - self._phaserevs = None + # filterunknown is called by repo.destroyed, we may have no changes in + # root but phaserevs contents is certainly invalide (or at least we + # have not proper way to check that. related to issue 3858. + # + # The other caller is __init__ that have no _phaserevs initialized + # anyway. If this change we should consider adding a dedicated + # "destroyed" function to phasecache or a proper cache key mechanisme + # (see branchmap one) + self._phaserevs = None def advanceboundary(repo, targetphase, nodes): """Add nodes to a phase changing other nodes phases if necessary. This function move boundary *forward* this means that all nodes diff --git a/tests/test-rebase-cache.t b/tests/test-rebase-cache.t --- a/tests/test-rebase-cache.t +++ b/tests/test-rebase-cache.t @@ -383,5 +383,114 @@ Try both orders. 0 files updated, 0 files merged, 2 files removed, 0 files unresolved saved backup bundle to $TESTTMP/a3/c/.hg/strip-backup/*-backup.hg (glob) $ hg theads 0: 'A' + +Make sure rebase does not break for phase/filter related reason +---------------------------------------------------------------- +(issue3858) + + $ cd .. + + $ cat >> $HGRCPATH << EOF + > [ui] + > logtemplate={rev} {desc} {phase}\n + > EOF + $ cat $HGRCPATH + [ui] + slash = True + interactive = False + [defaults] + backout = -d "0 0" + commit = -d "0 0" + tag = -d "0 0" + [extensions] + graphlog= + rebase= + mq= + + [phases] + publish=False + + [alias] + tglog = log -G --template "{rev}: '{desc}' {branches}\n" + theads = heads --template "{rev}: '{desc}' {branches}\n" + [ui] + logtemplate={rev} {desc} {phase}\n + + + $ hg init c4 + $ cd c4 + + $ echo a > a + $ hg ci -Am A + adding a + $ echo b > b + $ hg ci -Am B + adding b + $ hg up 0 + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo c > c + $ hg ci -Am C + adding c + created new head + $ hg up 1 + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ hg merge + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (branch merge, don't forget to commit) + $ hg ci -m d + $ hg up 2 + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo e > e + $ hg ci -Am E + adding e + created new head + $ hg merge 3 + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (branch merge, don't forget to commit) + $ hg ci -m F + $ hg up 3 + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo g > g + $ hg ci -Am G + adding g + created new head + $ echo h > h + $ hg ci -Am H + adding h + $ hg up 5 + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved + $ echo i > i + $ hg ci -Am I + adding i + +Turn most changeset public + + $ hg ph -p 7 + + $ hg heads + 8 I draft + 7 H public + $ hg log -G + @ 8 I draft + | + | o 7 H public + | | + | o 6 G public + | | + o | 5 F draft + |\| + o | 4 E draft + | | + | o 3 d public + |/| + o | 2 C public + | | + | o 1 B public + |/ + o 0 A public + + + $ hg rebase --dest 7 --source 5 + saved backup bundle to $TESTTMP/a3/c4/.hg/strip-backup/*-backup.hg (glob)