[PATCH] hgwatchman: new experimental extension

[Martijn Pieters]

On 24 February 2016 at 18:19, Martin von Zweigbergk
<martinvonz at google.com> wrote:
> I guess the user can be expected to know or find out what watchman is,
> but maybe the commit message should explain what pywatchman is? I
> don't know what it is, even after spending a few minutes to try to
> find out. And why do we need a copy of it? (I'm guessing it is to
> avoid a config option pointing to it.)

Because this is an SDK to talk to watchman, which is installed as a
separate binary daemon that doesn't include those SDKs. By bundling it
we avoid requiring the extension user to installtwo things instead of
one (just watchman).

I'll update the commit message.

>> +def overridewalk(orig, self, match, subrepos, unknown, ignored, full=True):
>> +    '''Whenever full is False, ignored is False, and the watchman client is
>> +    available, use watchman combined with saved state to possibly return only a
>> +    subset of files.'''
>> +    def bail():
>> +        return orig(match, subrepos, unknown, ignored, full=True)
>> +
>> +    if full or ignored or not self._watchmanclient.available():
>> +        return bail()
>> +    clock, ignorehash, notefiles = self._watchmanstate.get()
>> +    if not clock:
>> +        if self._watchmanstate.crawl_on_invalidate:
>> +            return bail()
>> +        clock = 'c:0:0'
>> +        notefiles = []
>> +
>> +    def fwarn(f, msg):
>> +        self._ui.warn('%s: %s\n' % (self.pathto(f), msg))
>> +        return False
>> +
>> +    def badtype(mode):
>> +        kind = _('unknown')
>> +        if stat.S_ISCHR(mode):
>> +            kind = _('character device')
>> +        elif stat.S_ISBLK(mode):
>> +            kind = _('block device')
>> +        elif stat.S_ISFIFO(mode):
>> +            kind = _('fifo')
>> +        elif stat.S_ISSOCK(mode):
>> +            kind = _('socket')
>> +        elif stat.S_ISDIR(mode):
>> +            kind = _('directory')
>> +        return _('unsupported file type (type is %s)') % kind
>> +
>> +    ignore = self._ignore
>> +    dirignore = self._dirignore
>> +    if unknown:
>> +        if _hashignore(ignore) != ignorehash and clock != 'c:0:0':
>> +            # ignore list changed -- can't rely on watchman state any more
>> +            if self._watchmanstate.crawl_on_invalidate:
>> +                return bail()
>> +            notefiles = []
>> +            clock = 'c:0:0'
>> +    else:
>> +        # always ignore
>> +        ignore = util.always
>> +        dirignore = util.always
>> +
>> +    matchfn = match.matchfn
>> +    matchalways = match.always()
>> +    dmap = self._map
>> +    nonnormalset = None
>> +    if util.safehasattr(self, "_nonnormalset"):
>> +        nonnormalset = self._nonnormalset
>> +
>> +    copymap = self._copymap
>> +    getkind = stat.S_IFMT
>> +    dirkind = stat.S_IFDIR
>> +    regkind = stat.S_IFREG
>> +    lnkkind = stat.S_IFLNK
>> +    join = self._join
>> +    normcase = util.normcase
>> +    fresh_instance = False
>> +
>> +    exact = skipstep3 = False
>> +    if matchfn == match.exact:  # match.exact
>> +        exact = True
>> +        dirignore = util.always  # skip step 2
>> +    elif match.files() and not match.anypats():  # match.match, no patterns
>> +        skipstep3 = True
>> +
>> +    if not exact and self._checkcase:
>> +        # note that even though we could receive directory entries, we're only
>> +        # interested in checking if a file with the same name exists. So only
>> +        # normalize files if possible (Mercurial >= 3.4), not directories.
>> +        normalize = getattr(self, '_normalizefile', self._normalize)
>> +        skipstep3 = False
>> +    else:
>> +        normalize = None
>
> This function looks similar but different from dirstate.walk() (in
> both wanted and unwanted ways). I guess we'll refactor after it's in?

Yes, if you see opportunities to make this gel better, please do
refactor once we land this!

>> +    try:
>> +        if self._watchmanstate.crawl_on_invalidate:
>> +            # Use a short timeout to query the current clock.  If that
>> +            # takes too long then we assume that the service will be slow
>> +            # to answer our query.
>> +            # crawl_on_invalidate indicates that we prefer to crawl the
>> +            # tree ourselves because we can ignore portions that watchman
>
> nit: does "crawl the tree" mean the same thing as hg normally calls
> "walk the tree"? If so, use that terminology?
>
>> +        # clean isn't tested since it's set to True above
>> +        _cmpsets([modified, added, removed, deleted, unknown, ignored, clean],
>> +                 rv2)
>
> aka "_cmpsets(r, rv2)"?

Yes.

>> +class state(object):
>> +    def __init__(self, repo):
>> +        self._opener = repo.opener
>> +        self._ui = repo.ui
>> +        self._rootdir = pathutil.normasprefix(repo.root)
>> +        self._lastclock = None
>> +
>> +        self.mode = self._ui.config('watchman', 'mode', default='on')
>> +        self.crawl_on_invalidate = self._ui.configbool(
>> +            'watchman', 'crawl_on_invalidate', True)
>
> I don't think this option was documented. Should it be?

Good catch; the mercurial style linter missed this one. It probably
should be documented.

-- 
Martijn Pieters