Rev 5288: Merge first-try into propagate-exceptions in file:///home/vila/src/bzr/experimental/leaking-tests/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Jun 17 15:58:25 BST 2010


At file:///home/vila/src/bzr/experimental/leaking-tests/

------------------------------------------------------------
revno: 5288 [merge]
revision-id: v.ladeuil+lp at free.fr-20100617145825-e74dii5j5nf6lbwx
parent: v.ladeuil+lp at free.fr-20100617092327-k153de2hpnomv3zo
parent: v.ladeuil+lp at free.fr-20100617145823-tq22fy9pbbqqt915
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: propagate-exceptions
timestamp: Thu 2010-06-17 16:58:25 +0200
message:
  Merge first-try into propagate-exceptions
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/osutils.py              osutils.py-20050309040759-eeaff12fbf77ac86
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
  bzrlib/tests/test_osutils.py   test_osutils.py-20051201224856-e48ee24c12182989
  bzrlib/transport/ssh.py        ssh.py-20060824042150-0s9787kng6zv1nwq-1
  bzrlib/treebuilder.py          treebuilder.py-20060907214856-4omn6hf1u7fvrart-1
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2010-06-17 09:23:27 +0000
+++ b/NEWS	2010-06-17 14:58:25 +0000
@@ -70,6 +70,14 @@
   which previously caused "SyntaxError: No command for line".
   (Martin Pool)
 
+* ``walkdirs`` now raises a useful message when the filenames are not using
+  the filesystem encoding. (Eric Moritz, #488519)
+
+* URL displayed for use with ``break-lock`` when smart server sees lock
+  contention are now valid. Default timeout for lock contention retry is
+  now 30 seconds instead of 300 seconds.
+  (Parth Malwankar, #250451)
+
 Improvements
 ************
 

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2010-05-28 06:04:57 +0000
+++ b/bzrlib/errors.py	2010-06-17 11:52:13 +0000
@@ -1041,8 +1041,6 @@
 class LockContention(LockError):
 
     _fmt = 'Could not acquire lock "%(lock)s": %(msg)s'
-    # TODO: show full url for lock, combining the transport and relative
-    # bits?
 
     internal_error = False
 
@@ -1923,12 +1921,12 @@
 class CantMoveRoot(BzrError):
 
     _fmt = "Moving the root directory is not supported at this time"
-    
-    
+
+
 class TransformRenameFailed(BzrError):
 
     _fmt = "Failed to rename %(from_path)s to %(to_path)s: %(why)s"
-    
+
     def __init__(self, from_path, to_path, why, errno):
         self.from_path = from_path
         self.to_path = to_path

=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2010-05-25 17:27:52 +0000
+++ b/bzrlib/lockdir.py	2010-06-15 09:53:42 +0000
@@ -151,7 +151,7 @@
 # files/dirs created.
 
 
-_DEFAULT_TIMEOUT_SECONDS = 300
+_DEFAULT_TIMEOUT_SECONDS = 30
 _DEFAULT_POLL_SECONDS = 1.0
 
 
@@ -539,24 +539,27 @@
                 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)
-                # See <https://bugs.launchpad.net/bzr/+bug/250451>
-                # the URL here is sometimes not one that is useful to the
-                # user, perhaps being wrapped in a lp-%d or chroot decorator,
-                # especially if this error is issued from the server.
-                self._report_function('%s %s\n'
-                    '%s\n' # held by
-                    '%s\n' # locked ... ago
-                    'Will continue to try until %s, unless '
-                    'you press Ctrl-C.\n'
-                    'See "bzr help break-lock" for more.',
-                    start,
-                    formatted_info[0],
-                    formatted_info[1],
-                    formatted_info[2],
-                    deadline_str,
-                    )
-
+                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
+                    '%s\n'                  # user
+                    'at %s '                # hostname
+                    '[process #%s], '       # pid
+                    'acquired %s.')         # time ago
+                msg_args = [start, lock_url, user, hostname, pid, time_ago]
+                if timeout > 0:
+                    msg += ('\nWill continue to try until %s, unless '
+                        'you press Ctrl-C.')
+                    msg_args.append(deadline_str)
+                msg += '\nSee "bzr help break-lock" for more.'
+                self._report_function(msg, *msg_args)
             if (max_attempts is not None) and (attempt_count >= max_attempts):
                 self._trace("exceeded %d attempts")
                 raise LockContention(self)
