Rev 6345: (gz) Merge 2.4 into bzr.dev (Martin Packman) in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Mon Dec 5 14:53:57 UTC 2011


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6345 [merge]
revision-id: pqm at pqm.ubuntu.com-20111205145357-aesrpb6b49pxzym8
parent: pqm at pqm.ubuntu.com-20111205122358-4vml1qhofl1ll84q
parent: martin.packman at canonical.com-20111205142155-t7s4lr5aau50dp8i
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2011-12-05 14:53:57 +0000
message:
  (gz) Merge 2.4 into bzr.dev (Martin Packman)
modified:
  bzrlib/atomicfile.py           atomicfile.py-20050509044450-dbd24e6c564f7c66
  bzrlib/osutils.py              osutils.py-20050309040759-eeaff12fbf77ac86
  bzrlib/plugins/launchpad/lp_api.py lp_api.py-20090704082908-79il6zl4gugwl3wz-1
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/features.py       features.py-20090820042958-jglgza3wrn03ha9e-1
  bzrlib/tests/per_merger.py     per_merger.py-20091216002111-bzeo6wx2tcfpuj67-1
  bzrlib/tests/test_trace.py     testtrace.py-20051110225523-a21117fc7a07eeff
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/trace.py                trace.py-20050309040759-c8ed824bdcd4748a
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
  bzrlib/transport/local.py      local_transport.py-20050711165921-9b1f142bfe480c24
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/atomicfile.py'
--- a/bzrlib/atomicfile.py	2011-07-15 09:04:54 +0000
+++ b/bzrlib/atomicfile.py	2011-12-05 14:21:55 +0000
@@ -78,7 +78,7 @@
             # the common case is that we won't, though.
             st = os.fstat(self._fd)
             if stat.S_IMODE(st.st_mode) != new_mode:
