Merge lp:~mbp/bzr/2.0-498378-peek-lock into lp:bzr/2.0

[Martin Pool]

[Martin Pool (mbp)]

This won't fix the actual underlying problem of bug 498378 but it will give a clearer message.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/2.0-498378-peek-lock into lp:bzr/2.0.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This won't fix the actual underlying problem of bug 498378 but it will give a clearer message.
>

 review: approve
 merge: approve

I wonder if you could have a simple test that monkeypatched .peek() or
deleted the info file at the appropriate time, etc. (Or a child class
that inherits from LockDir and overrides peek to return None.)

Anyway, this is still better than what we have.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksvixUACgkQJdeBCYSNAAM8KACfYZNtbyyFy/x4RIZqV53MMXW+
PzcAnjREVIRkbEinlWXOfFSwcv1aKKXX
=gr/E
-----END PGP SIGNATURE-----

review: Approve

[Martin Pool (mbp)]

2009/12/22 John A Meinel <email address hidden>:
> I wonder if you could have a simple test that monkeypatched .peek() or
> deleted the info file at the appropriate time, etc. (Or a child class
> that inherits from LockDir and overrides peek to return None.)

You could, but I don't think it would be a good use of time to write it.

--
Martin <http://launchpad.net/~mbp/>

=== modified file 'NEWS'
--- NEWS	2009-12-21 16:45:39 +0000
+++ NEWS	2009-12-22 04:12:17 +0000
@@ -31,6 +31,9 @@
   This will likely have an impact on any other process that is serving for
   an extended period of time.  (John Arbash Meinel, #494406)
 
+* Give a clearer message if the lockdir disappears after being apparently
+  successfully taken.  (Martin Pool, #498378)
+
 * The 2a format wasn't properly restarting autopacks when something
   changed underneath it (like another autopack). Now concurrent
   autopackers will properly succeed. (John Arbash Meinel, #495000)

=== modified file 'bzrlib/lockdir.py'
--- bzrlib/lockdir.py	2009-07-27 05:24:02 +0000
+++ bzrlib/lockdir.py	2009-12-22 04:12:17 +0000
@@ -240,8 +240,16 @@
         # incorrect.  It's possible some other servers or filesystems will
         # have a similar bug allowing someone to think they got the lock
         # when it's already held.
+        #
+        # See <https://bugs.edge.launchpad.net/bzr/+bug/498378> for one case.
+        #
+        # Strictly the check is unnecessary and a waste of time for most
+        # people, but probably worth trapping if something is wrong.
         info = self.peek()
         self._trace("after locking, info=%r", info)
+        if info is None:
+            raise LockFailed(self, "lock was renamed into place, but "
+                "now is missing!")
         if info['nonce'] != self.nonce:
             self._trace("rename succeeded, "
                 "but lock is still held by someone else")