@@ -564,8 +567,11 @@
                 self._trace("waiting %ss", poll)
                 time.sleep(poll)
             else:
+                # As timeout is always 0 for remote locks
+                # this block is applicable only for local
+                # lock contention
                 self._trace("timeout after waiting %ss", timeout)
-                raise LockContention(self)
+                raise LockContention('(local)', lock_url)
 
     def leave_in_place(self):
         self._locked_via_token = True
@@ -616,17 +622,19 @@
 
     def _format_lock_info(self, info):
         """Turn the contents of peek() into something for the user"""
-        lock_url = self.transport.abspath(self.path)
         start_time = info.get('start_time')
         if start_time is None:
             time_ago = '(unknown)'
         else:
             time_ago = format_delta(time.time() - int(info['start_time']))
+        user = info.get('user', '<unknown>')
+        hostname = info.get('hostname', '<unknown>')
+        pid = info.get('pid', '<unknown>')
         return [
-            'lock %s' % (lock_url,),
-            'held by %s on host %s [process #%s]' %
-                tuple([info.get(x, '<unknown>') for x in ['user', 'hostname', 'pid']]),
-            'locked %s' % (time_ago,),
+            user,
+            hostname,
+            pid,
+            time_ago,
             ]
 
     def validate_token(self, token):

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2010-06-17 09:23:27 +0000
+++ b/bzrlib/osutils.py	2010-06-17 14:58:25 +0000
@@ -918,7 +918,7 @@
 
 def parent_directories(filename):
     """Return the list of parent directories, deepest first.
-    
+
     For example, parent_directories("a/b/c") -> ["a/b", "a"].
     """
     parents = []
@@ -948,7 +948,7 @@
     # NB: This docstring is just an example, not a doctest, because doctest
     # currently can't cope with the use of lazy imports in this namespace --
     # mbp 20090729
-    
+
     # This currently doesn't report the failure at the time it occurs, because
     # they tend to happen very early in startup when we can't check config
     # files etc, and also we want to report all failures but not spam the user
@@ -1024,8 +1024,8 @@
 
 
 def delete_any(path):
-    """Delete a file, symlink or directory.  
-    
+    """Delete a file, symlink or directory.
+
     Will delete even if readonly.
     """
     try:
@@ -1220,6 +1220,22 @@
     # but for now, we haven't optimized...
     return [canonical_relpath(base, p) for p in paths]
 
+
+def decode_filename(filename):
+    """Decode the filename using the filesystem encoding
+
+    If it is unicode, it is returned.
+    Otherwise it is decoded from the the filesystem's encoding. If decoding
+    fails, a errors.BadFilenameEncoding exception is raised.
+    """
+    if type(filename) is unicode:
+        return filename
+    try:
+        return filename.decode(_fs_enc)
+    except UnicodeDecodeError:
+        raise errors.BadFilenameEncoding(filename, _fs_enc)
+
+
 def safe_unicode(unicode_or_utf8_string):
     """Coerce unicode_or_utf8_string into unicode.
 
@@ -1631,7 +1647,7 @@
         dirblock = []
         append = dirblock.append
         try:
-            names = sorted(_listdir(top))
+            names = sorted(map(decode_filename, _listdir(top)))
         except OSError, e:
             if not _is_error_enotdir(e):
                 raise
@@ -2007,14 +2023,14 @@
 
 def send_all(sock, bytes, report_activity=None):
     """Send all bytes on a socket.
- 
+
     Breaks large blocks in smaller chunks to avoid buffering limitations on
     some platforms, and catches EINTR which may be thrown if the send is
     interrupted by a signal.
 
     This is preferred to socket.sendall(), because it avoids portability bugs
     and provides activity reporting.
- 
+
     :param report_activity: Call this as bytes are read, see
         Transport._report_activity
     """
