On Fri, Mar 27, 2020 at 12:21:50 -0400, Augie Fackler wrote:
> On Mar 27, 2020, at 10:56, Josef 'Jeff' Sipek <jef...@josefsipek.net> wrote:
> > On Fri, Mar 27, 2020 at 10:48:58 -0400, Josef 'Jeff' Sipek wrote:
> > > # HG changeset patch
> > > # User Josef 'Jeff' Sipek <jef...@josefsipek.net>
> > > # Date 1585319999 14400
> > > # Fri Mar 27 10:39:59 2020 -0400
> > > # Node ID f313b33e0341724093d866f0bf5478a28cad092a
> > > # Parent 4f4c2622ec748ce806c6577c30d4f1ae8660a0c2
> > > pathutil: document that dirs map type implies manifest/dirstate processing
> >
> > FWIW, this "argument type implies manifest or dirstate" seems like a hack.
> > I'm not familiar enough with python to know if I'm wrong here, but I'm open
> > to replacing this patch with somethig that adds a "type" argument. Then,
> > __init__ can assert/abort/etc. if it gets a mismatch. I imagine something
> > like:
> >
> > def __init__(self, map, is_dirstate, skip=None):
> > if is_dirstate:
> > assert isinstance(map, dict)
> > else:
> > assert isinstance(map, list)
> > ...code more or less as before...
> >
> > Would this be a good change?
>
> I'm conflicted. What we've got is akin to a type-overload in C++, but
> there's (obviously) no function overloading in Python. It might be clearer
> to convert this to three methods, eg (PEP484 hints included for clarity):
...
> Thoughts?
Agreed.
I already shared this on IRC last week, but I should share it here as well.
It is a work-in-progress. I haven't run the tests, but normal usage seems
to work WITHOUT the C or Rust code. Those still need to be changed and I
haven't even looked at how to implement class methods in those.
Jeff.
# HG changeset patch
# User Josef 'Jeff' Sipek <jef...@josefsipek.net>
# Date 1585358782 14400
# Fri Mar 27 21:26:22 2020 -0400
# Node ID c594db96ec9d119655d30305b5ac8d22e9d4a3bb
# Parent f313b33e0341724093d866f0bf5478a28cad092a
WIP: rewrite pathutils.dirs
diff --git a/hgext/git/manifest.py b/hgext/git/manifest.py
--- a/hgext/git/manifest.py
+++ b/hgext/git/manifest.py
@@ -120,7 +120,7 @@ class gittreemanifest(object):
@util.propertycache
def _dirs(self):
- return pathutil.dirs(self)
+ return pathutil.dirs.from_manifest(self)
def hasdir(self, dir):
return dir in self._dirs
@@ -232,7 +232,7 @@ class memgittreemanifestctx(object):
# just narrow?
assert not match or isinstance(match, matchmod.alwaysmatcher)
- touched_dirs = pathutil.dirs(list(self._pending_changes))
+ touched_dirs = pathutil.dirs.from_manifest(self._pending_changes)
trees = {
b'': self._tree,
}
diff --git a/hgext/narrow/narrowcommands.py b/hgext/narrow/narrowcommands.py
--- a/hgext/narrow/narrowcommands.py
+++ b/hgext/narrow/narrowcommands.py
@@ -278,7 +278,7 @@ def _narrow(
todelete.append(f)
elif f.startswith(b'meta/'):
dir = f[5:-13]
- dirs = sorted(pathutil.dirs({dir})) + [dir]
+ dirs = sorted(pathutil.dirs.from_manifest({dir})) + [dir]
include = True
for d in dirs:
visit = newmatch.visitdir(d)
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -186,7 +186,7 @@ def uncommit(ui, repo, *pats, **opts):
# if not everything tracked in that directory can be
# uncommitted.
if badfiles:
- badfiles -= {f for f in pathutil.dirs(eligible)}
+ badfiles -= {f for f in pathutil.dirs.from_manifest(eligible)}
for f in sorted(badfiles):
if f in s.clean:
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2823,7 +2823,7 @@ def remove(
progress.complete()
# warn about failure to delete explicit files/dirs
- deleteddirs = pathutil.dirs(deleted)
+ deleteddirs = pathutil.dirs.from_manifest(deleted)
files = m.files()
progress = ui.makeprogress(
_(b'deleting'), total=len(files), unit=_(b'files')
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -1577,11 +1577,11 @@ class dirstatemap(object):
@propertycache
def _dirs(self):
- return pathutil.dirs(self._map, b'r')
+ return pathutil.dirs.from_dirstate(self._map, b'r')
@propertycache
def _alldirs(self):
- return pathutil.dirs(self._map)
+ return pathutil.dirs.from_dirstate(self._map)
def _opendirstatefile(self):
fp, mode = txnutil.trypending(self._root, self._opener, self._filename)
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -491,7 +491,7 @@ class manifestdict(object):
@propertycache
def _dirs(self):
- return pathutil.dirs(self)
+ return pathutil.dirs.from_manifest(self)
def dirs(self):
return self._dirs
@@ -1103,7 +1103,7 @@ class treemanifest(object):
@propertycache
def _alldirs(self):
- return pathutil.dirs(self)
+ return pathutil.dirs.from_manifest(self)
def dirs(self):
return self._alldirs
diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -592,7 +592,7 @@ class patternmatcher(basematcher):
@propertycache
def _dirs(self):
- return set(pathutil.dirs(self._fileset))
+ return set(pathutil.dirs.from_manifest(self._fileset))
def visitdir(self, dir):
if self._prefix and dir in self._fileset:
@@ -763,7 +763,7 @@ class exactmatcher(basematcher):
@propertycache
def _dirs(self):
- return set(pathutil.dirs(self._fileset))
+ return set(pathutil.dirs.from_manifest(self._fileset))
def visitdir(self, dir):
return dir in self._dirs
@@ -1489,8 +1489,8 @@ def _rootsdirsandparents(kindpats):
p = set()
# Add the parents as non-recursive/exact directories, since they must be
# scanned to get to either the roots or the other exact directories.
- p.update(pathutil.dirs(d))
- p.update(pathutil.dirs(r))
+ p.update(pathutil.dirs.from_manifest(d))
+ p.update(pathutil.dirs.from_manifest(r))
# FIXME: all uses of this function convert these to sets, do so before
# returning.
diff --git a/mercurial/pathutil.py b/mercurial/pathutil.py
--- a/mercurial/pathutil.py
+++ b/mercurial/pathutil.py
@@ -285,23 +285,21 @@ def finddirs(path):
class dirs(object):
'''a multiset of directory names from a set of file paths'''
- def __init__(self, map, skip=None):
- '''
- a dict map indicates a dirstate while a list indicates a manifest
- '''
+ @classmethod
+ def from_dirstate(cls, dirstate_map, skip=None):
+ if skip is not None:
+ return cls(f for f, s in pycompat.iteritems(dirstate_map) if s[0]
!= skip)
+ return cls(dirstate)
+
+ @classmethod
+ def from_manifest(cls, files):
+ return cls(list(files))
+
+ def __init__(self, seq):
self._dirs = {}
addpath = self.addpath
- if isinstance(map, dict) and skip is not None:
- for f, s in pycompat.iteritems(map):
- if s[0] != skip:
- addpath(f)
- elif skip is not None:
- raise error.ProgrammingError(
- b"skip character is only supported with a dict source"
- )
- else:
- for f in map:
- addpath(f)
+ for f in seq:
+ addpath(f)
def addpath(self, path):
dirs = self._dirs
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -479,7 +479,7 @@ def rebuildfncache(ui, repo):
if b'treemanifest' in repo.requirements:
# This logic is safe if treemanifest isn't enabled, but also
# pointless, so we skip it if treemanifest isn't enabled.
- for dir in pathutil.dirs(seenfiles):
+ for dir in pathutil.dirs.from_manifest(seenfiles):
i = b'meta/%s/00manifest.i' % dir
d = b'meta/%s/00manifest.d' % dir
diff --git a/tests/test-dirs.py b/tests/test-dirs.py
--- a/tests/test-dirs.py
+++ b/tests/test-dirs.py
@@ -13,13 +13,13 @@ class dirstests(unittest.TestCase):
(b'a/a/a', [b'a', b'a/a', b'']),
(b'alpha/beta/gamma', [b'', b'alpha', b'alpha/beta']),
]:
- d = pathutil.dirs({})
+ d = pathutil.dirs.from_manifest({})
d.addpath(case)
self.assertEqual(sorted(d), sorted(want))
def testinvalid(self):
with self.assertRaises(ValueError):
- d = pathutil.dirs({})
+ d = pathutil.dirs.from_manifest({})
d.addpath(b'a//b')
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
