keyword question

Christian Ebert blacktrash at gmx.net
Wed Feb 17 17:49:18 CST 2010


Salut Benoît,

* Benoit Boissinot on Thursday, February 18, 2010 at 00:07:45 +0100
> I was just reading keyword.py and wondering why overwrite() needed to
> get the repo lock.

I've been wondering too since I wrapped repo.commitctx for
overwriting during commit.

> Could you explain?

The way I understand it the following should be safe:

# HG changeset patch
# User Christian Ebert <blacktrash at gmx.net>
# Date 1266449886 -3600
# Branch stable
# Node ID 3484ede77de1ecb4d5f50213b0337dbd3598bd55
# Parent  2c2d2f1354b44427976c2279e18d7c51a47a921b
keyword: remove unneeded locks

- kwcommitctx is inside the wlock of repo.commit: no lock
- _kwfwrite only need wlock

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -271,10 +271,9 @@
     wlock = lock = None
     try:
         wlock = repo.wlock()
-        lock = repo.lock()
         kwt.overwrite(None, expand, status[6])
     finally:
-        release(lock, wlock)
+        release(wlock)
 
 def demo(ui, repo, *args, **opts):
     '''print [keywordmaps] configuration and an expansion example
@@ -485,15 +484,9 @@
                 del self.commitctx
 
         def kwcommitctx(self, ctx, error=False):
-            wlock = lock = None
-            try:
-                wlock = self.wlock()
-                lock = self.lock()
-                n = super(kwrepo, self).commitctx(ctx, error)
-                kwt.overwrite(n, True, None)
-                return n
-            finally:
-                release(lock, wlock)
+            n = super(kwrepo, self).commitctx(ctx, error)
+            kwt.overwrite(n, True, None)
+            return n
 
     # monkeypatches
     def kwpatchfile_init(orig, self, ui, fname, opener,

 
All tests pass, but I am very insecure whether this introduces
some race conditions or possibilites of concurrent writes ...

Of course I would prefer it that way. Perhaps you could double
check/critized the reasoning I've outlined above.

One danger I can think of is that overwrite() forces a clean
dirstate when called for _kwfwrite (kwexpand, kwshrink commands),
it doesn't do it any more during commit since the commitctx
wrapping.

Thank you.

c
-- 
\black\trash movie    _C O W B O Y_  _C A N O E_  _C O M A_
                     Ein deutscher Western/A German Western

         --->> http://www.blacktrash.org/underdogma/ccc.php


More information about the Mercurial-devel mailing list