Patchwork [1,of,4] match: make _fileroots a @propertycache and rename it to _fileset

login
register
mail settings
Submitter via Mercurial-devel
Date May 18, 2017, 6:03 p.m.
Message ID <48275b23f01283fdf1ae.1495130623@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/20686/
State Accepted
Headers show

Comments

via Mercurial-devel - May 18, 2017, 6:03 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1495123477 25200
#      Thu May 18 09:04:37 2017 -0700
# Node ID 48275b23f01283fdf1ae6e630c2c63660528edfc
# Parent  0d6b3572ad924103128bb9cd296000fc6fd821ef
match: make _fileroots a @propertycache and rename it to _fileset

The files in the set are not necesserily roots of anything. Making it
a @propertycache will help towards extracting a base class for
matchers.
Yuya Nishihara - May 19, 2017, 2:32 p.m.
On Thu, 18 May 2017 11:03:43 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495123477 25200
> #      Thu May 18 09:04:37 2017 -0700
> # Node ID 48275b23f01283fdf1ae6e630c2c63660528edfc
> # Parent  0d6b3572ad924103128bb9cd296000fc6fd821ef
> match: make _fileroots a @propertycache and rename it to _fileset

Looks good overall. Queued, thanks.

> @@ -399,7 +402,6 @@
>                  return matcher.visitdir(self._path)
>              return matcher.visitdir(self._path + "/" + dir)
>          self.visitdir = visitdir
> -        self._fileroots = set(self._files)

This change should be fine as self._fileset is never be cached in __init__().
via Mercurial-devel - May 19, 2017, 3:37 p.m.
On Fri, May 19, 2017 at 7:32 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Thu, 18 May 2017 11:03:43 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1495123477 25200
>> #      Thu May 18 09:04:37 2017 -0700
>> # Node ID 48275b23f01283fdf1ae6e630c2c63660528edfc
>> # Parent  0d6b3572ad924103128bb9cd296000fc6fd821ef
>> match: make _fileroots a @propertycache and rename it to _fileset
>
> Looks good overall. Queued, thanks.
>
>> @@ -399,7 +402,6 @@
>>                  return matcher.visitdir(self._path)
>>              return matcher.visitdir(self._path + "/" + dir)
>>          self.visitdir = visitdir
>> -        self._fileroots = set(self._files)
>
> This change should be fine as self._fileset is never be cached in __init__().

Even if it were, this would overwrite it (which is what this was meant
to do), no?

Anyway, I plan on dropping these few lines in a later patch, either
just sacrificing what the feature this code was for or rewriting it in
a better way (I'm not sure yet). I have another 20-30 matcher patches
coming up.
Yuya Nishihara - May 19, 2017, 4:07 p.m.
On Fri, 19 May 2017 08:37:06 -0700, Martin von Zweigbergk wrote:
> On Fri, May 19, 2017 at 7:32 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Thu, 18 May 2017 11:03:43 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> >> # HG changeset patch
> >> # User Martin von Zweigbergk <martinvonz@google.com>
> >> # Date 1495123477 25200
> >> #      Thu May 18 09:04:37 2017 -0700
> >> # Node ID 48275b23f01283fdf1ae6e630c2c63660528edfc
> >> # Parent  0d6b3572ad924103128bb9cd296000fc6fd821ef
> >> match: make _fileroots a @propertycache and rename it to _fileset
> >
> > Looks good overall. Queued, thanks.
> >
> >> @@ -399,7 +402,6 @@
> >>                  return matcher.visitdir(self._path)
> >>              return matcher.visitdir(self._path + "/" + dir)
> >>          self.visitdir = visitdir
> >> -        self._fileroots = set(self._files)
> >
> > This change should be fine as self._fileset is never be cached in __init__().
> 
> Even if it were, this would overwrite it (which is what this was meant
> to do), no?

Doh, I thought subdirmatcher would call match.__init__() before doing the
hack, (and if match.__init__() accessed to _fileset, set(_files) != _fileset.)
But I was wrong. :)
via Mercurial-devel - May 19, 2017, 4:11 p.m.
On Fri, May 19, 2017 at 9:07 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 19 May 2017 08:37:06 -0700, Martin von Zweigbergk wrote:
>> On Fri, May 19, 2017 at 7:32 AM, Yuya Nishihara <yuya@tcha.org> wrote:
>> > On Thu, 18 May 2017 11:03:43 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> >> # HG changeset patch
>> >> # User Martin von Zweigbergk <martinvonz@google.com>
>> >> # Date 1495123477 25200
>> >> #      Thu May 18 09:04:37 2017 -0700
>> >> # Node ID 48275b23f01283fdf1ae6e630c2c63660528edfc
>> >> # Parent  0d6b3572ad924103128bb9cd296000fc6fd821ef
>> >> match: make _fileroots a @propertycache and rename it to _fileset
>> >
>> > Looks good overall. Queued, thanks.
>> >
>> >> @@ -399,7 +402,6 @@
>> >>                  return matcher.visitdir(self._path)
>> >>              return matcher.visitdir(self._path + "/" + dir)
>> >>          self.visitdir = visitdir
>> >> -        self._fileroots = set(self._files)
>> >
>> > This change should be fine as self._fileset is never be cached in __init__().
>>
>> Even if it were, this would overwrite it (which is what this was meant
>> to do), no?
>
> Doh, I thought subdirmatcher would call match.__init__() before doing the