@@ -2130,7 +2146,7 @@
 
 def until_no_eintr(f, *a, **kw):
     """Run f(*a, **kw), retrying if an EINTR error occurs.
-    
+
     WARNING: you must be certain that it is safe to retry the call repeatedly
     if EINTR does occur.  This is typically only true for low-level operations
     like os.read.  If in any doubt, don't use this.
@@ -2267,7 +2283,7 @@
 if sys.platform == 'win32':
     def open_file(filename, mode='r', bufsize=-1):
         """This function is used to override the ``open`` builtin.
-        
+
         But it uses O_NOINHERIT flag so the file handle is not inherited by
         child processes.  Deleting or renaming a closed file opened with this
         function is not blocking child processes.

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2010-05-20 18:23:17 +0000
+++ b/bzrlib/remote.py	2010-06-11 07:56:46 +0000
@@ -2420,9 +2420,16 @@
             repo_token = self.repository.lock_write().repository_token
             self.repository.unlock()
         err_context = {'token': token}
-        response = self._call(
-            'Branch.lock_write', self._remote_path(), branch_token,
-            repo_token or '', **err_context)
+        try:
+            response = self._call(
+                'Branch.lock_write', self._remote_path(), branch_token,
+                repo_token or '', **err_context)
+        except errors.LockContention, e:
+            # The LockContention from the server doesn't have any
+            # information about the lock_url. We re-raise LockContention
+            # with valid lock_url.
+            raise errors.LockContention('(remote lock)',
+                self.repository.base.split('.bzr/')[0])
         if response[0] != 'ok':
             raise errors.UnexpectedSmartServerResponse(response)
         ok, branch_token, repo_token = response

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2010-06-10 17:23:05 +0000
+++ b/bzrlib/tests/__init__.py	2010-06-17 14:58:25 +0000
@@ -4350,6 +4350,17 @@
 UnicodeFilename = _UnicodeFilename()
 
 
+class _ByteStringNamedFilesystem(Feature):
+    """Is the filesystem based on bytes?"""
+
+    def _probe(self):
+        if os.name == "posix":
+            return True
+        return False
+
+ByteStringNamedFilesystem = _ByteStringNamedFilesystem()
+
+
 class _UTF8Filesystem(Feature):
     """Is the filesystem UTF-8?"""
 

=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py	2010-05-20 18:23:17 +0000
+++ b/bzrlib/tests/test_lockdir.py	2010-06-15 09:53:42 +0000
@@ -191,22 +191,21 @@
                     "took %f seconds to detect lock contention" % (after - before))
         finally:
             lf1.unlock()
-        lock_base = lf2.transport.abspath(lf2.path)
         self.assertEqual(1, len(self._logged_reports))