-                os.chmod(self.tmpfilename, new_mode)
+                osutils.chmod_if_possible(self.tmpfilename, new_mode)
 
     def __repr__(self):
         return '%s(%r)' % (self.__class__.__name__,

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2011-12-05 12:23:58 +0000
+++ b/bzrlib/osutils.py	2011-12-05 14:21:55 +0000
@@ -100,14 +100,33 @@
     mod = os.lstat(filename).st_mode
     if not stat.S_ISLNK(mod):
         mod = mod & 0777555
-        os.chmod(filename, mod)
+        chmod_if_possible(filename, mod)
 
 
 def make_writable(filename):
     mod = os.lstat(filename).st_mode
     if not stat.S_ISLNK(mod):
         mod = mod | 0200
-        os.chmod(filename, mod)
+        chmod_if_possible(filename, mod)
+
+
+def chmod_if_possible(filename, mode):
+    # Set file mode if that can be safely done.
+    # Sometimes even on unix the filesystem won't allow it - see
+    # https://bugs.launchpad.net/bzr/+bug/606537
+    try:
+        # It is probably faster to just do the chmod, rather than
+        # doing a stat, and then trying to compare
+        os.chmod(filename, mode)
+    except (IOError, OSError),e:
+        # Permission/access denied seems to commonly happen on smbfs; there's
+        # probably no point warning about it.
+        # <https://bugs.launchpad.net/bzr/+bug/606537>
+        if getattr(e, 'errno') in (errno.EPERM, errno.EACCES):
+            trace.mutter("ignore error on chmod of %r: %r" % (
+                filename, e))
+            return
+        raise
 
 
 def minimum_path_selection(paths):
@@ -2543,6 +2562,21 @@
         fn(fileno)
 
 
+def ensure_empty_directory_exists(path, exception_class):
+    """Make sure a local directory exists and is empty.
+    
+    If it does not exist, it is created.  If it exists and is not empty, an
+    instance of exception_class is raised.
+    """
+    try:
+        os.mkdir(path)
+    except OSError, e:
+        if e.errno != errno.EEXIST:
+            raise
+        if os.listdir(path) != []:
+            raise exception_class(path)
+
+
 def is_environment_error(evalue):
     """True if exception instance is due to a process environment issue
 

=== modified file 'bzrlib/plugins/launchpad/lp_api.py'
--- a/bzrlib/plugins/launchpad/lp_api.py	2011-09-20 10:19:58 +0000
+++ b/bzrlib/plugins/launchpad/lp_api.py	2011-12-05 14:21:55 +0000
@@ -135,7 +135,7 @@
         proxy_info=proxy_info)
     # XXX: Work-around a minor security bug in launchpadlib 1.5.1, which would
     # create this directory with default umask.
-    os.chmod(cache_directory, 0700)
+    osutils.chmod_if_possible(cache_directory, 0700)
     return launchpad
 
 

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-12-01 15:43:56 +0000
+++ b/bzrlib/tests/__init__.py	2011-12-05 14:21:55 +0000
@@ -1990,8 +1990,8 @@
 
         self.log('run bzr: %r', args)
         # FIXME: don't call into logging here
-        handler = logging.StreamHandler(stderr)
-        handler.setLevel(logging.INFO)
+        handler = trace.EncodedStreamHandler(stderr, errors="replace",
+            level=logging.INFO)
         logger = logging.getLogger('')
         logger.addHandler(handler)
         old_ui_factory = ui.ui_factory

=== modified file 'bzrlib/tests/features.py'
--- a/bzrlib/tests/features.py	2011-11-29 16:27:56 +0000
+++ b/bzrlib/tests/features.py	2011-12-05 14:21:55 +0000
@@ -424,7 +424,7 @@
             f = tempfile.mkstemp(prefix='bzr_perms_chk_')
             fd, name = f
             os.close(fd)
-            os.chmod(name, write_perms)
+            osutils.chmod_if_possible(name, write_perms)
 
             read_perms = os.stat(name).st_mode & 0777
             os.unlink(name)

=== modified file 'bzrlib/tests/per_merger.py'
--- a/bzrlib/tests/per_merger.py	2011-11-28 19:22:07 +0000
+++ b/bzrlib/tests/per_merger.py	2011-12-05 14:21:55 +0000
@@ -174,17 +174,33 @@
         transform.finalize()
         return (limbodir, deletiondir)
 
-    def test_merge_with_existing_limbo(self):
-        wt = self.make_branch_and_tree('this')
-        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
-        os.mkdir(limbodir)
+    def test_merge_with_existing_limbo_empty(self):
+        """Empty limbo dir is just cleaned up - see bug 427773"""
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(limbodir)
+        self.do_merge(wt, wt)
+
+    def test_merge_with_existing_limbo_non_empty(self):
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(limbodir)
+        os.mkdir(os.path.join(limbodir, 'something'))
         self.assertRaises(errors.ExistingLimbo, self.do_merge, wt, wt)
         self.assertRaises(errors.LockError, wt.unlock)
 
-    def test_merge_with_pending_deletion(self):
-        wt = self.make_branch_and_tree('this')
-        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
-        os.mkdir(deletiondir)
+    def test_merge_with_pending_deletion_empty(self):
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(deletiondir)
+        self.do_merge(wt, wt)
+
+    def test_merge_with_pending_deletion_non_empty(self):
+        """Also see bug 427773"""
+        wt = self.make_branch_and_tree('this')
+        (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
+        os.mkdir(deletiondir)
+        os.mkdir(os.path.join(deletiondir, 'something'))
         self.assertRaises(errors.ExistingPendingDeletion, self.do_merge, wt, wt)
         self.assertRaises(errors.LockError, wt.unlock)
 

=== modified file 'bzrlib/tests/test_trace.py'
--- a/bzrlib/tests/test_trace.py	2011-11-29 16:27:56 +0000
+++ b/bzrlib/tests/test_trace.py	2011-12-05 14:21:55 +0000
@@ -20,6 +20,7 @@
 
 from cStringIO import StringIO
 import errno
+import logging
 import os
 import re
 import sys
@@ -346,6 +347,72 @@
         self.assertEqual(0, get_verbosity_level())
 
 
+class TestLogging(TestCase):
+    """Check logging functionality robustly records information"""
+
+    def test_note(self):
+        trace.note("Noted")
+        self.assertEqual("    INFO  Noted\n", self.get_log())
+
+    def test_warning(self):
+        trace.warning("Warned")
+        self.assertEqual(" WARNING  Warned\n", self.get_log())
+
+    def test_log(self):
+        logging.getLogger("bzr").error("Errored")
+        self.assertEqual("   ERROR  Errored\n", self.get_log())
+
+    def test_log_sub(self):
+        logging.getLogger("bzr.test_log_sub").debug("Whispered")
+        self.assertEqual("   DEBUG  Whispered\n", self.get_log())
+
+    def test_log_unicode_msg(self):
+        logging.getLogger("bzr").debug(u"\xa7")
+        self.assertEqual(u"   DEBUG  \xa7\n", self.get_log())
+
+    def test_log_unicode_arg(self):
+        logging.getLogger("bzr").debug("%s", u"\xa7")
+        self.assertEqual(u"   DEBUG  \xa7\n", self.get_log())
+
+    def test_log_utf8_msg(self):
+        logging.getLogger("bzr").debug("\xc2\xa7")
+        self.assertEqual(u"   DEBUG  \xa7\n", self.get_log())
+
+    def test_log_utf8_arg(self):
+        logging.getLogger("bzr").debug("%s", "\xc2\xa7")
+        self.assertEqual(u"   DEBUG  \xa7\n", self.get_log())
+
+    def test_log_bytes_msg(self):
+        logging.getLogger("bzr").debug("\xa7")
+        log = self.get_log()
+        self.assertContainsString(log, "UnicodeDecodeError: ")
+        self.assertContainsString(log,
+            "Logging record unformattable: '\\xa7' % ()\n")
+
+    def test_log_bytes_arg(self):
+        logging.getLogger("bzr").debug("%s", "\xa7")
+        log = self.get_log()
+        self.assertContainsString(log, "UnicodeDecodeError: ")
+        self.assertContainsString(log,
+            "Logging record unformattable: '%s' % ('\\xa7',)\n")
+
+    def test_log_mixed_strings(self):
+        logging.getLogger("bzr").debug(u"%s", "\xa7")
+        log = self.get_log()
+        self.assertContainsString(log, "UnicodeDecodeError: ")
+        self.assertContainsString(log,
+            "Logging record unformattable: u'%s' % ('\\xa7',)\n")
+
+    def test_log_repr_broken(self):
+        class BadRepr(object):
+            def __repr__(self):
+                raise ValueError("Broken object")
+        logging.getLogger("bzr").debug("%s", BadRepr())
+        log = self.get_log()
+        self.assertContainsRe(log, "ValueError: Broken object\n")
+        self.assertContainsRe(log, "Logging record unformattable: '%s' % .*\n")
+
+
 class TestBzrLog(TestCaseInTempDir):
 
     def test_log_rollover(self):

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2011-09-12 16:31:46 +0000
+++ b/bzrlib/tests/test_transport.py	2011-12-05 14:21:55 +0000
@@ -16,6 +16,7 @@
 
 
 from cStringIO import StringIO
+import errno
 import os
 import subprocess
 import sys
@@ -738,6 +739,29 @@
         self.assertEquals(t.local_abspath(''), here)
 
 
+class TestLocalTransportMutation(tests.TestCaseInTempDir):
+
+    def test_local_transport_mkdir(self):
+        here = osutils.abspath('.')
+        t = transport.get_transport(here)
+        t.mkdir('test')
+        self.assertTrue(os.path.exists('test'))
+
+    def test_local_transport_mkdir_permission_denied(self):
+        # See https://bugs.launchpad.net/bzr/+bug/606537
+        here = osutils.abspath('.')
+        t = transport.get_transport(here)
+        def fake_chmod(path, mode):
+            e = OSError('permission denied')
+            e.errno = errno.EPERM
+            raise e
+        self.overrideAttr(os, 'chmod', fake_chmod)
+        t.mkdir('test')
+        t.mkdir('test2', mode=0707)
+        self.assertTrue(os.path.exists('test'))
+        self.assertTrue(os.path.exists('test2'))
+
+
 class TestLocalTransportWriteStream(tests.TestCaseWithTransport):
 
     def test_local_fdatasync_calls_fdatasync(self):

=== modified file 'bzrlib/trace.py'
--- a/bzrlib/trace.py	2011-12-02 12:49:48 +0000
+++ b/bzrlib/trace.py	2011-12-05 14:21:55 +0000
@@ -54,7 +54,6 @@
 # increased cost of logging.py is not so bad, and we could standardize on
 # that.
 
-import codecs
 import logging
 import os
 import sys
@@ -111,8 +110,6 @@
 
     :return: None
     """
-    # FIXME note always emits utf-8, regardless of the terminal encoding
-    #
     # FIXME: clearing the ui and then going through the abstract logging
     # framework is whack; we should probably have a logging Handler that
     # deals with terminal output if needed.
@@ -285,7 +282,6 @@
     """
     start_time = osutils.format_local_date(_bzr_log_start_time,
                                            timezone='local')
-    # create encoded wrapper around stderr
     bzr_log_file = _open_bzr_log()
     if bzr_log_file is not None:
         bzr_log_file.write(start_time.encode('utf-8') + '\n')
@@ -294,11 +290,8 @@
         r'%Y-%m-%d %H:%M:%S')
     # after hooking output into bzr_log, we also need to attach a stderr
     # handler, writing only at level info and with encoding
-    term_encoding = osutils.get_terminal_encoding()
-    writer_factory = codecs.getwriter(term_encoding)
-    encoded_stderr = writer_factory(sys.stderr, errors='replace')
-    stderr_handler = logging.StreamHandler(encoded_stderr)
-    stderr_handler.setLevel(logging.INFO)
+    stderr_handler = EncodedStreamHandler(sys.stderr,
+        osutils.get_terminal_encoding(), 'replace', level=logging.INFO)
     logging.getLogger('bzr').addHandler(stderr_handler)
     return memento
 
@@ -313,8 +306,7 @@
     """
     global _trace_file
     # make a new handler
-    new_handler = logging.StreamHandler(to_file)
-    new_handler.setLevel(logging.DEBUG)
+    new_handler = EncodedStreamHandler(to_file, "utf-8", level=logging.DEBUG)
     if log_format is None:
         log_format = '%(levelname)8s  %(message)s'
     new_handler.setFormatter(logging.Formatter(log_format, date_format))
@@ -575,6 +567,49 @@
         _trace_file.flush()
 
 
+class EncodedStreamHandler(logging.Handler):
+    """Robustly write logging events to a stream using the specified encoding
+
+    Messages are expected to be formatted to unicode, but UTF-8 byte strings
+    are also accepted. An error during formatting or a str message in another
+    encoding will be quitely noted as an error in the Bazaar log file.
+
+    The stream is not closed so sys.stdout or sys.stderr may be passed.
+    """
+
+    def __init__(self, stream, encoding=None, errors='strict', level=0):
+        logging.Handler.__init__(self, level)
+        self.stream = stream
+        if encoding is None:
+            encoding = getattr(stream, "encoding", "ascii")
+        self.encoding = encoding
+        self.errors = errors
+
+    def flush(self):
+        flush = getattr(self.stream, "flush", None)
+        if flush is not None:
+            flush()
+
+    def emit(self, record):
+        try:
+            line = self.format(record)
+            if not isinstance(line, unicode):
+                line = line.decode("utf-8")
+            self.stream.write(line.encode(self.encoding, self.errors) + "\n")
+        except Exception:
+            log_exception_quietly()
+            # Try saving the details that would have been logged in some form
+            msg = args = "<Unformattable>"
+            try:
+                msg = repr(record.msg).encode("ascii")
+                args = repr(record.args).encode("ascii")
+            except Exception:
+                pass
+            # Using mutter() bypasses the logging module and writes directly
+            # to the file so there's no danger of getting into a loop here.
+            mutter("Logging record unformattable: %s %% %s", msg, args)
+
+
 class Config(object):
     """Configuration of message tracing in bzrlib.
 

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2011-10-27 15:38:14 +0000
+++ b/bzrlib/transform.py	2011-12-05 14:21:55 +0000
@@ -776,7 +776,7 @@
                     to_mode |= 0010 & ~umask
             else:
                 to_mode = current_mode & ~0111
-            os.chmod(abspath, to_mode)
+            osutils.chmod_if_possible(abspath, to_mode)
 
     def _new_entry(self, name, parent_id, file_id):
         """Helper function to create a new filesystem entry."""
@@ -1555,18 +1555,14 @@
         try:
             limbodir = urlutils.local_path_from_url(
                 tree._transport.abspath('limbo'))
-            try:
-                os.mkdir(limbodir)
-            except OSError, e:
-                if e.errno == errno.EEXIST:
-                    raise ExistingLimbo(limbodir)
+            osutils.ensure_empty_directory_exists(
+                limbodir,
+                errors.ExistingLimbo)
             deletiondir = urlutils.local_path_from_url(
                 tree._transport.abspath('pending-deletion'))
-            try:
-                os.mkdir(deletiondir)
-            except OSError, e:
-                if e.errno == errno.EEXIST:
-                    raise errors.ExistingPendingDeletion(deletiondir)
+            osutils.ensure_empty_directory_exists(
+                deletiondir,
+                errors.ExistingPendingDeletion)
         except:
             tree.unlock()
             raise
@@ -1635,7 +1631,7 @@
             else:
                 raise
         if typefunc(mode):
-            os.chmod(self._limbo_name(trans_id), mode)
+            osutils.chmod_if_possible(self._limbo_name(trans_id), mode)
 
     def iter_tree_children(self, parent_id):
         """Iterate through the entry's tree children, if any"""

=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py	2011-11-04 21:27:48 +0000
+++ b/bzrlib/transport/local.py	2011-12-05 14:21:55 +0000
@@ -255,7 +255,7 @@
             if mode is not None and mode != S_IMODE(st.st_mode):
                 # Because of umask, we may still need to chmod the file.
                 # But in the general case, we won't have to
-                os.chmod(abspath, mode)
+                osutils.chmod_if_possible(abspath, mode)
             writer(fd)
         finally:
             os.close(fd)
@@ -314,12 +314,13 @@
             local_mode = mode
         try:
             os.mkdir(abspath, local_mode)
-            if mode is not None:
-                # It is probably faster to just do the chmod, rather than
-                # doing a stat, and then trying to compare
-                os.chmod(abspath, mode)
         except (IOError, OSError),e:
             self._translate_error(e, abspath)
+        if mode is not None:
+            try:
+                osutils.chmod_if_possible(abspath, mode)
+            except (IOError, OSError), e:
+                self._translate_error(e, abspath)
 
     def mkdir(self, relpath, mode=None):
         """Create a directory at the given path."""
@@ -357,7 +358,7 @@
         if mode is not None and mode != S_IMODE(st.st_mode):
             # Because of umask, we may still need to chmod the file.
             # But in the general case, we won't have to
-            os.chmod(file_abspath, mode)
+            osutils.chmod_if_possible(file_abspath, mode)
         return st.st_size
 
     def append_file(self, relpath, f, mode=None):
@@ -456,7 +457,7 @@
                     otherpath = other._abspath(path)
                     shutil.copy(mypath, otherpath)
                     if mode is not None:
-                        os.chmod(otherpath, mode)
+                        osutils.chmod_if_possible(otherpath, mode)
                 except (IOError, OSError),e:
                     self._translate_error(e, path)
                 count += 1

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-11-29 12:06:33 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-12-05 14:21:55 +0000
@@ -32,6 +32,15 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Cope with Unix filesystems, such as smbfs, where chmod gives 'permission
+  denied'.  (Martin Pool, #606537)
+
+* When the ``limbo`` or ``pending-deletion`` directories exist, typically
+  because of an interrupted tree update, but are empty, bzr no longer
+  errors out, because there is nothing for the user to clean up.  Also,
+  errors in creation of these directories are no longer squelched.
+  (Martin Pool, #427773)
+
 * During merges, when two entries end up using the same path for two
   different file-ids (the same file being 'bzr added' in two different
   branches) , 'duplicate' conflicts are created instead of 'content'
@@ -43,6 +52,9 @@
   confusion as there is generally nothing users can do about them.
   (Vincent Ladeuil, #880701)
 
+* Prevent a traceback being printed to stderr when logging has problems and
+  accept utf-8 byte string without breaking. (Martin Packman, #714449)
+
 Documentation
 *************
 




More information about the bazaar-commits mailing list