Preserve order of revisions after hg pullbackup

Summary:
This diff fixes hg pullbackup so it retrieves commits in the same order that there were pushed. This was caused by commits being stored in sets and dictionare which are by unorered types. These have been replaced by OrderedDict to mantain the order.

**Description of the task:**
Infinitepush is a mercurial extension that allows sharing commits easily. Among other things it also backs up all local commites that were made on devservers and laptops. That means that we always have up-to-date backup You can read more about it here - https://our.intern.facebook.com/intern/dex/mercurial-workflow/infinitepush/ .

Backup is triggered whenever mercurial transaction is triggered i.e. during commit, rebase or any other operation that changes mercurial repo. `hg pullbackup` is the way to pull backed up commits.

There is a problem. Let's say host A has this repo:

` o`
` |`
` o  C1`
` |  /`
` |   C2`
` |  /`
` o`

So we have two draft commits: C1 and C2. C2 is probably an older commit. But if we restore it on another host we can get another output:

` o`
` |`
` o  C2`
` |  /`
` |   C1`
` |  /`
` o `

This happens because `hg sl` orders commits by it's revision number in revlog - https://www.mercurial-scm.org/wiki/Revlog .

The point of the task is to make sure commits are ordered by their creation date.

Link to comment: http://www.facebook.com/groups/scm/permalink/1504906422892306/?comment_id=1505017576214524&reply_comment_id=1506426179406997

Reviewed By: StanislavGlebik

Differential Revision: D6884670

fbshipit-source-id: 3281dbc1e25e24662a4b6ba78b96b85d5bae78c9

hgext/infinitepush/__init__.py

@@ -419,7 +419,9 @@ def wireprotolistkeyspatterns(repo, proto, namespace, patterns):
 def localrepolistkeys(orig, self, namespace, patterns=None):
     if namespace == 'bookmarks' and patterns:
         index = self.bundlestore.index
-        results = {}
+        # Using sortdict instead of a dictionary to ensure that bookmaks are
+        # restored in the same order after a pullbackup. See T24417531
+        results = util.sortdict()
         bookmarks = orig(self, namespace)
         for pattern in patterns:
             results.update(index.getbookmarks(pattern))

hgext/infinitepush/backupcommands.py

@@ -85,6 +85,7 @@ osutil = policy.importmod(r'osutil')
 
 cmdtable = {}
 command = registrar.command(cmdtable)
+
 revsetpredicate = registrar.revsetpredicate()
 templatekeyword = registrar.templatekeyword()
 localoverridesfile = 'generated.infinitepushbackups.rc'
