Patchwork [4,of,7,bm-refactor,V2] commands: remove locking code since the bookmarks module does that

login
register
mail settings
Submitter Sean Farley
Date June 21, 2017, 8:45 p.m.
Message ID <c77f6d15508faf1d9575.1498077911@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/21596/
State Changes Requested
Headers show

Comments

Sean Farley - June 21, 2017, 8:45 p.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1497998203 25200
#      Tue Jun 20 15:36:43 2017 -0700
# Branch bm-refactor
# Node ID c77f6d15508faf1d95753c7e1f52b7b5d3d63cf4
# Parent  03933080a047f8438b1e400b513a7909779382a8
commands: remove locking code since the bookmarks module does that
via Mercurial-devel - June 22, 2017, 5:48 a.m.
On Wed, Jun 21, 2017 at 1:45 PM, Sean Farley <sean@farley.io> wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1497998203 25200
> #      Tue Jun 20 15:36:43 2017 -0700
> # Branch bm-refactor
> # Node ID c77f6d15508faf1d95753c7e1f52b7b5d3d63cf4
> # Parent  03933080a047f8438b1e400b513a7909779382a8
> commands: remove locking code since the bookmarks module does that

This sounds obsolete. Should probably say something about context
managers instead.

>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> index 9d4088b..085dcfb 100644
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -965,41 +965,28 @@ def bookmark(ui, repo, *names, **opts):
>          raise error.Abort(_("--rev is incompatible with --rename"))
>      if not names and (delete or rev):
>          raise error.Abort(_("bookmark name required"))
>
>      if delete or rename or names or inactive:
> -        wlock = lock = tr = None
> -        try:
> -            wlock = repo.wlock()
> -            lock = repo.lock()
> -            cur = repo.changectx('.').node()
> -            marks = repo._bookmarks
> +        with repo.wlock(), repo.lock(), repo.transaction('bookmark') as tr:
>              if delete:
> -                tr = repo.transaction('bookmark')
>                  bookmarks.delete(repo, tr, names)
>              elif rename:
> -                tr = repo.transaction('bookmark')
>                  if not names:
>                      raise error.Abort(_("new bookmark name required"))
>                  elif len(names) > 1:
>                      raise error.Abort(_("only one new bookmark name allowed"))
>                  bookmarks.rename(repo, tr, rename, names[0], force, inactive)
>              elif names:
> -                tr = repo.transaction('bookmark')
>                  bookmarks.addbookmarks(repo, tr, names, rev, force, inactive)
>              elif inactive:
> -                if len(marks) == 0:
> +                if len(repo._bookmarks) == 0:
>                      ui.status(_("no bookmarks set\n"))
>                  elif not repo._activebookmark:
>                      ui.status(_("no active bookmark\n"))
>                  else:
>                      bookmarks.deactivate(repo)
> -            if tr is not None:
> -                marks.recordchange(tr)
> -                tr.close()
> -        finally:
> -            lockmod.release(tr, lock, wlock)
>      else: # show bookmarks
>          fm = ui.formatter('bookmarks', opts)
>          hexfn = fm.hexfunc
>          marks = repo._bookmarks
>          if len(marks) == 0 and fm.isplain():
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Sean Farley - June 22, 2017, 8:31 p.m.
Martin von Zweigbergk <martinvonz@google.com> writes:

> On Wed, Jun 21, 2017 at 1:45 PM, Sean Farley <sean@farley.io> wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1497998203 25200
>> #      Tue Jun 20 15:36:43 2017 -0700
>> # Branch bm-refactor
>> # Node ID c77f6d15508faf1d95753c7e1f52b7b5d3d63cf4
>> # Parent  03933080a047f8438b1e400b513a7909779382a8
>> commands: remove locking code since the bookmarks module does that
>
> This sounds obsolete. Should probably say something about context
> managers instead.

Argh, I completely forgot to rewrite the commit messages.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
index 9d4088b..085dcfb 100644
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -965,41 +965,28 @@  def bookmark(ui, repo, *names, **opts):
         raise error.Abort(_("--rev is incompatible with --rename"))
     if not names and (delete or rev):
         raise error.Abort(_("bookmark name required"))
 
     if delete or rename or names or inactive:
-        wlock = lock = tr = None
-        try:
-            wlock = repo.wlock()
-            lock = repo.lock()
-            cur = repo.changectx('.').node()
-            marks = repo._bookmarks
+        with repo.wlock(), repo.lock(), repo.transaction('bookmark') as tr:
             if delete:
-                tr = repo.transaction('bookmark')
                 bookmarks.delete(repo, tr, names)
             elif rename:
-                tr = repo.transaction('bookmark')
                 if not names:
                     raise error.Abort(_("new bookmark name required"))
                 elif len(names) > 1:
                     raise error.Abort(_("only one new bookmark name allowed"))
                 bookmarks.rename(repo, tr, rename, names[0], force, inactive)
             elif names:
-                tr = repo.transaction('bookmark')
                 bookmarks.addbookmarks(repo, tr, names, rev, force, inactive)
             elif inactive:
-                if len(marks) == 0:
+                if len(repo._bookmarks) == 0:
                     ui.status(_("no bookmarks set\n"))
                 elif not repo._activebookmark:
                     ui.status(_("no active bookmark\n"))
                 else:
                     bookmarks.deactivate(repo)
-            if tr is not None:
-                marks.recordchange(tr)
-                tr.close()
-        finally:
-            lockmod.release(tr, lock, wlock)
     else: # show bookmarks
         fm = ui.formatter('bookmarks', opts)
         hexfn = fm.hexfunc
         marks = repo._bookmarks
         if len(marks) == 0 and fm.isplain():