Patchwork [1,of,3] bundle2: use a sorted dict for holding parameters

login
register
mail settings
Submitter Gregory Szorc
Date July 17, 2016, 10:18 p.m.
Message ID <3987b98844077a160447.1468793901@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15925/
State Accepted
Headers show

Comments

Gregory Szorc - July 17, 2016, 10:18 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468792260 25200
#      Sun Jul 17 14:51:00 2016 -0700
# Node ID 3987b98844077a160447a9280f23b4fb47ef2b79
# Parent  1cc5a918b7d8acdc918809d74842fecc128c4ec4
bundle2: use a sorted dict for holding parameters

An upcoming change that introduces a 2nd part parameter to a part
reveals that `hg debugbundle` isn't deterministic because parameters
are stored on n plain, unsorted dict.

While we could change that command to sort before output, I think
the more important underlying issue is that bundle2 reading is taking
an ordered data structure and converting it to an unordered one.
Plugging in util.sortdict() fixes that problem while preserving API
compatibility.

This patch also appears to shine light on the fact that we don't
have tests verifying parts with multiple parameters roundtrip
correctly. That would be a good thing to test (and fuzz)... someday.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -685,17 +685,17 @@  class unbundle20(unpackermixin):
                                          % paramssize)
         if paramssize:
             params = self._readexact(paramssize)
             params = self._processallparams(params)
         return params
 
     def _processallparams(self, paramsblock):
         """"""
-        params = {}
+        params = util.sortdict()
         for p in paramsblock.split(' '):
             p = p.split('=', 1)
             p = [urlreq.unquote(i) for i in p]
             if len(p) < 2:
                 p.append(None)
             self._processparam(*p)
             params[p[0]] = p[1]
         return params
@@ -1110,18 +1110,18 @@  class unbundlepart(unpackermixin):
         return _unpack(format, data)
 
     def _initparams(self, mandatoryparams, advisoryparams):
         """internal function to setup all logic related parameters"""
         # make it read only to prevent people touching it by mistake.
         self.mandatoryparams = tuple(mandatoryparams)
         self.advisoryparams  = tuple(advisoryparams)
         # user friendly UI
-        self.params = dict(self.mandatoryparams)
-        self.params.update(dict(self.advisoryparams))
+        self.params = util.sortdict(self.mandatoryparams)
+        self.params.update(self.advisoryparams)
         self.mandatorykeys = frozenset(p[0] for p in mandatoryparams)
 
     def _payloadchunks(self, chunknum=0):
         '''seek to specified chunk and start yielding data'''
         if len(self._chunkindex) == 0:
             assert chunknum == 0, 'Must start with chunk 0'
             self._chunkindex.append((0, super(unbundlepart, self).tell()))
         else: