Annoying API change in dirstate.py

[Greg Ward]

Between 1.4 and 1.5, there was an API change to dirstate.walk() and
dirstate.status() that breaks two of my extensions.  The changeset was

changeset:   10178:24ce8f0c0a39
user:        Augie Fackler <durin42 at gmail.com>
date:        Thu Dec 31 17:19:30 2009 -0600
files:       contrib/perf.py hgext/inotify/__init__.py
mercurial/context.py mercurial/dirstate.py mercurial/localrepo.py
description:
dirstate: don't check state of subrepo directories

and the API change in particular is:

--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -425,7 +425,7 @@
[...]
-    def walk(self, match, unknown, ignored):
+    def walk(self, match, subrepos, unknown, ignored):
[...]
-    def status(self, match, ignored, clean, unknown):
+    def status(self, match, subrepos, ignored, clean, unknown):

Just to see how painful it would be, I started hacking up bfiles so it
can work with dirstate from either 1.4 or 1.5.  Here is the best I
have been able to come up with (this is scattered fragments of a patch
to bfiles.py):

+_dirstate_version = None
+
+def _get_dirstate_version(ui):
+    global _dirstate_version
+    if _dirstate_version is None:
+        varnames = dirstate.dirstate.status.im_func.func_code.co_varnames
+        if 'subrepos' in varnames:
+            ui.debug('bfiles: dirstate.status() takes \'subrepos\':
this is hg 1.5\n')
+            _dirstate_version = '1.5'
+        else:
+            ui.debug('bfiles: dirstate.status() does not take
\'subrepos\': this is hg 1.4\n')
+            _dirstate_version = '1.4'
+
+    return _dirstate_version
[...]
+    if _get_dirstate_version(ui) == '1.5':
+        status_args = ([], False, False, False)
+    else:
+        status_args = (False, False, False)
+
     matcher = match.match(repo.root, repo.getcwd(), pats, default='relpath')
-    status = bfdirstate.status(matcher, False, False, False)
+    status = bfdirstate.status(matcher, *status_args)
[...]
+    if _get_dirstate_version(ui) == '1.5':
+        walk_args = ([], False, False)
+    else:
+        walk_args = (False, False)
     matcher = _get_standin_matcher(repo, pats, opts)
-    for standin in repo.dirstate.walk(matcher, False, False):
+    for standin in repo.dirstate.walk(matcher, *walk_args):
[...]

You get the idea.  I have to do this everywhere I call
dirstate.status() or dirstate.walk().  Gross.  Yuck.  Blecch.  And in
my other affected extension, I wrap dirstate.status(), which makes
this even more painful.  Now multiply my pain by the number of
extension authors out there calling or wrapping dirstate.status()
and/or dirstate.walk().  No fun!

I would like to propose a more extension-friendly API change, namely:

--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -430,7 +430,7 @@
                 return True
         return False

-    def walk(self, match, subrepos, unknown, ignored):
+    def walk(self, match, unknown, ignored, subrepos=None):
         '''
         Walk recursively through the directory tree, finding all files
         matched by match.
@@ -580,7 +580,7 @@
         del results['.hg']
         return results

-    def status(self, match, subrepos, ignored, clean, unknown):
+    def status(self, match, ignored, clean, unknown, subrepos=None):
         '''Determine the status of the working copy relative to the
         dirstate and return a tuple of lists (unsure, modified, added,
         removed, deleted, unknown, ignored, clean), where:

This will require a little extra work in dirstate.walk(), but IMHO
it's worth it to preserve the sanity of extension authors.

I'm going to work on a patch now.  Please let me know if you object!

Greg