-        lock_url = lf2.transport.abspath(lf2.path)
-        self.assertEqual('%s %s\n'
-                         '%s\n%s\n'
-                         'Will continue to try until %s, unless '
-                         'you press Ctrl-C.\n'
-                         'See "bzr help break-lock" for more.',
-                         self._logged_reports[0][0])
-        args = self._logged_reports[0][1]
-        self.assertEqual('Unable to obtain', args[0])
-        self.assertEqual('lock %s' % (lock_base,), args[1])
-        self.assertStartsWith(args[2], 'held by ')
-        self.assertStartsWith(args[3], 'locked ')
-        self.assertEndsWith(args[3], ' ago')
-        self.assertContainsRe(args[4], r'\d\d:\d\d:\d\d')
+        self.assertEqual(self._logged_reports[0][0],
+            '%s lock %s held by %s\n'
+            'at %s [process #%s], acquired %s.\n'
+            'Will continue to try until %s, unless '
+            'you press Ctrl-C.\n'
+            'See "bzr help break-lock" for more.')
+        start, lock_url, user, hostname, pid, time_ago, deadline_str = \
+            self._logged_reports[0][1]
+        self.assertEqual(start, u'Unable to obtain')
+        self.assertEqual(user, u'jrandom at example.com')
+        # skip hostname
+        self.assertContainsRe(pid, r'\d+')
+        self.assertContainsRe(time_ago, r'.* ago')
+        self.assertContainsRe(deadline_str, r'\d{2}:\d{2}:\d{2}')
 
     def test_31_lock_wait_easy(self):
         """Succeed when waiting on a lock with no contention.
@@ -597,11 +596,10 @@
             info_list = ld1._format_lock_info(ld1.peek())
         finally:
             ld1.unlock()
-        self.assertEqual('lock %s' % (ld1.transport.abspath(ld1.path),),
-                         info_list[0])
-        self.assertContainsRe(info_list[1],
-                              r'^held by .* on host .* \[process #\d*\]$')
-        self.assertContainsRe(info_list[2], r'locked \d+ seconds? ago$')
+        self.assertEqual(info_list[0], u'jrandom at example.com')
+        # info_list[1] is hostname. we skip this.
+        self.assertContainsRe(info_list[2], '^\d+$') # pid
+        self.assertContainsRe(info_list[3], r'^\d+ seconds? ago$') # time_ago
 
     def test_lock_without_email(self):
         global_config = config.GlobalConfig()
@@ -680,9 +678,7 @@
         info = lf.peek()
         formatted_info = lf._format_lock_info(info)
         self.assertEquals(
-            ['lock %s' % t.abspath('test_lock'),
-             'held by <unknown> on host <unknown> [process #<unknown>]',
-             'locked (unknown)'],
+            ['<unknown>', '<unknown>', '<unknown>', '(unknown)'],
             formatted_info)
 
 

=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py	2010-06-02 11:51:58 +0000
+++ b/bzrlib/tests/test_osutils.py	2010-06-17 14:58:20 +0000
@@ -1083,6 +1083,40 @@
         # Ensure the message contains the file name
         self.assertContainsRe(str(e), "\./test-unreadable")
 
+
+    def test_walkdirs_encoding_error(self):
+        # <https://bugs.launchpad.net/bzr/+bug/488519>
+        # walkdirs didn't raise a useful message when the filenames
+        # are not using the filesystem's encoding
+
+        # require a bytestring based filesystem
+        self.requireFeature(tests.ByteStringNamedFilesystem)
+
+        tree = [
+            '.bzr',
+            '0file',
+            '1dir/',
+            '1dir/0file',
+            '1dir/1dir/',
+            '1file'
+            ]
+
+        self.build_tree(tree)
+
+        # rename the 1file to a latin-1 filename
+        os.rename("./1file", "\xe8file")
+
+        self._save_platform_info()
+        win32utils.winver = None # Avoid the win32 detection code
+        osutils._fs_enc = 'UTF-8'
+
+        # this should raise on error
+        def attempt():
+            for dirdetail, dirblock in osutils.walkdirs('.'):
+                pass
+
+        self.assertRaises(errors.BadFilenameEncoding, attempt)
+
     def test__walkdirs_utf8(self):
         tree = [
             '.bzr',
@@ -1921,7 +1955,7 @@
     def restore_osutils_globals(self):
         osutils._terminal_size_state = self._orig_terminal_size_state
         osutils._first_terminal_size = self._orig_first_terminal_size
-        
+
     def replace_stdout(self, new):
         self.overrideAttr(sys, 'stdout', new)
 

=== modified file 'bzrlib/transport/ssh.py'
--- a/bzrlib/transport/ssh.py	2010-06-16 06:21:34 +0000
+++ b/bzrlib/transport/ssh.py	2010-06-17 13:16:51 +0000
@@ -701,7 +701,7 @@
 
     def recv(self, count):
         if self._sock is not None:
-            return self._sock.read(count)
+            return self._sock.recv(count)
         else:
             return os.read(self.proc.stdout.fileno(), count)
 

=== modified file 'bzrlib/treebuilder.py'
--- a/bzrlib/treebuilder.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/treebuilder.py	2010-06-17 07:11:55 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2008, 2009, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -16,7 +16,7 @@
 
 """TreeBuilder helper class.
 
-TreeBuilders are used to build trees of various shapres or properties. This
+TreeBuilders are used to build trees of various shapes or properties. This
 can be extremely useful in testing for instance.
 """
 



More information about the bazaar-commits mailing list