Rev 4928: (andrew) Merge lp:bzr/2.0 into lp:bzr, including fixes for #343218, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Jan 4 03:10:02 GMT 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4928 [merge]
revision-id: pqm at pqm.ubuntu.com-20100104031000-z5cdnjpq7phqxwf2
parent: pqm at pqm.ubuntu.com-20091230125719-o6l0dwzf0gxhkgn1
parent: andrew.bennetts at canonical.com-20100104022511-2tq2r9w2te84wzgs
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2010-01-04 03:10:00 +0000
message:
  (andrew) Merge lp:bzr/2.0 into lp:bzr, including fixes for #343218,
  	#495000, #495023, #494406 and #498378.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/_bencode_pyx.pyx        bencode.pyx-20070806220735-j75g4ebfnado2i60-3
  bzrlib/_readdir_pyx.pyx        readdir.pyx-20060609152855-rm6v321vuaqyh9tu-1
  bzrlib/export/dir_exporter.py  dir_exporter.py-20051114235828-b51397f56bc7b117
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/repofmt/groupcompress_repo.py repofmt.py-20080715094215-wp1qfvoo7093c8qr-1
  bzrlib/tests/per_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
=== modified file 'NEWS'
--- a/NEWS	2009-12-25 13:33:03 +0000
+++ b/NEWS	2010-01-04 02:25:11 +0000
@@ -31,6 +31,17 @@
 Bug Fixes
 *********
 
