Patchwork [STABLE] push: continue without locking on lock failure other than EEXIST (issue5882)

login
register
mail settings
Submitter Yuya Nishihara
Date May 15, 2018, 1:41 p.m.
Message ID <11d6f26077c93e36a311.1526391661@mimosa>
Download mbox | patch
Permalink /patch/31610/
State Accepted
Headers show

Comments

Yuya Nishihara - May 15, 2018, 1:41 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1526389975 -32400
#      Tue May 15 22:12:55 2018 +0900
# Branch stable
# Node ID 11d6f26077c93e36a311db92d54fa372808e0f0d
# Parent  273ea09f65500ea7936afe2983bf6a126c5bf4e6
push: continue without locking on lock failure other than EEXIST (issue5882)

This code was added by 3f5e75c22585 "push: make locking of source optional
(issue3684)", but EACCES isn't the only error that could be triggered by
filesystem permission. I think catching LockUnavailable is more appropriate
than testing errno value by caller.
Augie Fackler - May 20, 2018, 2 a.m.
> On May 15, 2018, at 9:41 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1526389975 -32400
> #      Tue May 15 22:12:55 2018 +0900
> # Branch stable
> # Node ID 11d6f26077c93e36a311db92d54fa372808e0f0d
> # Parent  273ea09f65500ea7936afe2983bf6a126c5bf4e6
> push: continue without locking on lock failure other than EEXIST (issue5882)

queued, thanks

> 
> This code was added by 3f5e75c22585 "push: make locking of source optional
> (issue3684)", but EACCES isn't the only error that could be triggered by
> filesystem permission. I think catching LockUnavailable is more appropriate
> than testing errno value by caller.
> 
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -8,7 +8,6 @@
> from __future__ import absolute_import
> 
> import collections
> -import errno
> import hashlib
> 
> from .i18n import _
> @@ -513,9 +512,7 @@ def push(repo, remote, force=False, revs
>         pushop.trmanager = transactionmanager(pushop.repo,
>                                               'push-response',
>                                               pushop.remote.url())
> -    except IOError as err:
> -        if err.errno != errno.EACCES:
> -            raise
> +    except error.LockUnavailable as err:
>         # source repo cannot be locked.
>         # We do not abort the push, but just disable the local phase
>         # synchronisation.
> diff --git a/tests/test-phases-exchange.t b/tests/test-phases-exchange.t
> --- a/tests/test-phases-exchange.t
> +++ b/tests/test-phases-exchange.t
> @@ -1307,6 +1307,18 @@ 2. Test that failed phases movement are 
>   [1]
>   $ chmod -R +w .hg
> 
> +3. Test that push is prevented if lock was already acquired (not a permission
> +error, but EEXIST)
> +
> +  $ touch .hg/store/lock
> +  $ hg push ../Phi --config ui.timeout=1
> +  pushing to ../Phi
> +  waiting for lock on repository $TESTTMP/Upsilon held by ''
> +  abort: repository $TESTTMP/Upsilon: timed out waiting for lock held by ''
> +  (lock might be very busy)
> +  [255]
> +  $ rm .hg/store/lock
> +
>   $ cd ..
> 
> #endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -8,7 +8,6 @@ 
 from __future__ import absolute_import
 
 import collections
-import errno
 import hashlib
 
 from .i18n import _
@@ -513,9 +512,7 @@  def push(repo, remote, force=False, revs
         pushop.trmanager = transactionmanager(pushop.repo,
                                               'push-response',
                                               pushop.remote.url())
-    except IOError as err:
-        if err.errno != errno.EACCES:
-            raise
+    except error.LockUnavailable as err:
         # source repo cannot be locked.
         # We do not abort the push, but just disable the local phase
         # synchronisation.
diff --git a/tests/test-phases-exchange.t b/tests/test-phases-exchange.t
--- a/tests/test-phases-exchange.t
+++ b/tests/test-phases-exchange.t
@@ -1307,6 +1307,18 @@  2. Test that failed phases movement are 
   [1]
   $ chmod -R +w .hg
 
+3. Test that push is prevented if lock was already acquired (not a permission
+error, but EEXIST)
+
+  $ touch .hg/store/lock
+  $ hg push ../Phi --config ui.timeout=1
+  pushing to ../Phi
+  waiting for lock on repository $TESTTMP/Upsilon held by ''
+  abort: repository $TESTTMP/Upsilon: timed out waiting for lock held by ''
+  (lock might be very busy)
+  [255]
+  $ rm .hg/store/lock
+
   $ cd ..
 
 #endif