[PATCH 3 of 4 STABLE] manifest: make manifestctx store the repo

[Yuya Nishihara]

On Wed, 26 Oct 2016 09:42:09 -0700, Durham Goode wrote:
> On 10/26/16 5:54 AM, Yuya Nishihara wrote:
> > On Tue, 25 Oct 2016 12:46:58 -0700, Durham Goode wrote:
> >> On 10/22/16 1:59 AM, Yuya Nishihara wrote:
> >>> On Tue, 18 Oct 2016 17:50:16 -0700, Durham Goode wrote:
> >>>> # HG changeset patch
> >>>> # User Durham Goode <durham at fb.com>
> >>>> # Date 1476837866 25200
> >>>> #      Tue Oct 18 17:44:26 2016 -0700
> >>>> # Branch stable
> >>>> # Node ID 3efece5c59853908d65de53635488361dbf20c49
> >>>> # Parent  ed607426a3ff4deda8c7f2de8b86d5b6ca976d67
> >>>> manifest: make manifestctx store the repo
> >>>>
> >>>> The old manifestctx stored a reference to the revlog. If the inmemory revlog
> >>>> became invalid, the ctx now held an old copy and would be incorrect. To fix
> >>>> this, we need the ctx to go through the manifestlog for each access.
> >>>>
> >>>> This is the same pattern that changectx already uses (it stores the repo, and
> >>>> accesses commit data through self._repo.changelog).
> >>>>
> >>>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> >>>> --- a/mercurial/manifest.py
> >>>> +++ b/mercurial/manifest.py
> >>>> @@ -1276,7 +1276,7 @@ class manifestlog(object):
> >>>>            if self._treeinmem:
> >>>>                m = treemanifestctx(self._revlog, '', node)
> >>>>            else:
> >>>> -            m = manifestctx(self._revlog, node)
> >>>> +            m = manifestctx(self._repo, node)
> >>>>            if node != revlog.nullid:
> >>>>                self._mancache[node] = m
> >>> This will create a reference cycle, but I don't know if the situation gets
> >>> better or worse after this patch.
> >>>
> >>>     repo -> manifestlog -> _mancache -> manifestctx -> repo
> >>>
> >>> Is _mancache valid after the manifestlog is recreated?
> >> _mancache is not really valid after manifestlog is recreated, since it
> >> may contain manifest entries that no longer exist in the file. Even if
> >> it didn't contain bad entries, the manifestctx itself needs up-to-date
> >> access to the manifest revlog, which is only available through the repo
> >> object (since the repo object's property is what handles manifest revlog
> >> cache checking).
> > Perhaps I miss the point. Do we have stale copies of manifestlog and manifestctx
> > somewhere? My understanding is we've moved the @storecache to manifestlog() at
> > 3c8811efdddc, so manifestlog, _revlog (and _mancache) should be accessible only
> > when they are valid, and manifestlog doesn't live longer than repo.manifest().
> Ah, yes manifestctx's may be held for longer than the manifestlog lives, 
> by code that is out of our control.  I'm not aware of specific cases 
> where this is happening, but it could.

Got it, thanks.

It seems manifestctx has slightly different guarantee about its lifetime from
changectx. changectx is effectively a snapshot when repo[changeid] is called
(more precisely, when propertycached function is called.)