This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1345

dirstate: add explicit methods for querying directories (API)
ClosedPublic

Authored by mbthomas on Nov 8 2017, 12:31 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG61888bd0b300: dirstate: add explicit methods for querying directories (API)
rHG88ed6f5f6c86: dirstate: add explicit methods for querying directories (API)
Summary

The set-like object returned by dirstate.dirs may be difficult for other
implementations of the dirstate to provide, and is unnecessary as it is
only ever used for contains. Instead, provide an explicit method for
testing for a directory.

.. api::

dirstate no longer provides a `dirs()` method.  To test for the existence of
a directory in the dirstate, use `dirstate.hasdir(dirname)`.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mbthomas created this revision.Nov 8 2017, 12:31 PM
mbolin added a subscriber: mbolin.Nov 9 2017, 8:24 PM
mbolin added inline comments.
mercurial/dirstate.py
393

Can you just use hasdir here instead so there's only one thing to override?

397

Here too?

mbthomas updated this revision to Diff 3442.Nov 13 2017, 7:18 AM
durin42 accepted this revision.Nov 13 2017, 5:48 PM
durin42 added a subscriber: durin42.

Looks good. For bonus points do one more upload with the right .. api:: annotation for the releasenotes extension.

This revision is now accepted and ready to land.Nov 13 2017, 5:48 PM
mbthomas added inline comments.Nov 15 2017, 3:56 AM
mercurial/dirstate.py
393

The external dirstate object only provides hasdir(), however the internal object needs to provide two objects as there are two behaviours it needs to expose:

hastrackeddir() considers only files that are not marked as removed. This is used almost everywhere.

hasdir() additionally considers directories that contain only removed files. This is used in one place below (line 803). I don't understand what _walkexplicit is trying to do, so I haven't changed it. I'm not convinced _walkexplicit is correct, though.

mbthomas edited the summary of this revision. (Show Details)Nov 15 2017, 4:08 AM
mbthomas retitled this revision from dirstate: add explicit methods for querying directories to dirstate: add explicit methods for querying directories (API).
mbthomas updated this revision to Diff 3519.
durin42 accepted this revision.Nov 17 2017, 5:22 PM
Closed by commit rHG88ed6f5f6c86: dirstate: add explicit methods for querying directories (API) (authored by mbthomas). · Explain WhyNov 17 2017, 5:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/perf.py (10 lines)
Mhgext/largefiles/reposetup.py (4 lines)
Mmercurial/dirstate.py (62 lines)

Diff 3619

contrib/perf.py

