Rev 2479: (John Arbash Meinel) Fix bug #75721: 'bzr push' should only connect a single time. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu May 3 20:22:21 BST 2007


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

------------------------------------------------------------
revno: 2479
revision-id: pqm at pqm.ubuntu.com-20070503192218-gq01rpszgraa681p
parent: pqm at pqm.ubuntu.com-20070503094732-4ypcrctuxb93rwcv
parent: john at arbash-meinel.com-20070502145420-k2vebnl0rd45q8kk
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-05-03 20:22:18 +0100
message:
  (John Arbash Meinel) Fix bug #75721: 'bzr push' should only connect a single time.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/builtins.py             builtins.py-20050830033751-fc01482b9ca23183
  bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/bzrdir_implementations/__init__.py __init__.py-20060131065642-34c39b54f42dd048
  bzrlib/tests/bzrdir_implementations/test_bzrdir.py test_bzrdir.py-20060131065642-0ebeca5e30e30866
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/workingtree.py          workingtree.py-20050511021032-29b6ec0a681e02e3
    ------------------------------------------------------------
    revno: 2475.3.3
    merged: john at arbash-meinel.com-20070502145420-k2vebnl0rd45q8kk
    parent: john at arbash-meinel.com-20070502143655-id25373m3lgue8ke
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: single_connect_for_push_75721
    timestamp: Wed 2007-05-02 09:54:20 -0500
    message:
      Change calls to try/mkdir('.')/except FileExists to ensure_base()
    ------------------------------------------------------------
    revno: 2475.3.2
    merged: john at arbash-meinel.com-20070502143655-id25373m3lgue8ke
    parent: john at arbash-meinel.com-20070501224141-23intuz4dabm0j73
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: single_connect_for_push_75721
    timestamp: Wed 2007-05-02 09:36:55 -0500
    message:
      Add Transport.ensure_base()
    ------------------------------------------------------------
    revno: 2475.3.1
    merged: john at arbash-meinel.com-20070501224141-23intuz4dabm0j73
    parent: pqm at pqm.ubuntu.com-20070501182714-71xp33bziogu3qu0
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: single_connect_for_push_75721
    timestamp: Tue 2007-05-01 17:41:41 -0500
    message:
      Fix bug #75721. Update the BzrDir api to add clone_on_transport()
      This allows us to pass around a Transport which we already have open
      rather calling get_transport(url).
      This is mostly noticeable in 'bzr push' which was connecting 3-4 times.
