On Wed, 2017-07-05 at 23:55 +0900, Yuya Nishihara wrote: > On Sun, 02 Jul 2017 04:56:37 +0200, Pierre-Yves David wrote: > > # HG changeset patch > > # User Pierre-Yves David <pierre-yves.da...@ens-lyon.org> > > # Date 1470323266 -7200 > > # Thu Aug 04 17:07:46 2016 +0200 > > # Node ID ebf81efdd6d7ff15c64683933635616c00475688 > > # Parent 34b8be7f0420b07d0f7b71379c6055e5b26223d5 > > # EXP-Topic vfs.ward > > # Available At https://www.mercurial-scm.org/repo/users/marmoute/me > > rcurial/ > > # hg pull https://www.mercurial-scm.org/repo/users/mar > > moute/mercurial/ -r ebf81efdd6d7 > > vfs: add the possibility to have a "ward" to check vfs usage > > The feature has some similarity with the 'pathauditor', but further > > digging > > show it make more sense to keep them separated. The path auditor is > > fairly > > autonomous (create from vfs.__init__ without any extra > > information), and check > > very generic details. On the other hand, the wards will have deeper > > knownledge > > of the Mercurial logic and is created at the local repository > > level. Mixing the > > two would add much complexity for no real gains. > > My gut feeling is that vfs isn't the right place if they require > deeper > knowledge of repository/dirstate logic. And the whole weakref > business > floating around wards, hooks and transaction seems like just getting > around > layering violation.
We pondered on this choice a long time and couldn't find any other layer that is used by both Mercurial core and the extensions to access the file system. It seems to be the highest abstraction layer we could hook into without introducing a new one and updating all callers. Our reflexion is, as the vfs classes comes from the scmutil file, it seems okay to have scm related logic in that layer. What do you think? Do you have another layer in mind where the callback could be hooked to be able to catch any file-related operation? > > > We currently only apply the ward on 'open' operation. We will > > extend this to > > other operations like copy, creation and removal later. The current > > readonlyvfs > > seems to have the same shortcoming. > > > > diff --git a/mercurial/vfs.py b/mercurial/vfs.py > > --- a/mercurial/vfs.py > > +++ b/mercurial/vfs.py > > @@ -284,7 +284,7 @@ class vfs(abstractvfs): > > This class is used to hide the details of COW semantics and > > remote file access from higher level code. > > ''' > > - def __init__(self, base, expandpath=False, realpath=False): > > + def __init__(self, base, expandpath=False, realpath=False, > > ward=None): > > if expandpath: > > base = util.expandpath(base) > > if realpath: > > @@ -293,6 +293,11 @@ class vfs(abstractvfs): > > self.audit = pathutil.pathauditor(self.base) > > self.createmode = None > > self._trustnlink = None > > + # optional function to validate operation on file > > + # intended to be user for developer checks. > > + # > > + # XXX should be call for other things than 'open' > > + self._ward = ward > > > > @util.propertycache > > def _cansymlink(self): > > @@ -343,6 +348,9 @@ class vfs(abstractvfs): > > if not text and "b" not in mode: > > mode += "b" # for that other OS > > > > + if self._ward is not None: > > + self._ward(f, mode, atomictemp) > > Do you have any idea how to expand this ward() callback to the other > vfs > operations, which have different set of parameters? All other operations (move, delete, copy, etc) could be "translated" to 'read' or 'write' mode. An alternative would be to abstract the mode with a couple of symbolic constant "create, update, append, delete, …". What do you think? Cheers, Boris > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
