[PATCH 2 of 2] dispatch: modifying an option list without copying it first makes a mess

Dan Villiom Podlaski Christiansen danchr at gmail.com
Fri Aug 27 09:45:26 CDT 2010


# HG changeset patch
# User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
# Date 1279885948 -7200
# Node ID 306046d9aa6cacc2bce7df34ad5357c7c2a31155
# Parent  407b0c1a71bd64743a35757e64df29300b345978
dispatch: modifying an option list without copying it first makes a mess.

When one or more option lists refer to the same list instance,
aliasing each other, updates to one list them will affects the others
as well. This can lead to changes to options for one command
inadvertently affecting other commands.

We try to ensure this distinctness of option lists in the main two
ways. First, we ensure it when adding commands to the command table
using a dict subclass. This subclass maintains a set of the instance
IDs of entries, and clones any duplicates. Second, and just to be on
the safe side, extensions.wrapcommand() always creates a copy of the
option list.

31f02288bbc4 caused the --mq option to be listed twice for the commit
command, if both mq and record were enabled. A test for this specific
bug is included.

diff --git a/doc/gendoc.py b/doc/gendoc.py
--- a/doc/gendoc.py
+++ b/doc/gendoc.py
@@ -49,7 +49,7 @@ def get_cmd(cmd):
     d['opts'] = list(get_opts(attr[1]))
 
     s = 'hg ' + cmds[0]
-    if len(attr) > 2:
+    if len(attr) > 2 and attr[2]:
         if not attr[2].startswith('hg'):
             s += ' ' + attr[2]
         else:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1848,11 +1848,8 @@ def help_(ui, name=None, with_version=Fa
             return
 
         # synopsis
-        if len(entry) > 2:
-            if entry[2].startswith('hg'):
-                ui.write("%s\n" % entry[2])
-            else:
-                ui.write('hg %s %s\n' % (aliases[0], entry[2]))
+        if entry[2]:
+            ui.write('hg %s %s\n' % (aliases[0], entry[2]))
         else:
             ui.write('hg %s\n' % aliases[0])
 
@@ -4470,6 +4467,9 @@ table = {
     "version": (version_, []),
 }
 
+# prevent option lists from aliasing eachother
+table = util.commanddict(table)
+
 norepo = ("clone init version help debugcommands debugcomplete debugdata"
           " debugindex debugindexdot debugdate debuginstall debugfsinfo"
           " debugpushkey")
diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -131,10 +131,8 @@ def wrapcommand(table, command, wrapper)
     wrap.__doc__ = getattr(origfn, '__doc__')
     wrap.__module__ = getattr(origfn, '__module__')
 
-    newentry = list(entry)
-    newentry[0] = wrap
-    table[key] = tuple(newentry)
-    return entry
+    table[key] = (wrap,) + entry[1:]
+    return table[key]
 
 def wrapfunction(container, funcname, wrapper):
     '''Wrap the function named funcname in container
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -80,6 +80,51 @@ def version():
     except ImportError:
         return 'unknown'
 
+class commanddict(dict):
+    '''
+    This is a subclass of dict that is useful for command tables. It ensures
+    that no entries in it share option list instances, by duplicating them
+    when necessary.
+    '''
+
+    __slots__ = ('ids')
+    def __init__(self, other={}):
+        super(commanddict, self).__init__()
+
+        self.ids = ids = set()
+        self.update(other)
+
+    def update(self, other):
+        for cmd, entry in other.iteritems():
+            self[cmd] = entry
+
+    def __setitem__(self, cmd, entry):
+        '''ensures that every option list is a distinct instance.'''
+        if not isinstance(cmd, basestring):
+            raise TypeError('invalid command key type ' + type(cmd).__name__)
+        elif not isinstance(entry, tuple):
+            raise TypeError('invalid command entry type ' +
+                            type(entry).__name__)
+        elif len(entry) == 2:
+            fn, opts = entry
+            desc = None
+        elif len(entry) == 3:
+            fn, opts, desc = entry
+            if desc:
+                desc = desc.split()
+                if desc[0] == 'hg':
+                    desc[:2] = []
+                desc = ' '.join(desc or []) or None
+        else:
+            raise TypeError('invalid command entry length %d' % len(entry))
+
+        if id(opts) in self.ids:
+            opts = list(opts)
+        self.ids.add(id(opts))
+
+        return super(commanddict, self).__setitem__(cmd, (fn, opts, desc))
+
+
 # used by parsedate
 defaultdateformats = (
     '%Y-%m-%d %H:%M:%S',
diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
--- a/tests/test-qrecord.t
+++ b/tests/test-qrecord.t
@@ -60,6 +60,13 @@ help (mq present)
   
   use "hg -v help qrecord" to show global options
 
+Test that the --mq option is only added once
+
+  $ hg help commit | grep -e --mq
+      --mq                   operate on patch repository
+  $ hg help record | grep -e --mq
+      --mq                   operate on patch repository
+
   $ hg init a
   $ cd a
 


More information about the Mercurial-devel mailing list