=== modified file 'NEWS'
--- a/NEWS	2007-04-30 17:45:10 +0000
+++ b/NEWS	2007-05-01 22:41:41 +0000
@@ -12,6 +12,12 @@
       Previously we would report the first file as missing, but not show
       the new unknown file. (John Arbash Meinel, #111288)
 
+    * ``bzr push`` should only connect to the remote location one time.
+      We have been connecting 3 times because we forget to pass around
+      the Transport object. This adds ``BzrDir.clone_on_transport()``, so
+      that we can pass in the Transport that we already have.
+      (John Arbash Meinel, #75721)
+
 bzr 0.16rc2  2007-04-30
 
   BUGFIXES:

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2007-04-24 19:43:09 +0000
+++ b/bzrlib/branch.py	2007-05-02 14:54:20 +0000
@@ -783,10 +783,7 @@
         :return: The tree of the created checkout
         """
         t = transport.get_transport(to_location)
-        try:
-            t.mkdir('.')
-        except errors.FileExists:
-            pass
+        t.ensure_base()
         if lightweight:
             format = self._get_checkout_format()
             checkout = format.initialize_on_transport(t)

=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py	2007-04-26 05:06:29 +0000
+++ b/bzrlib/builtins.py	2007-05-02 14:54:20 +0000
@@ -709,7 +709,6 @@
                 location = stored_loc
 
         to_transport = transport.get_transport(location)
-        location_url = to_transport.base
 
         br_to = repository_to = dir_to = None
         try:
@@ -772,12 +771,12 @@
                 # Now we only need to create child directories
                 while needed:
                     cur_transport = needed.pop()
-                    cur_transport.mkdir('.')
-            
+                    cur_transport.ensure_base()
+
             # Now the target directory exists, but doesn't have a .bzr
             # directory. So we need to create it, along with any work to create
             # all of the dependent branches, etc.
-            dir_to = br_from.bzrdir.clone(location_url,
+            dir_to = br_from.bzrdir.clone_on_transport(to_transport,
                 revision_id=br_from.last_revision())
             br_to = dir_to.open_branch()
             # TODO: Some more useful message about what was copied
@@ -1284,11 +1283,8 @@
         # believe that we want to create a bunch of
         # locations if the user supplies an extended path
         # TODO: create-prefix
-        try:
-            to_transport.mkdir('.')
-        except errors.FileExists:
-            pass
-                    
+        to_transport.ensure_base()
+
         try:
             existing_bzrdir = bzrdir.BzrDir.open(location)
         except errors.NotBranchError:
@@ -1349,10 +1345,7 @@
             location = '.'
 
         to_transport = transport.get_transport(location)
-        try:
-            to_transport.mkdir('.')
-        except errors.FileExists:
-            pass
+        to_transport.ensure_base()
 
         newdir = format.initialize_on_transport(to_transport)
         repo = newdir.create_repository(shared=True)

=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2007-04-26 08:15:40 +0000
+++ b/bzrlib/bzrdir.py	2007-05-02 14:54:20 +0000
@@ -155,8 +155,23 @@
         :param force_new_repo: Do not use a shared repository for the target 
                                even if one is available.
         """
-        self._make_tail(url)
-        result = self._format.initialize(url)
+        return self.clone_on_transport(get_transport(url),
+                                       revision_id=revision_id,
+                                       force_new_repo=force_new_repo)
+
+    def clone_on_transport(self, transport, revision_id=None,
+                           force_new_repo=False):
+        """Clone this bzrdir and its contents to transport verbatim.
+
+        If the target directory does not exist, it will be created.
+
+        if revision_id is not None, then the clone operation may tune
+            itself to download less data.
+        :param force_new_repo: Do not use a shared repository for the target 
+                               even if one is available.
+        """
+        transport.ensure_base()
+        result = self._format.initialize_on_transport(transport)
         try:
             local_repo = self.find_repository()
         except errors.NoRepositoryPresent:
@@ -195,13 +210,8 @@
     # TODO: This should be given a Transport, and should chdir up; otherwise
     # this will open a new connection.
     def _make_tail(self, url):
-        head, tail = urlutils.split(url)
-        if tail and tail != '.':
-            t = get_transport(head)
-            try:
-                t.mkdir(tail)
-            except errors.FileExists:
-                pass
+        t = get_transport(url)
+        t.ensure_base()
 
     # TODO: Should take a Transport
     @classmethod
@@ -217,13 +227,8 @@
         if cls is not BzrDir:
             raise AssertionError("BzrDir.create always creates the default"
                 " format, not one of %r" % cls)
-        head, tail = urlutils.split(base)
-        if tail and tail != '.':
-            t = get_transport(head)
-            try:
-                t.mkdir(tail)
-            except errors.FileExists:
-                pass
+        t = get_transport(base)
+        t.ensure_base()
         if format is None:
             format = BzrDirFormat.get_default_format()
         return format.initialize(safe_unicode(base))
@@ -756,9 +761,10 @@
         if revision_id is not None, then the clone operation may tune
             itself to download less data.
         """
-        self._make_tail(url)
+        target_transport = get_transport(url)
+        target_transport.ensure_base()
         cloning_format = self.cloning_metadir()
-        result = cloning_format.initialize(url)
+        result = cloning_format.initialize_on_transport(target_transport)
         try:
             source_branch = self.open_branch()
             source_repository = source_branch.repository

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2007-04-30 19:52:44 +0000
+++ b/bzrlib/tests/__init__.py	2007-05-03 19:22:18 +0000
@@ -1805,10 +1805,7 @@
             segments = maybe_a_url.rsplit('/', 1)
             t = get_transport(maybe_a_url)
             if len(segments) > 1 and segments[-1] not in ('', '.'):
-                try:
-                    t.mkdir('.')
-                except errors.FileExists:
-                    pass
+                t.ensure_base()
             if format is None:
                 format = 'default'
             if isinstance(format, basestring):

=== modified file 'bzrlib/tests/bzrdir_implementations/__init__.py'
--- a/bzrlib/tests/bzrdir_implementations/__init__.py	2007-04-17 09:16:29 +0000
+++ b/bzrlib/tests/bzrdir_implementations/__init__.py	2007-05-01 22:41:41 +0000
@@ -28,12 +28,29 @@
 from bzrlib.tests import (
                           adapt_modules,
                           default_transport,
+                          TestCaseWithTransport,
                           TestLoader,
                           TestSuite,
                           )
 from bzrlib.transport.memory import MemoryServer
 
 
+class TestCaseWithBzrDir(TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestCaseWithBzrDir, self).setUp()
+        self.bzrdir = None
+
+    def get_bzrdir(self):
+        if self.bzrdir is None:
+            self.bzrdir = self.make_bzrdir(None)
+        return self.bzrdir
+
+    def make_bzrdir(self, relpath, format=None):
+        return super(TestCaseWithBzrDir, self).make_bzrdir(
+            relpath, format=self.bzrdir_format)
+
+
 def test_suite():
     result = TestSuite()
     test_bzrdir_implementations = [

=== modified file 'bzrlib/tests/bzrdir_implementations/test_bzrdir.py'
--- a/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2007-04-23 08:30:30 +0000
+++ b/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2007-05-01 22:41:41 +0000
@@ -48,6 +48,7 @@
                           TestCaseWithTransport,
                           TestSkipped,
                           )
+from bzrlib.tests.bzrdir_implementations import TestCaseWithBzrDir
 from bzrlib.trace import mutter
 from bzrlib.transport import get_transport
 from bzrlib.upgrade import upgrade
@@ -55,23 +56,6 @@
 from bzrlib.repofmt import weaverepo
 
 
-class TestCaseWithBzrDir(TestCaseWithTransport):
-
-    def setUp(self):
-        super(TestCaseWithBzrDir, self).setUp()
-        self.bzrdir = None
-
-    def get_bzrdir(self):
-        if self.bzrdir is None:
-            self.bzrdir = self.make_bzrdir(None)
-        return self.bzrdir
-
-    def make_bzrdir(self, relpath, format=None):
-        return super(TestCaseWithBzrDir, self).make_bzrdir(
-            relpath, format=self.bzrdir_format)
-
-
-
 class TestBzrDir(TestCaseWithBzrDir):
     # Many of these tests test for disk equality rather than checking
     # for semantic equivalence. This works well for some tests but
@@ -191,7 +175,15 @@
             # so this test is irrelevant.
             return
         self.assertRaises(errors.NoWorkingTree, dir.open_workingtree)
-            
+
+    def test_clone_on_transport(self):
+        a_dir = self.make_bzrdir('source')
+        target_transport = a_dir.root_transport.clone('..').clone('target')
+        target = a_dir.clone_on_transport(target_transport)
+        self.assertNotEqual(a_dir.transport.base, target.transport.base)
+        self.assertDirectoriesEqual(a_dir.root_transport, target.root_transport,
+                                    ['./.bzr/merge-hashes'])
+
     def test_clone_bzrdir_empty(self):
         dir = self.make_bzrdir('source')
         target = dir.clone(self.get_url('target'))

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-04-23 05:03:44 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-05-02 14:36:55 +0000
@@ -61,6 +61,38 @@
         """Check that transport.get(relpath).read() == content."""
         self.assertEqualDiff(content, transport.get(relpath).read())
 
+    def test_ensure_base_missing(self):
+        """.ensure_base() should create the directory if it doesn't exist"""
+        t = self.get_transport()
+        t_a = t.clone('a')
+        if t_a.is_readonly():
+            self.assertRaises(TransportNotPossible,
+                              t_a.ensure_base)
+            return
+        self.assertTrue(t_a.ensure_base())
+        self.assertTrue(t.has('a'))
+
+    def test_ensure_base_exists(self):
+        """.ensure_base() should just be happy if it already exists"""
+        t = self.get_transport()
+        if t.is_readonly():
+            return
+
+        t.mkdir('a')
+        t_a = t.clone('a')
+        # ensure_base returns False if it didn't create the base
+        self.assertFalse(t_a.ensure_base())
+
+    def test_ensure_base_missing_parent(self):
+        """.ensure_base() will fail if the parent dir doesn't exist"""
+        t = self.get_transport()
+        if t.is_readonly():
+            return
+
+        t_a = t.clone('a')
+        t_b = t_a.clone('b')
+        self.assertRaises(NoSuchFile, t_b.ensure_base)
+
     def test_has(self):
         t = self.get_transport()
 

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-04-26 09:07:38 +0000
+++ b/bzrlib/transport/__init__.py	2007-05-02 14:36:55 +0000
@@ -276,6 +276,22 @@
         """
         raise NotImplementedError(self.clone)
 
+    def ensure_base(self):
+        """Ensure that the directory this transport references exists.
+
+        This will create a directory if it doesn't exist.
+        :return: True if the directory was created, False otherwise.
+        """
+        # The default implementation just uses "Easier to ask for forgiveness
+        # than permission". We attempt to create the directory, and just
+        # suppress a FileExists exception.
+        try:
+            self.mkdir('.')
+        except errors.FileExists:
+            return False
+        else:
+            return True
+
     def should_cache(self):
         """Return True if the data pulled across should be cached locally.
         """

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2007-04-25 06:50:22 +0000
+++ b/bzrlib/workingtree.py	2007-05-02 14:54:20 +0000
@@ -932,20 +932,14 @@
             transport = self.branch.bzrdir.root_transport
             for name in segments:
                 transport = transport.clone(name)
-                try:
-                    transport.mkdir('.')
-                except errors.FileExists:
-                    pass
+                transport.ensure_base()
             return transport
             
         sub_path = self.id2path(file_id)
         branch_transport = mkdirs(sub_path)
         if format is None:
             format = bzrdir.format_registry.make_bzrdir('dirstate-with-subtree')
-        try:
-            branch_transport.mkdir('.')
-        except errors.FileExists:
-            pass
+        branch_transport.ensure_base()
         branch_bzrdir = format.initialize_on_transport(branch_transport)
         try:
             repo = branch_bzrdir.find_repository()




More information about the bazaar-commits mailing list