It will eventually call into its base class's __init__() once I'm done
(although that will be a "basematcher" class, not the "match class
itself)).

> hack, (and if match.__init__() accessed to _fileset, set(_files) != _fileset.)
> But I was wrong. :)

There's a first time for everything :-) (I say that with admiration,
no sarcasm.)

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -41,7 +41,7 @@ 
     m = copy.copy(match)
     lfile = lambda f: lfutil.standin(f) in manifest
     m._files = filter(lfile, m._files)
-    m._fileroots = set(m._files)
+    m._fileset = set(m._files)
     m._always = False
     origmatchfn = m.matchfn
     m.matchfn = lambda f: lfile(f) and origmatchfn(f)
@@ -56,7 +56,7 @@ 
     notlfile = lambda f: not (lfutil.isstandin(f) or lfutil.standin(f) in
             manifest or f in excluded)
     m._files = filter(notlfile, m._files)
-    m._fileroots = set(m._files)
+    m._fileset = set(m._files)
     m._always = False
     origmatchfn = m.matchfn
     m.matchfn = lambda f: notlfile(f) and origmatchfn(f)
@@ -368,7 +368,7 @@ 
             elif m._files[i] not in ctx and repo.wvfs.isdir(standin):
                 m._files.append(standin)
 
-        m._fileroots = set(m._files)
+        m._fileset = set(m._files)
         m._always = False
         origmatchfn = m.matchfn
         def lfmatchfn(f):
@@ -644,7 +644,7 @@ 
             m = copy.copy(match)
             lfile = lambda f: lfutil.standin(f) in manifest
             m._files = [lfutil.standin(f) for f in m._files if lfile(f)]
-            m._fileroots = set(m._files)
+            m._fileset = set(m._files)
             origmatchfn = m.matchfn
             def matchfn(f):
                 lfile = lfutil.splitstandin(f)
@@ -767,7 +767,7 @@ 
                 else:
                     matchfiles.append(f)
             m._files = matchfiles
-            m._fileroots = set(m._files)
+            m._fileset = set(m._files)
             origmatchfn = m.matchfn
             def matchfn(f):
                 lfile = lfutil.splitstandin(f)
diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -188,7 +188,6 @@ 
                 return True
 
         self.matchfn = m
-        self._fileroots = set(self._files)
 
     def __call__(self, fn):
         return self.matchfn(fn)
@@ -235,8 +234,12 @@ 
         return self._files
 
     @propertycache
+    def _fileset(self):
+        return set(self._files)
+
+    @propertycache
     def _dirs(self):
-        return set(util.dirs(self._fileroots)) | {'.'}
+        return set(util.dirs(self._fileset)) | {'.'}
 
     def visitdir(self, dir):
         '''Decides whether a directory should be visited based on whether it
@@ -250,7 +253,7 @@ 
         This function's behavior is undefined if it has returned False for
         one of the dir's parent directories.
         '''
-        if self.prefix() and dir in self._fileroots:
+        if self.prefix() and dir in self._fileset:
             return 'all'
         if dir in self._excluderoots:
             return False
@@ -261,16 +264,16 @@ 
             not any(parent in self._includeroots
                     for parent in util.finddirs(dir))):
             return False
-        return (not self._fileroots or
-                '.' in self._fileroots or
-                dir in self._fileroots or
+        return (not self._fileset or
+                '.' in self._fileset or
+                dir in self._fileset or
                 dir in self._dirs or
-                any(parentdir in self._fileroots
+                any(parentdir in self._fileset
                     for parentdir in util.finddirs(dir)))
 
     def exact(self, f):
         '''Returns True if f is in .files().'''
-        return f in self._fileroots
+        return f in self._fileset
 
     def anypats(self):
         '''Matcher uses patterns or include/exclude.'''
@@ -399,7 +402,6 @@ 
                 return matcher.visitdir(self._path)
             return matcher.visitdir(self._path + "/" + dir)
         self.visitdir = visitdir
-        self._fileroots = set(self._files)
 
     def abs(self, f):
         return self._matcher.abs(self._path + "/" + f)
@@ -428,8 +430,8 @@ 
         # inexact case matches are treated as exact, and not noted without -v.
         if self._files:
             roots, dirs = _rootsanddirs(self._kp)
-            self._fileroots = set(roots)
-            self._fileroots.update(dirs)
+            self._fileset = set(roots)
+            self._fileset.update(dirs)
 
     def _normalize(self, patterns, default, root, cwd, auditor):
         self._kp = super(icasefsmatcher, self)._normalize(patterns, default,