Patchwork [evolve-ext] bookmarks: update to use new bookmarks api via compatibility layer

login
register
mail settings
Submitter Ryan McElroy
Date May 10, 2015, 1:59 a.m.
Message ID <3a177ff325cc2d6fef34.1431223162@devbig105.prn2.facebook.com>
Download mbox | patch
Permalink /patch/9000/
State Accepted
Headers show

Comments

Ryan McElroy - May 10, 2015, 1:59 a.m.
# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1431030011 25200
#      Thu May 07 13:20:11 2015 -0700
# Branch stable
# Node ID 3a177ff325cc2d6fef34eaf35ace9e4e550deb86
# Parent  1218ad94a351b8294a4627a0ec9a27b68206c5b9
bookmarks: update to use new bookmarks api via compatibility layer

Tested against 3.4 and latest hg + upcoming bookmarks patches
Matt Mackall - May 10, 2015, 8:02 p.m.
On Sat, 2015-05-09 at 18:59 -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1431030011 25200
> #      Thu May 07 13:20:11 2015 -0700
> # Branch stable
> # Node ID 3a177ff325cc2d6fef34eaf35ace9e4e550deb86
> # Parent  1218ad94a351b8294a4627a0ec9a27b68206c5b9
> bookmarks: update to use new bookmarks api via compatibility layer
> 
> Tested against 3.4 and latest hg + upcoming bookmarks patches

I locally had to make 3 hacks to get "hg next" working for my usual
review process: activate / deactivate / repo._activebookmark. I only see
two of them here.
Ryan McElroy - May 11, 2015, 7:36 p.m.
On 5/10/2015 1:02 PM, Matt Mackall wrote:
> On Sat, 2015-05-09 at 18:59 -0700, Ryan McElroy wrote:
>> # HG changeset patch
>> # User Ryan McElroy <rmcelroy@fb.com>
>> # Date 1431030011 25200
>> #      Thu May 07 13:20:11 2015 -0700
>> # Branch stable
>> # Node ID 3a177ff325cc2d6fef34eaf35ace9e4e550deb86
>> # Parent  1218ad94a351b8294a4627a0ec9a27b68206c5b9
>> bookmarks: update to use new bookmarks api via compatibility layer
>>
>> Tested against 3.4 and latest hg + upcoming bookmarks patches
> I locally had to make 3 hacks to get "hg next" working for my usual
> review process: activate / deactivate / repo._activebookmark. I only see
> two of them here.
>

I can't find where activate is used inside of evolve -- are you sure 
that change was necessary?

searching through evolve's log -p for 'setcurrent' (as well as using 
histedit --all) was also unhelpful for me figuring out what that might 
have been necessary.

Several other extensions certainly need this -- I've added it to some of 
FB's extensions (like remotenames for example), any chance this is what 
you were seeing? (A subtle way to ask, "do you use remotenames?")
Pierre-Yves David - May 11, 2015, 9:31 p.m.
On 05/11/2015 12:36 PM, Ryan McElroy wrote:
> On 5/10/2015 1:02 PM, Matt Mackall wrote:
>> On Sat, 2015-05-09 at 18:59 -0700, Ryan McElroy wrote:
>>> # HG changeset patch
>>> # User Ryan McElroy <rmcelroy@fb.com>
>>> # Date 1431030011 25200
>>> #      Thu May 07 13:20:11 2015 -0700
>>> # Branch stable
>>> # Node ID 3a177ff325cc2d6fef34eaf35ace9e4e550deb86
>>> # Parent  1218ad94a351b8294a4627a0ec9a27b68206c5b9
>>> bookmarks: update to use new bookmarks api via compatibility layer

