[PATCH] extensions: move wrapfilecache function from fsmonitor

[Yuya Nishihara]

On Thu, 8 Jun 2017 23:49:27 +0100, Pierre-Yves David wrote:
> On 06/08/2017 05:47 PM, Augie Fackler wrote:
> >> On Jun 8, 2017, at 11:53, Yuya Nishihara <yuya at tcha.org> wrote:
> >> On Thu, 08 Jun 2017 10:51:38 -0400, Augie Fackler wrote:
> >>> # HG changeset patch
> >>> # User Augie Fackler <augie at google.com>
> >>> # Date 1496933093 14400
> >>> #      Thu Jun 08 10:44:53 2017 -0400
> >>> # Node ID ff739458783d32b8b0ce97a6771513caf280c09b
> >>> # Parent  04c19c8082410049465e2cdc510e24801530c94b
> >>> extensions: move wrapfilecache function from fsmonitor
> >>>
> >>> It makes more sense to put this in core, so other extensions can
> >>> trivially get access to it without having to rely on importing
> >>> fsmonitor.
> >>>
> >>> diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
> >>> --- a/hgext/fsmonitor/__init__.py
> >>> +++ b/hgext/fsmonitor/__init__.py
> >>> @@ -561,7 +561,8 @@ def wrapdirstate(orig, self):
> >>>     return ds
> >>>
> >>> def extsetup(ui):
> >>> -    wrapfilecache(localrepo.localrepository, 'dirstate', wrapdirstate)
> >>> +    extensions.wrapfilecache(
> >>> +        localrepo.localrepository, 'dirstate', wrapdirstate)
> >>>     if pycompat.sysplatform == 'darwin':
> >>>         # An assist for avoiding the dangling-symlink fsevents bug
> >>>         extensions.wrapfunction(os, 'symlink', wrapsymlink)
> >>> @@ -709,21 +710,3 @@ def reposetup(ui, repo):
> >>>                 return overridestatus(orig, self, *args, **kwargs)
> >>>
> >>>         repo.__class__ = fsmonitorrepo
> >>> -
> >>> -def wrapfilecache(cls, propname, wrapper):
> >>> -    """Wraps a filecache property. These can't be wrapped using the normal
> >>> -    wrapfunction. This should eventually go into upstream Mercurial.
> >>> -    """
> >>> -    assert callable(wrapper)
> >>> -    for currcls in cls.__mro__:
> >>> -        if propname in currcls.__dict__:
> >>> -            origfn = currcls.__dict__[propname].func
> >>> -            assert callable(origfn)
> >>> -            def wrap(*args, **kwargs):
> >>> -                return wrapper(origfn, *args, **kwargs)
> >>> -            currcls.__dict__[propname].func = wrap
> >>> -            break
> >>> -
> >>> -    if currcls is object:
> >>> -        raise AttributeError(
> >>> -            _("type '%s' has no property '%s'") % (cls, propname))
> >>
> >> IIRC, Jun had a bad time with this. Is this a good pattern to reliably
> >> wrap filecache?
> >
> > No idea - we need it for some stuff internal to Google (ugh), and it felt like we should move this in core and then improve it as needed to correct defects.
> 
> +1
> 
> This that is something needed in evolve from time to time too. +1 for 
> having something in core.
> 
> It seems sensible to move the fsmonitor version in core -as-is- and 
> improve it later (in core) if necessary. I assume fsmonitor will still 
> be happy if the patterns improves.

I think wrapfilecache() inherently conflicts with our class wrapping rule,
but anyway it would be better to not copy-paste wrapfilecache() hack around.

https://www.mercurial-scm.org/wiki/WritingExtensions#Wrapping_methods_on_the_ui_and_repo_classes