| 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
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__().
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.
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. :)
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,