Merge lp:~mbp/bzr/733136-lock_url into lp:bzr/2.3

[Martin Pool]

avoid UnboundLocal error when locking fails (bug 733136)

This fixes bug 733136 by just moving the variable declaration to a place it will definitely be seen.

[Martin Pool (mbp)]

The fix isn't especially shallow, but I am for not writing a test for it. If people feel I should, I can.

[John A Meinel (jameinel)]

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

On 3/31/2011 7:42 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/733136-lock_url into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #733136 in Bazaar: "UnboundLocalError: local variable 'lock_url' in wait_lock"
> https://bugs.launchpad.net/bzr/+bug/733136
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/733136-lock_url/+merge/55682
>
> This does a kind of shallow fix for bug 733136 by just moving the variable declaration to a place it will definitely be seen.

What if we pull this out into a helper function, so we don't clutter the
function itself?

Note that if this is getting triggered, there is another bug. Which is
that QBzr is being unable to get write-access to the config when it
wants to set its values. So it sounds like a lock-race or lock
confusion. (bzr is locking and qbzr is locking on top of it.)

Anyway, the fix seems fine, just cleaner in a helper. Would you want to
target 2.3 instead?

John
=:->

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

iEYEARECAAYFAk2UKn8ACgkQJdeBCYSNAAPlDQCgq+DWQ/GdJ3d/pykjYOPrCbQ+
9wkAoK68su5hgMw2Xg8iE7OVCuzOOjsv
=OyWm
-----END PGP SIGNATURE-----

review: Approve

[Martin Pool (mbp)]

rebase to 2.3

review: Approve

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/lockdir.py'
--- bzrlib/lockdir.py	2011-01-12 20:31:15 +0000
+++ bzrlib/lockdir.py	2011-03-31 09:08:48 +0000
@@ -537,6 +537,17 @@
             hook(hook_result)
         return result
 
+    def lock_url_for_display(self):
+        """Give a nicely-printable representation of the URL of this lock."""
+        # As local lock urls are correct we display them.
+        # We avoid displaying remote lock urls.
+        lock_url = self.transport.abspath(self.path)
+        if lock_url.startswith('file://'):
+            lock_url = lock_url.split('.bzr/')[0]
+        else:
+            lock_url = ''
+        return lock_url
+
     def wait_lock(self, timeout=None, poll=None, max_attempts=None):
         """Wait a certain period for a lock.
 
@@ -566,6 +577,7 @@
         deadline_str = None
         last_info = None
         attempt_count = 0
+        lock_url = self.lock_url_for_display()
         while True:
             attempt_count += 1
             try:
@@ -590,13 +602,6 @@
                 if deadline_str is None:
                     deadline_str = time.strftime('%H:%M:%S',
                                                  time.localtime(deadline))
-                # As local lock urls are correct we display them.
-                # We avoid displaying remote lock urls.
-                lock_url = self.transport.abspath(self.path)
-                if lock_url.startswith('file://'):
-                    lock_url = lock_url.split('.bzr/')[0]
-                else:
-                    lock_url = ''
                 user, hostname, pid, time_ago = formatted_info
                 msg = ('%s lock %s '        # lock_url
                     'held by '              # start

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- doc/en/release-notes/bzr-2.3.txt	2011-03-23 11:07:49 +0000
+++ doc/en/release-notes/bzr-2.3.txt	2011-03-31 09:08:48 +0000
@@ -44,6 +44,10 @@
   ancestry of the source branch, like it does in all other cases.
   (John Arbash Meinel, #465517)
 
+* Fix ``UnboundLocalError: local variable 'lock_url' in wait_lock`` error,
+  especially while trying to save configuration from QBzr.
+  (Martin Pool, #733136)
+
 * Fix "Unable to obtain lock" error when pushing to a bound branch if tags
   had changed.  Bazaar was attempting to open and lock the master branch
   twice in this case.  (Andrew Bennetts, #733350)