+* ``bzr export dir`` now requests all file content as a record stream,
+  rather than requsting the file content one file-at-a-time. This can make
+  exporting over the network significantly faster (54min => 9min in one
+  case). (John Arbash Meinel, #343218)
+
+* ``bzr serve`` no longer slowly leaks memory. The compiled
+  ``bzrlib.bencode.Encoder()`` class was using ``__del__`` to cleanup and
+  free resources, and it should have been using ``__dealloc__``.
+  This will likely have an impact on any other process that is serving for
+  an extended period of time.  (John Arbash Meinel, #494406)
+
 * ``bzr switch -b`` can now create branches that are located using directory
   services such as ``lp:``, even when the branch name doesn't contain a
   '/'.  (Neil Martinsen-Burrell, #495263)
@@ -38,9 +49,20 @@
 * ``bzr unshelve`` has improved messages about what it is doing.
   (Neil Martinsen-Burrell, #496917)
 
+* Check for SIGINT (Ctrl-C) and other signals immediately if ``readdir``
+  returns ``EINTR`` by calling ``PyErr_CheckSignals``.  This affected the
+  optional ``_readdir_pyx`` extension.  (Andrew Bennetts, #495023)
+
+* Give a clearer message if the lockdir disappears after being apparently
+  successfully taken.  (Martin Pool, #498378)
+
 * Listen to the SIGWINCH signal to update the terminal width.
   (Vincent Ladeuil, #316357)
 
+* 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)
+
 Improvements
 ************
 
@@ -97,7 +119,7 @@
 bzr 2.0.4 (not released yet)
 ############################
 
-:Codename: template
+:Codename:
 :2.0.4: ???
 
 Compatibility Breaks
@@ -109,6 +131,28 @@
 Bug Fixes
 *********
 
+* ``bzr export dir`` now requests all file content as a record stream,
+  rather than requsting the file content one file-at-a-time. This can make
+  exporting over the network significantly faster (54min => 9min in one
+  case). (John Arbash Meinel, #343218)
+
+* ``bzr serve`` no longer slowly leaks memory. The compiled
+  ``bzrlib.bencode.Encoder()`` class was using ``__del__`` to cleanup and
+  free resources, and it should have been using ``__dealloc__``.
+  This will likely have an impact on any other process that is serving for
+  an extended period of time.  (John Arbash Meinel, #494406)
+
+* Check for SIGINT (Ctrl-C) and other signals immediately if ``readdir``
+  returns ``EINTR`` by calling ``PyErr_CheckSignals``.  This affected the
+  optional ``_readdir_pyx`` extension.  (Andrew Bennetts, #495023)
+
+* 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)
+
 Improvements
 ************
 

=== modified file 'bzrlib/_bencode_pyx.pyx'
--- a/bzrlib/_bencode_pyx.pyx	2009-10-15 18:28:14 +0000
+++ b/bzrlib/_bencode_pyx.pyx	2010-01-04 02:25:11 +0000
@@ -268,7 +268,7 @@
         self.maxsize = maxsize
         self.tail = p
 
-    def __del__(self):
+    def __dealloc__(self):
         free(self.buffer)
         self.buffer = NULL
         self.maxsize = 0

=== modified file 'bzrlib/_readdir_pyx.pyx'
--- a/bzrlib/_readdir_pyx.pyx	2009-10-08 07:03:05 +0000
+++ b/bzrlib/_readdir_pyx.pyx	2009-12-23 02:19:04 +0000
@@ -78,6 +78,7 @@
 
 
 cdef extern from 'Python.h':
+    int PyErr_CheckSignals() except -1
     char * PyString_AS_STRING(object)
     ctypedef int Py_ssize_t # Required for older pyrex versions
     ctypedef struct PyObject:
@@ -271,6 +272,12 @@
         return result
 
 
+cdef raise_os_error(int errnum, char *msg_prefix, path):
+    if errnum == EINTR:
+        PyErr_CheckSignals()
+    raise OSError(errnum, msg_prefix + strerror(errnum), path)
+
+
 cdef _read_dir(path):
     """Like os.listdir, this reads the contents of a directory.
 
@@ -298,16 +305,16 @@
         # passing full paths every time.
         orig_dir_fd = open(".", O_RDONLY, 0)
         if orig_dir_fd == -1:
-            raise OSError(errno, "open: " + strerror(errno), ".")
+            raise_os_error(errno, "open: ", ".")
         if -1 == chdir(path):
-            raise OSError(errno, "chdir: " + strerror(errno), path)
+            raise_os_error(errno, "chdir: ", path)
     else:
         orig_dir_fd = -1
 
     try:
         the_dir = opendir(".")
         if NULL == the_dir:
-            raise OSError(errno, "opendir: " + strerror(errno), path)
+            raise_os_error(errno, "opendir: ", path)
         try:
             result = []
             entry = &sentinel
@@ -319,6 +326,8 @@
                     errno = 0
                     entry = readdir(the_dir)
                     if entry == NULL and (errno == EAGAIN or errno == EINTR):
+                        if errno == EINTR:
+                            PyErr_CheckSignals()
                         # try again
                         continue
                     else:
@@ -330,7 +339,7 @@
                         # we consider ENOTDIR to be 'no error'.
                         continue
                     else:
-                        raise OSError(errno, "readdir: " + strerror(errno), path)
+                        raise_os_error(errno, "readdir: ", path)
                 name = entry.d_name
                 if not (name[0] == c"." and (
                     (name[1] == 0) or 
@@ -340,7 +349,7 @@
                     stat_result = lstat(entry.d_name, &statvalue._st)
                     if stat_result != 0:
                         if errno != ENOENT:
-                            raise OSError(errno, "lstat: " + strerror(errno),
+                            raise_os_error(errno, "lstat: ",
                                 path + "/" + entry.d_name)
                         else:
                             # the file seems to have disappeared after being
@@ -358,7 +367,7 @@
                         statvalue, None))
         finally:
             if -1 == closedir(the_dir):
-                raise OSError(errno, "closedir: " + strerror(errno), path)
+                raise_os_error(errno, "closedir: ", path)
     finally:
         if -1 != orig_dir_fd:
             failed = False
@@ -366,7 +375,7 @@
                 # try to close the original directory anyhow
                 failed = True
             if -1 == close(orig_dir_fd) or failed:
-                raise OSError(errno, "return to orig_dir: " + strerror(errno))
+                raise_os_error(errno, "return to orig_dir: ", "")
 
     return result
 

=== modified file 'bzrlib/export/dir_exporter.py'
--- a/bzrlib/export/dir_exporter.py	2009-07-29 13:46:55 +0000
+++ b/bzrlib/export/dir_exporter.py	2009-12-18 05:38:40 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2009 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
@@ -52,21 +52,17 @@
                 raise errors.BzrError("Can't export tree to non-empty directory.")
         else:
             raise
+    # Iterate everything, building up the files we will want to export, and
+    # creating the directories and symlinks that we need.
+    # This tracks (file_id, (destination_path, executable))
+    # This matches the api that tree.iter_files_bytes() wants
+    # Note in the case of revision trees, this does trigger a double inventory
+    # lookup, hopefully it isn't too expensive.
+    to_fetch = []
     for dp, ie in _export_iter_entries(tree, subdir):
         fullpath = osutils.pathjoin(dest, dp)
         if ie.kind == "file":
-            if filtered:
-                chunks = tree.get_file_lines(ie.file_id)
-                filters = tree._content_filter_stack(dp)
-                context = ContentFilterContext(dp, tree, ie)
-                contents = filtered_output_bytes(chunks, filters, context)
-                content = ''.join(contents)
-                fileobj = StringIO.StringIO(content)
-            else:
-                fileobj = tree.get_file(ie.file_id)
-            osutils.pumpfile(fileobj, file(fullpath, 'wb'))
-            if tree.is_executable(ie.file_id):
-                os.chmod(fullpath, 0755)
+            to_fetch.append((ie.file_id, (dp, tree.is_executable(ie.file_id))))
         elif ie.kind == "directory":
             os.mkdir(fullpath)
         elif ie.kind == "symlink":
@@ -80,3 +76,21 @@
         else:
             raise errors.BzrError("don't know how to export {%s} of kind %r" %
                (ie.file_id, ie.kind))
+    # The data returned here can be in any order, but we've already created all
+    # the directories
+    flags = os.O_CREAT | os.O_TRUNC | os.O_WRONLY | getattr(os, 'O_BINARY', 0)
+    for (relpath, executable), chunks in tree.iter_files_bytes(to_fetch):
+        if filtered:
+            filters = tree._content_filter_stack(relpath)
+            context = ContentFilterContext(relpath, tree, ie)
+            chunks = filtered_output_bytes(chunks, filters, context)
+        fullpath = osutils.pathjoin(dest, relpath)
+        # We set the mode and let the umask sort out the file info
+        mode = 0666
+        if executable:
+            mode = 0777
+        out = os.fdopen(os.open(fullpath, flags, mode), 'wb')
+        try:
+            out.writelines(chunks)
+        finally:
+            out.close()

=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2009-12-02 17:49:45 +0000
+++ b/bzrlib/lockdir.py	2010-01-04 02:25:11 +0000
@@ -242,8 +242,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")

=== modified file 'bzrlib/repofmt/groupcompress_repo.py'
--- a/bzrlib/repofmt/groupcompress_repo.py	2009-10-23 17:27:45 +0000
+++ b/bzrlib/repofmt/groupcompress_repo.py	2010-01-04 02:25:11 +0000
@@ -353,7 +353,8 @@
         """Build a VersionedFiles instance on top of this group of packs."""
         index_name = index_name + '_index'
         index_to_pack = {}
-        access = knit._DirectPackAccess(index_to_pack)
+        access = knit._DirectPackAccess(index_to_pack,
+                                        reload_func=self._reload_func)
         if for_write:
             # Use new_pack
             if self.new_pack is None:

=== modified file 'bzrlib/tests/per_pack_repository.py'
--- a/bzrlib/tests/per_pack_repository.py	2009-12-16 22:29:31 +0000
+++ b/bzrlib/tests/per_pack_repository.py	2010-01-04 02:25:11 +0000
@@ -545,6 +545,42 @@
         finally:
             tree.unlock()
 
+    def test_concurrent_pack_during_autopack(self):
+        tree = self.make_branch_and_tree('tree')
+        tree.lock_write()
+        try:
+            for i in xrange(9):
+                tree.commit('rev %d' % (i,))
+            r2 = repository.Repository.open('tree')
+            r2.lock_write()
+            try:
+                # Monkey patch so that pack occurs while the other repo is
+                # autopacking. This is slightly bad, but all current pack
+                # repository implementations have a _pack_collection, and we
+                # test that it gets triggered. So if a future format changes
+                # things, the test will fail rather than succeed accidentally.
+                autopack_count = [0]
+                r1 = tree.branch.repository
+                orig = r1._pack_collection.pack_distribution
+                def trigger_during_auto(*args, **kwargs):
+                    ret = orig(*args, **kwargs)
+                    if not autopack_count[0]:
+                        r2.pack()
+                    autopack_count[0] += 1
+                    return ret
+                r1._pack_collection.pack_distribution = trigger_during_auto
+                tree.commit('autopack-rev')
+                # This triggers 2 autopacks. The first one causes r2.pack() to
+                # fire, but r2 doesn't see the new pack file yet. The
+                # autopack restarts and sees there are 2 files and there
+                # should be only 1 for 10 commits. So it goes ahead and
+                # finishes autopacking.
+                self.assertEqual([2], autopack_count)
+            finally:
+                r2.unlock()
+        finally:
+            tree.unlock()
+
     def test_lock_write_does_not_physically_lock(self):
         repo = self.make_repository('.', format=self.get_format())
         repo.lock_write()




More information about the bazaar-commits mailing list