On 09/16/2016 01:10 PM, Stanislau Hlebik wrote:
# HG changeset patch
# User Stanislau Hlebik <st...@fb.com>
# Date 1473954996 25200
#      Thu Sep 15 08:56:36 2016 -0700
# Node ID 3456cba8a4879e74f3b928df321ce7e7c695fb4c
# Parent  f3fb030f0e4601561ac94137c7481694407db7b7
bundle2: add `bookmarks` part handler

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -154,8 +154,12 @@
 import sys

 from .i18n import _
+from .node import (
+    bin,
 from . import (
+    encoding,
@@ -1614,3 +1618,20 @@

     op.ui.debug('applied %i hgtags fnodes cache entries\n' % count)
+def handlebookmarks(op, inpart):

Please add some docstring to this part.

Greg, do you think we should also update the technical documentation specification of each part or should the docstring take care of this ?

+    dec = encoding.tolocal
+    bookmarks = {}
+    for bookmarknode in inpart.read().splitlines():
+        book, node = bookmarknode.rsplit(' ', 1)

We should probably stick on binary encoding for bookmark. This would avoid the need to encode/decode the hex and probably be more extensible in the future.

+        bookmarks[dec(book)] = node
+    if op.applybookmarks:
+        for bookmark, node in bookmarks.items():
+            if node:
+                op.repo._bookmarks[bookmark] = bin(node)
+            else:
+                del op.repo._bookmarks[bookmark]
+        op.repo._bookmarks.recordchange(op.gettransaction())

You wan to call gettransaction before doing any change. The pushrebase extension delay the locking until the transaction is actually fetched and this would expose you to a race here.

As you pointed on IRC, this is no longer trigger the pushkey hooks for bookmarks. This highlight the fact that:

(1) We should most probably introduces some hook dedicated to bookmark
(2) unfortunatly, we probably need to trigger the pushkey hook even if we are not using pushkey for BC reason :-(

+    else:
+        op.records.add('bookmarks', bookmarks)

We want to add record in all cases. The record are here to help other part of the push (eg: handler, hooks) to understand and collaborate with what happened.


Pierre-Yves David
Mercurial-devel mailing list

Reply via email to