Patchwork [7,of,9,STABLE] shelve: execute checkunfinished inside wlock scope

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Dec. 1, 2015, 6:18 p.m.
Message ID <f001138597fb71160d2a.1448993925@feefifofum>
Download mbox | patch
Permalink /patch/11721/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Dec. 1, 2015, 6:18 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1448993528 -32400
#      Wed Dec 02 03:12:08 2015 +0900
# Branch stable
# Node ID f001138597fb71160d2acce9c5838a2f0d03c1e8
# Parent  064cd627433570e5efc70c9de69d68d278a13bb9
shelve: execute checkunfinished inside wlock scope

Before this patch, "hg shelve" of shelve extension executes
'cmdutil.checkunfinished()' before acquisition of wlock.

It may cause unintentional result, if another command runs parallelly
(see also issue4368).

To avoid this issue, this patch executes 'cmdutil.checkunfinished()'
inside wlock scope of "hg shelve".

This also fixes issue4957, because now 'cmdutil.checkunfinished()'
isn't invoked at "hg shelve" with options below:

  --cleanup
  --delete
  --list
  --patch
  --stat
Pierre-Yves David - Dec. 3, 2015, 9:07 a.m.
On 12/01/2015 10:18 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1448993528 -32400
> #      Wed Dec 02 03:12:08 2015 +0900
> # Branch stable
> # Node ID f001138597fb71160d2acce9c5838a2f0d03c1e8
> # Parent  064cd627433570e5efc70c9de69d68d278a13bb9
> shelve: execute checkunfinished inside wlock scope

I've pushed patch 1 to 7. Nice work. I like the "def _doX trick"

Should we add a devel-warning for checkunfinished outside of lock?

Patch 8 fail to apply. Can you rebase on the clowncopter and resend?
Katsunori FUJIWARA - Dec. 3, 2015, 1:47 p.m.
At Thu, 03 Dec 2015 01:07:51 -0800,
Pierre-Yves David wrote:
> 
> 
> 
> On 12/01/2015 10:18 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1448993528 -32400
> > #      Wed Dec 02 03:12:08 2015 +0900
> > # Branch stable
> > # Node ID f001138597fb71160d2acce9c5838a2f0d03c1e8
> > # Parent  064cd627433570e5efc70c9de69d68d278a13bb9
> > shelve: execute checkunfinished inside wlock scope
> 
> I've pushed patch 1 to 7. Nice work. I like the "def _doX trick"
> 
> Should we add a devel-warning for checkunfinished outside of lock?

I think so, too (and I'll do :-))

> Patch 8 fail to apply. Can you rebase on the clowncopter and resend?

OK, I'll send revised series for ones not yet pushed to clowncopter.

> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -226,6 +226,7 @@  def createcmd(ui, repo, pats, opts):
     """subcommand that creates a new shelve"""
     wlock = repo.wlock()
     try:
+        cmdutil.checkunfinished(repo)
         return _docreatecmd(ui, repo, pats, opts)
     finally:
         lockmod.release(wlock)
@@ -801,8 +802,6 @@  def shelvecmd(ui, repo, *pats, **opts):
     To delete specific shelved changes, use ``--delete``. To delete
     all shelved changes, use ``--cleanup``.
     '''
-    cmdutil.checkunfinished(repo)
-
     allowables = [
         ('addremove', set(['create'])), # 'create' is pseudo action
         ('cleanup', set(['cleanup'])),