@@ -95,8 +96,8 @@ backupbookmarktuple = namedtuple('backupbookmarktuple',
 
 class backupstate(object):
     def __init__(self):
-        self.heads = set()
-        self.localbookmarks = {}
+        self.heads = util.sortdict()
+        self.localbookmarks = util.sortdict()
 
     def empty(self):
         return not self.heads and not self.localbookmarks
@@ -228,7 +229,6 @@ def backup(ui, repo, dest=None, **opts):
     Local heads are saved remotely as:
         infinitepush/backups/USERNAME/HOST/REPOROOT/heads/HEAD_HASH
     """
-
     if opts.get('background'):
         _dobackgroundbackup(ui, repo, dest)
         return 0
@@ -272,9 +272,12 @@ def restore(ui, repo, dest=None, **opts):
 
     __, backupstate = allbackupstates.popitem()
     pullcmd, pullopts = _getcommandandoptions('^pull')
-    # pull backuped heads and nodes that are pointed by bookmarks
-    pullopts['rev'] = list(backupstate.heads |
-                           set(backupstate.localbookmarks.values()))
+    # Pull backuped heads and nodes that are pointed by bookmarks.
+    # Note that we are avoiding the use of set() because we want to pull
+    # revisions in the same order
+    pullopts['rev'] = list(backupstate.heads) \
+                      + [x for x in backupstate.localbookmarks.values()
+                            if x not in backupstate.heads]
     if dest:
         pullopts['source'] = dest
     result = pullcmd(ui, repo, **pullopts)
@@ -292,7 +295,7 @@ def restore(ui, repo, dest=None, **opts):
     # manually write local backup state and flag to not autobackup
     # just after we restored, which would be pointless
     _writelocalbackupstate(repo.vfs,
-                           list(backupstate.heads),
+                           backupstate.heads.values(),
                            backupstate.localbookmarks)
     repo.ignoreautobackup = True
 
@@ -557,7 +560,7 @@ def _dobackup(ui, repo, dest, **opts):
     afterbackupheads = _backupheads(ui, repo)
     other = _getremote(repo, ui, dest, **opts)
     outgoing, badhexnodes = _getrevstobackup(repo, ui, other,
-                                             afterbackupheads - bkpstate.heads)
+                                        afterbackupheads - set(bkpstate.heads))
     # If remotefilelog extension is enabled then there can be nodes that we
     # can't backup. In this case let's remove them from afterbackupheads
     afterbackupheads.difference_update(badhexnodes)
@@ -568,8 +571,8 @@ def _dobackup(ui, repo, dest, **opts):
     afterbackuplocalbooks = _filterbookmarks(
         afterbackuplocalbooks, repo, afterbackupheads)
 
-    newheads = afterbackupheads - bkpstate.heads
-    removedheads = bkpstate.heads - afterbackupheads
+    newheads = afterbackupheads - set(bkpstate.heads)
+    removedheads = set(bkpstate.heads) - afterbackupheads
     newbookmarks = _dictdiff(afterbackuplocalbooks, bkpstate.localbookmarks)
     removedbookmarks = _dictdiff(bkpstate.localbookmarks, afterbackuplocalbooks)
 
@@ -606,7 +609,7 @@ def _dobackup(ui, repo, dest, **opts):
 
         if backup:
             _sendbundle(bundler, other)
-            _writelocalbackupstate(repo.vfs, afterbackupheads,
+            _writelocalbackupstate(repo.vfs, list(afterbackupheads),
                                    afterbackuplocalbooks)
             if ui.config('infinitepushbackup', 'savelatestbackupinfo'):
                 _writelocalbackupinfo(repo.vfs, **afterbackupinfo)
@@ -739,7 +742,7 @@ def _downloadbackupstate(ui, other, sourcereporoot, sourcehostname, namingmgr):
                 bookname = parsed.localbookmark
                 allbackupstates[key].localbookmarks[bookname] = hexnode
             else:
-                allbackupstates[key].heads.add(hexnode)
+                allbackupstates[key].heads[hexnode] = hexnode
         else:
             ui.warn(_('wrong format of backup bookmark: %s') % book)
 

hgext/infinitepush/sqlindexapi.py

@@ -17,6 +17,8 @@ from indexapi import (
     indexexception,
 )
 
+from mercurial import util
+
 def _convertbookmarkpattern(pattern):
     pattern = pattern.replace('_', '\\_')
     pattern = pattern.replace('%', '\\%')
@@ -285,10 +287,13 @@ class sqlindexapi(indexapi):
         query = _convertbookmarkpattern(query)
         self.sqlcursor.execute(
             "SELECT bookmark, node from bookmarkstonode WHERE "
-            "reponame = %s AND bookmark LIKE %s",
+            "reponame = %s AND bookmark LIKE %s "
+            # Bookmarks have to be restored in the same order of creation
+            # See T24417531
+            "ORDER BY time ASC",
             params=(self.reponame, query))
         result = self.sqlcursor.fetchall()
-        bookmarks = {}
+        bookmarks = util.sortdict()
         for row in result:
             if len(row) != 2:
                 self.log.info("Bad row returned: %s" % row)

mercurial/pushkey.py

@@ -12,6 +12,7 @@ from . import (
     encoding,
     obsolete,
     phases,
+    util,
 )
 
 def _nslist(repo):
@@ -54,7 +55,10 @@ def encodekeys(keys):
 
 def decodekeys(data):
     """decode the content of a pushkey namespace from exchange over the wire"""
-    result = {}
+    # Note that the order is required in some cases. E.g. pullbackup needs to
+    # retrieve commits in the same order of creation to mantain the order of
+    # revision codes. See T24417531
+    result = util.sortdict()
     for l in data.splitlines():
         k, v = l.split('\t')
         result[decode(k)] = decode(v)

tests/test-fb-hgext-infinitepush-backup-sql.t

@@ -24,6 +24,7 @@
   finished in \d+\.(\d+)? seconds (re)
   $ mkcommit commitwithbookmark
   $ hg book abook
+  $ sleep 1 # Resolution of the database is in seconds. This avoid test flakiness
   $ hg pushbackup
   starting backup .* (re)
   searching for changes
@@ -35,6 +36,32 @@
   bookmark	node	reponame
   infinitepush/backups/test/.*\$TESTTMP/client/bookmarks/abook	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	babar (re)
   infinitepush/backups/test/.*\$TESTTMP/client/heads/5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	babar (re)
+
+Create a few more commits to test that pullbackup preserves order
+  $ hg up -q 0
+  $ mkcommit anothercommit > /dev/null
+  $ hg pushbackup -q
+  $ hg up -q 0
+  $ sleep 1 # Resolution of the database is in seconds. This avoid test flakiness
+  $ mkcommit anothercommit2 > /dev/null
+  $ hg pushbackup -q
+  $ hg log -T '{rev}:{node}\n'
+  3:e1c1c1f2871f70bd24f941ebfec59f14adf7a13d
+  2:f0d24965f49e87fc581a603dee76196f433444ff
+  1:5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f
+  0:67145f4663446a9580364f70034fea6e21293b6f
+
+Pull backup and check that commits are in the same order
+  $ cd ..
+  $ hg clone -q ssh://user@dummy/server client2
+  $ cd client2
+  $ hg pullbackup -q
+  $ hg log -T '{rev}:{node}\n'
+  3:e1c1c1f2871f70bd24f941ebfec59f14adf7a13d
+  2:f0d24965f49e87fc581a603dee76196f433444ff
+  1:5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f
+  0:67145f4663446a9580364f70034fea6e21293b6f
+
 Create a server with different name that connects to the same db
   $ cd ..
   $ rm -rf server
@@ -49,14 +76,20 @@ Go to client, delete backup state and run pushbackup. Make sure that it doesn't
   $ hg pushbackup
   starting backup .* (re)
   searching for changes
-  remote: pushing 2 commits:
+  remote: pushing 4 commits:
   remote:     67145f466344  initialcommit
   remote:     5ea4271ca0f0  commitwithbookmark
+  remote:     f0d24965f49e  anothercommit
+  remote:     e1c1c1f2871f  anothercommit2
   finished in \d+\.(\d+)? seconds (re)
   $ mysql -h $DBHOST -P $DBPORT -D $DBNAME -u $DBUSER $DBPASSOPT -e 'SELECT bookmark, node, reponame from bookmarkstonode'
   bookmark	node	reponame
   infinitepush/backups/test/.*\$TESTTMP/client/bookmarks/abook	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	babar (re)
   infinitepush/backups/test/.*\$TESTTMP/client/heads/5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	babar (re)
+  infinitepush/backups/test/.*\$TESTTMP/client/heads/e1c1c1f2871f70bd24f941ebfec59f14adf7a13d	e1c1c1f2871f70bd24f941ebfec59f14adf7a13d	babar (re)
+  infinitepush/backups/test/.*\$TESTTMP/client/heads/f0d24965f49e87fc581a603dee76196f433444ff	f0d24965f49e87fc581a603dee76196f433444ff	babar (re)
   infinitepush/backups/test/.*\$TESTTMP/client/bookmarks/abook	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	newserver (re)
   infinitepush/backups/test/.*\$TESTTMP/client/heads/5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	5ea4271ca0f0cda5477241ae95ffc1fa7056ee6f	newserver (re)
+  infinitepush/backups/test/.*\$TESTTMP/client/heads/e1c1c1f2871f70bd24f941ebfec59f14adf7a13d	e1c1c1f2871f70bd24f941ebfec59f14adf7a13d	newserver (re)
+  infinitepush/backups/test/.*\$TESTTMP/client/heads/f0d24965f49e87fc581a603dee76196f433444ff	f0d24965f49e87fc581a603dee76196f433444ff	newserver (re)
 #endif