fm.end() fm.end()
@command('perfdirs', formatteropts) @command('perfdirs', formatteropts)
def perfdirs(ui, repo, **opts): def perfdirs(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
dirstate = repo.dirstate dirstate = repo.dirstate
'a' in dirstate 'a' in dirstate
def d(): def d():
dirstate.dirs() dirstate.hasdir('a')
del dirstate._map.dirs del dirstate._map._dirs
timer(d) timer(d)
fm.end() fm.end()
@command('perfdirstate', formatteropts) @command('perfdirstate', formatteropts)
def perfdirstate(ui, repo, **opts): def perfdirstate(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
"a" in repo.dirstate "a" in repo.dirstate
def d(): def d():
repo.dirstate.invalidate() repo.dirstate.invalidate()
"a" in repo.dirstate "a" in repo.dirstate
timer(d) timer(d)
fm.end() fm.end()
@command('perfdirstatedirs', formatteropts) @command('perfdirstatedirs', formatteropts)
def perfdirstatedirs(ui, repo, **opts): def perfdirstatedirs(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
"a" in repo.dirstate "a" in repo.dirstate
def d(): def d():
"a" in repo.dirstate._map.dirs repo.dirstate.hasdir("a")
del repo.dirstate._map.dirs del repo.dirstate._map._dirs
timer(d) timer(d)
fm.end() fm.end()
@command('perfdirstatefoldmap', formatteropts) @command('perfdirstatefoldmap', formatteropts)
def perfdirstatefoldmap(ui, repo, **opts): def perfdirstatefoldmap(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
dirstate = repo.dirstate dirstate = repo.dirstate
'a' in dirstate 'a' in dirstate
def d(): def d():
dirstate._map.filefoldmap.get('a') dirstate._map.filefoldmap.get('a')
del dirstate._map.filefoldmap del dirstate._map.filefoldmap
timer(d) timer(d)
fm.end() fm.end()
@command('perfdirfoldmap', formatteropts) @command('perfdirfoldmap', formatteropts)
def perfdirfoldmap(ui, repo, **opts): def perfdirfoldmap(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
dirstate = repo.dirstate dirstate = repo.dirstate
'a' in dirstate 'a' in dirstate
def d(): def d():
dirstate._map.dirfoldmap.get('a') dirstate._map.dirfoldmap.get('a')
del dirstate._map.dirfoldmap del dirstate._map.dirfoldmap
del dirstate._map.dirs del dirstate._map._dirs
timer(d) timer(d)
fm.end() fm.end()
@command('perfdirstatewrite', formatteropts) @command('perfdirstatewrite', formatteropts)
def perfdirstatewrite(ui, repo, **opts): def perfdirstatewrite(ui, repo, **opts):
timer, fm = gettimer(ui, opts) timer, fm = gettimer(ui, opts)
ds = repo.dirstate ds = repo.dirstate
"a" in ds "a" in ds

hgext/largefiles/reposetup.py

if not working: if not working:
return files return files
newfiles = [] newfiles = []
dirstate = self.dirstate dirstate = self.dirstate
for f in files: for f in files:
sf = lfutil.standin(f) sf = lfutil.standin(f)
if sf in dirstate: if sf in dirstate:
newfiles.append(sf) newfiles.append(sf)
elif sf in dirstate.dirs(): elif dirstate.hasdir(sf):
# Directory entries could be regular or # Directory entries could be regular or
# standin, check both # standin, check both
newfiles.extend((f, sf)) newfiles.extend((f, sf))
else: else:
newfiles.append(f) newfiles.append(f)
return newfiles return newfiles
m = copy.copy(match) m = copy.copy(match)
m._files = tostandins(m._files) m._files = tostandins(m._files)
result = orig(node1, node2, m, ignored, clean, unknown, result = orig(node1, node2, m, ignored, clean, unknown,
listsubrepos) listsubrepos)
if working: if working:
def sfindirstate(f): def sfindirstate(f):
sf = lfutil.standin(f) sf = lfutil.standin(f)
dirstate = self.dirstate dirstate = self.dirstate
return sf in dirstate or sf in dirstate.dirs() return sf in dirstate or dirstate.hasdir(sf)
match._files = [f for f in match._files match._files = [f for f in match._files
if sfindirstate(f)] if sfindirstate(f)]
# Don't waste time getting the ignored and unknown # Don't waste time getting the ignored and unknown
# files from lfdirstate # files from lfdirstate
unsure, s = lfdirstate.status(match, subrepos=[], unsure, s = lfdirstate.status(match, subrepos=[],
ignored=False, ignored=False,
clean=listclean, clean=listclean,

mercurial/dirstate.py

if inst.errno != errno.ENOENT: if inst.errno != errno.ENOENT:
raise raise
return "default" return "default"
@property @property
def _pl(self): def _pl(self):
return self._map.parents() return self._map.parents()
def dirs(self): def hasdir(self, d):
return self._map.dirs return self._map.hastrackeddir(d)
@rootcache('.hgignore') @rootcache('.hgignore')
def _ignore(self): def _ignore(self):
files = self._ignorefiles() files = self._ignorefiles()
if not files: if not files:
return matchmod.never(self._root, '') return matchmod.never(self._root, '')
pats = ['include:%s' % f for f in files] pats = ['include:%s' % f for f in files]
def copies(self): def copies(self):
return self._map.copymap return self._map.copymap
def _addpath(self, f, state, mode, size, mtime): def _addpath(self, f, state, mode, size, mtime):
oldstate = self[f] oldstate = self[f]
if state == 'a' or oldstate == 'r': if state == 'a' or oldstate == 'r':
scmutil.checkfilename(f) scmutil.checkfilename(f)
if f in self._map.dirs: if self._map.hastrackeddir(f):
mbolinUnsubmitted
Not Done

Can you just use hasdir here instead so there's only one thing to override?

mbolin: Can you just use `hasdir` here instead so there's only one thing to override?
mbthomasAuthorUnsubmitted
Not Done

The external dirstate object only provides hasdir(), however the internal object needs to provide two objects as there are two behaviours it needs to expose:

hastrackeddir() considers only files that are not marked as removed. This is used almost everywhere.

hasdir() additionally considers directories that contain only removed files. This is used in one place below (line 803). I don't understand what _walkexplicit is trying to do, so I haven't changed it. I'm not convinced _walkexplicit is correct, though.

mbthomas: The external dirstate object only provides `hasdir()`, however the internal object needs to…
raise error.Abort(_('directory %r already in dirstate') % f) raise error.Abort(_('directory %r already in dirstate') % f)
# shadows # shadows
for d in util.finddirs(f): for d in util.finddirs(f):
if d in self._map.dirs: if self._map.hastrackeddir(d):
mbolinUnsubmitted
Not Done

Here too?

mbolin: Here too?
break break
entry = self._map.get(d) entry = self._map.get(d)
if entry is not None and entry[0] != 'r': if entry is not None and entry[0] != 'r':
raise error.Abort( raise error.Abort(
_('file %r in dirstate clashes with %r') % (d, f)) _('file %r in dirstate clashes with %r') % (d, f))
self._dirty = True self._dirty = True
self._updatedfiles.add(f) self._updatedfiles.add(f)
self._map.addfile(f, oldstate, state, mode, size, mtime) self._map.addfile(f, oldstate, state, mode, size, mtime)
del files[i] del files[i]
j += 1 j += 1
if not files or '.' in files: if not files or '.' in files:
files = ['.'] files = ['.']
results = dict.fromkeys(subrepos) results = dict.fromkeys(subrepos)
results['.hg'] = None results['.hg'] = None
alldirs = None
for ff in files: for ff in files:
# constructing the foldmap is expensive, so don't do it for the # constructing the foldmap is expensive, so don't do it for the
# common case where files is ['.'] # common case where files is ['.']
if normalize and ff != '.': if normalize and ff != '.':
nf = normalize(ff, False, True) nf = normalize(ff, False, True)
else: else:
nf = ff nf = ff
if nf in results: if nf in results:
else: else:
badfn(ff, badtype(kind)) badfn(ff, badtype(kind))
if nf in dmap: if nf in dmap:
results[nf] = None results[nf] = None
except OSError as inst: # nf not found on disk - it is dirstate only except OSError as inst: # nf not found on disk - it is dirstate only
if nf in dmap: # does it exactly match a missing file? if nf in dmap: # does it exactly match a missing file?
results[nf] = None results[nf] = None
else: # does it match a missing directory? else: # does it match a missing directory?
if alldirs is None: if self._map.hasdir(nf):
alldirs = util.dirs(dmap._map)
if nf in alldirs:
if matchedir: if matchedir:
matchedir(nf) matchedir(nf)
notfoundadd(nf) notfoundadd(nf)
else: else:
badfn(ff, encoding.strtolocal(inst.strerror)) badfn(ff, encoding.strtolocal(inst.strerror))
# Case insensitive filesystems cannot rely on lstat() failing to detect # Case insensitive filesystems cannot rely on lstat() failing to detect
# a case-only rename. Prune the stat object for any file that does not # a case-only rename. Prune the stat object for any file that does not
The dirstate also provides the following views onto the state: The dirstate also provides the following views onto the state:
- `nonnormalset` is a set of the filenames that have state other - `nonnormalset` is a set of the filenames that have state other
than 'normal', or are normal but have an mtime of -1 ('normallookup'). than 'normal', or are normal but have an mtime of -1 ('normallookup').
- `otherparentset` is a set of the filenames that are marked as coming - `otherparentset` is a set of the filenames that are marked as coming
from the second parent when the dirstate is currently being merged. from the second parent when the dirstate is currently being merged.
- `dirs` is a set-like object containing all the directories that contain
files in the dirstate, excluding any files that are marked as removed.
- `filefoldmap` is a dict mapping normalized filenames to the denormalized - `filefoldmap` is a dict mapping normalized filenames to the denormalized
form that they appear as in the dirstate. form that they appear as in the dirstate.
- `dirfoldmap` is a dict mapping normalized directory names to the - `dirfoldmap` is a dict mapping normalized directory names to the
denormalized form that they appear as in the dirstate. denormalized form that they appear as in the dirstate.
""" """
def __init__(self, ui, opener, root): def __init__(self, ui, opener, root):
self.copymap = {} self.copymap = {}
self._map self._map
return self.copymap return self.copymap
def clear(self): def clear(self):
self._map.clear() self._map.clear()
self.copymap.clear() self.copymap.clear()
self.setparents(nullid, nullid) self.setparents(nullid, nullid)
util.clearcachedproperty(self, "dirs") util.clearcachedproperty(self, "_dirs")
util.clearcachedproperty(self, "_alldirs")
util.clearcachedproperty(self, "filefoldmap") util.clearcachedproperty(self, "filefoldmap")
util.clearcachedproperty(self, "dirfoldmap") util.clearcachedproperty(self, "dirfoldmap")
util.clearcachedproperty(self, "nonnormalset") util.clearcachedproperty(self, "nonnormalset")
util.clearcachedproperty(self, "otherparentset") util.clearcachedproperty(self, "otherparentset")
def iteritems(self): def iteritems(self):
return self._map.iteritems() return self._map.iteritems()
return self._map.keys() return self._map.keys()
def preload(self): def preload(self):
"""Loads the underlying data, if it's not already loaded""" """Loads the underlying data, if it's not already loaded"""
self._map self._map
def addfile(self, f, oldstate, state, mode, size, mtime): def addfile(self, f, oldstate, state, mode, size, mtime):
"""Add a tracked file to the dirstate.""" """Add a tracked file to the dirstate."""
if oldstate in "?r" and "dirs" in self.__dict__: if oldstate in "?r" and "_dirs" in self.__dict__:
self.dirs.addpath(f) self._dirs.addpath(f)
if oldstate == "?" and "_alldirs" in self.__dict__:
self._alldirs.addpath(f)
self._map[f] = dirstatetuple(state, mode, size, mtime) self._map[f] = dirstatetuple(state, mode, size, mtime)
if state != 'n' or mtime == -1: if state != 'n' or mtime == -1:
self.nonnormalset.add(f) self.nonnormalset.add(f)
if size == -2: if size == -2:
self.otherparentset.add(f) self.otherparentset.add(f)
def removefile(self, f, oldstate, size): def removefile(self, f, oldstate, size):
""" """
Mark a file as removed in the dirstate. Mark a file as removed in the dirstate.
The `size` parameter is used to store sentinel values that indicate The `size` parameter is used to store sentinel values that indicate
the file's previous state. In the future, we should refactor this the file's previous state. In the future, we should refactor this
to be more explicit about what that state is. to be more explicit about what that state is.
""" """
if oldstate not in "?r" and "dirs" in self.__dict__: if oldstate not in "?r" and "_dirs" in self.__dict__:
self.dirs.delpath(f) self._dirs.delpath(f)
if oldstate == "?" and "_alldirs" in self.__dict__:
self._alldirs.addpath(f)
if "filefoldmap" in self.__dict__: if "filefoldmap" in self.__dict__:
normed = util.normcase(f) normed = util.normcase(f)
self.filefoldmap.pop(normed, None) self.filefoldmap.pop(normed, None)
self._map[f] = dirstatetuple('r', 0, size, 0) self._map[f] = dirstatetuple('r', 0, size, 0)
self.nonnormalset.add(f) self.nonnormalset.add(f)
def dropfile(self, f, oldstate): def dropfile(self, f, oldstate):
""" """
Remove a file from the dirstate. Returns True if the file was Remove a file from the dirstate. Returns True if the file was
previously recorded. previously recorded.
""" """
exists = self._map.pop(f, None) is not None exists = self._map.pop(f, None) is not None
if exists: if exists:
if oldstate != "r" and "dirs" in self.__dict__: if oldstate != "r" and "_dirs" in self.__dict__:
self.dirs.delpath(f) self._dirs.delpath(f)
if "_alldirs" in self.__dict__:
self._alldirs.delpath(f)
if "filefoldmap" in self.__dict__: if "filefoldmap" in self.__dict__:
normed = util.normcase(f) normed = util.normcase(f)
self.filefoldmap.pop(normed, None) self.filefoldmap.pop(normed, None)
self.nonnormalset.discard(f) self.nonnormalset.discard(f)
return exists return exists
def clearambiguoustimes(self, files, now): def clearambiguoustimes(self, files, now):
for f in files: for f in files:
f = {} f = {}
normcase = util.normcase normcase = util.normcase
for name, s in self._map.iteritems(): for name, s in self._map.iteritems():
if s[0] != 'r': if s[0] != 'r':
f[normcase(name)] = name f[normcase(name)] = name
f['.'] = '.' # prevents useless util.fspath() invocation f['.'] = '.' # prevents useless util.fspath() invocation
return f return f
@propertycache def hastrackeddir(self, d):
def dirs(self): """
"""Returns a set-like object containing all the directories in the Returns True if the dirstate contains a tracked (not removed) file
current dirstate. in this directory.
"""
return d in self._dirs
def hasdir(self, d):
""" """
Returns True if the dirstate contains a file (tracked or removed)
in this directory.
"""
return d in self._alldirs
@propertycache
def _dirs(self):
return util.dirs(self._map, 'r') return util.dirs(self._map, 'r')
@propertycache
def _alldirs(self):
return util.dirs(self._map)
def _opendirstatefile(self): def _opendirstatefile(self):
fp, mode = txnutil.trypending(self._root, self._opener, self._filename) fp, mode = txnutil.trypending(self._root, self._opener, self._filename)
if self._pendingmode is not None and self._pendingmode != mode: if self._pendingmode is not None and self._pendingmode != mode:
fp.close() fp.close()
raise error.Abort(_('working directory state may be ' raise error.Abort(_('working directory state may be '
'changed parallelly')) 'changed parallelly'))
self._pendingmode = mode self._pendingmode = mode
return fp return fp
def identity(self): def identity(self):
self._map self._map
return self.identity return self.identity
@propertycache @propertycache
def dirfoldmap(self): def dirfoldmap(self):
f = {} f = {}
normcase = util.normcase normcase = util.normcase
for name in self.dirs: for name in self._dirs:
f[normcase(name)] = name f[normcase(name)] = name
return f return f