I've pushed that one to evolve main.

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -806,10 +806,9 @@  def relocate(repo, orig, dest, keepbranc
         try:
             if repo['.'].rev() != dest.rev():
                 merge.update(repo, dest, False, True, False)
-            if repo._bookmarkcurrent:
-                repo.ui.status(_("(leaving bookmark %s)\n") %
-                               repo._bookmarkcurrent)
-            bookmarks.unsetcurrent(repo)
+            if bmactive(repo):
+                repo.ui.status(_("(leaving bookmark %s)\n") % bmactive(repo))
+            bmdeactivate(repo)
             if keepbranch:
                 repo.dirstate.setbranch(orig.branch())
             r = merge.graft(repo, orig, orig.p1(), ['local', 'graft'])
@@ -864,7 +863,7 @@  def _bookmarksupdater(repo, oldid):
     """Return a callable update(newid) updating the current bookmark
     and bookmarks bound to oldid to newid.
     """
-    bm = bookmarks.readcurrent(repo)
+    bm = bmactive(repo)
     def updatebookmarks(newid):
         dirty = False
         if bm:
@@ -879,6 +878,19 @@  def _bookmarksupdater(repo, oldid):
             repo._bookmarks.write()
     return updatebookmarks
 
+### bookmarks api compatibility layer ###
+def bmdeactivate(repo):
+    try:
+        return bookmarks.deactivate(repo)
+    except AttributeError:
+        return bookmarks.unsetcurrent(repo)
+
+def bmactive(repo):
+    try:
+        return repo._activebookmark
+    except AttributeError:
+        return repo._bookmarkcurrent
+
 ### new command
 #############################
 metadataopts = [
@@ -1593,7 +1605,7 @@  shorttemplate = '[{rev}] {desc|firstline
 
 @command('^previous',
          [('B', 'move-bookmark', False,
-             _('Move current active bookmark after update'))],
+             _('Move active bookmark after update'))],
          '[-B]')
 def cmdprevious(ui, repo, **opts):
     """update to parent and display summary lines"""
@@ -1606,7 +1618,7 @@  def cmdprevious(ui, repo, **opts):
     displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
     if len(parents) == 1:
         p = parents[0]
-        bm = bookmarks.readcurrent(repo)
+        bm = bmactive(repo)
         shouldmove = opts.get('move_bookmark') and bm is not None
         ret = hg.update(repo, p.rev())
         if not ret:
@@ -1614,7 +1626,7 @@  def cmdprevious(ui, repo, **opts):
                 repo._bookmarks[bm] = p.node()
                 repo._bookmarks.write()
             else:
-                bookmarks.unsetcurrent(repo)
+                bmdeactivate(repo)
         displayer.show(p)
         return 0
     else:
@@ -1625,7 +1637,7 @@  def cmdprevious(ui, repo, **opts):
 
 @command('^next',
          [('B', 'move-bookmark', False,
-             _('Move current active bookmark after update'))],
+             _('Move active bookmark after update'))],
          '[-B]')
 def cmdnext(ui, repo, **opts):
     """update to child and display summary lines"""
@@ -1641,7 +1653,7 @@  def cmdnext(ui, repo, **opts):
         return 1
     if len(children) == 1:
         c = children[0]
-        bm = bookmarks.readcurrent(repo)
+        bm = bmactive(repo)
         shouldmove = opts.get('move_bookmark') and bm is not None
         ret = hg.update(repo, c.rev())
         if not ret:
@@ -1649,7 +1661,7 @@  def cmdnext(ui, repo, **opts):
                 repo._bookmarks[bm] = c.node()
                 repo._bookmarks.write()
             else:
-                bookmarks.unsetcurrent(repo)
+                bmdeactivate(repo)
         displayer.show(c)
         return 0
     else:
diff --git a/tests/test-evolve.t b/tests/test-evolve.t
--- a/tests/test-evolve.t
+++ b/tests/test-evolve.t
@@ -879,7 +879,7 @@  Evolve disables active bookmarks.
   $ hg bookmark testbookmark
   $ ls .hg/bookmarks*
   .hg/bookmarks
-  .hg/bookmarks.current
+  .hg/bookmarks.* (glob)
   $ hg evolve
   move:[8] a2
   atop:[10] a1__