Rev 4066: Change the return value of fetch() to None. in http://people.ubuntu.com/~robertc/baz2.0/fetch.returns-none

Robert Collins robertc at robertcollins.net
Mon Mar 2 04:45:09 GMT 2009


At http://people.ubuntu.com/~robertc/baz2.0/fetch.returns-none

------------------------------------------------------------
revno: 4066
revision-id: robertc at robertcollins.net-20090302044502-ev676ddgmzsvny8u
parent: pqm at pqm.ubuntu.com-20090301222412-o2s34646lnn958f3
committer: Robert Collins <robertc at robertcollins.net>
branch nick: fetch.returns-none
timestamp: Mon 2009-03-02 15:45:02 +1100
message:
  Change the return value of fetch() to None.
=== modified file 'NEWS'
--- a/NEWS	2009-03-01 02:57:51 +0000
+++ b/NEWS	2009-03-02 04:45:02 +0000
@@ -108,6 +108,13 @@
       updated to comply with this. The code already complied with the other
       criteria, but now it is enforced. (Marius Kruger)
 
+    * ``Branch.fetch`` and ``Repository.fetch`` now return None rather
+      than a count of copied revisions and failed revisions. A while back
+      we stopped ever reporting failed revisions because we started
+      erroring instead, and the copied revisions count is not used in the
+      UI at all - indeed it only reflects the repository status not
+      changes to the branch itself. (Robert Collins)
+
     * The ``_fetch_*`` attributes on ``Repository`` are now on
       ``RepositoryFormat``, more accurately reflecting their intent (they
       describe a disk format capability, not state of a particular

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2009-02-26 03:15:58 +0000
+++ b/bzrlib/branch.py	2009-03-02 04:45:02 +0000
@@ -464,9 +464,7 @@
         :param last_revision: What revision to stop at (None for at the end
                               of the branch.
         :param pb: An optional progress bar to use.
-
-        Returns the copied revision count and the failed revisions in a tuple:
-        (copied, failures).
+        :return: None
         """
         if self.base == from_branch.base:
             return (0, [])

=== modified file 'bzrlib/fetch.py'
--- a/bzrlib/fetch.py	2009-02-27 03:52:16 +0000
+++ b/bzrlib/fetch.py	2009-03-02 04:45:02 +0000
@@ -66,9 +66,6 @@
     last_revision
         if set, try to limit to the data this revision references.
 
-    after running:
-    count_copied -- number of revisions copied
-
     This should not be used directly, it's essential a object to encapsulate
     the logic in InterRepository.fetch().
     """
@@ -82,9 +79,6 @@
             exists to facilitate a hack done in InterPackRepo.fetch.  We would
             like to remove this parameter.
         """
-        # result variables.
-        self.failed_revisions = []
-        self.count_copied = 0
         if to_repository.has_same_location(from_repository):
             # repository.fetch should be taking care of this case.
             raise errors.BzrError('RepoFetcher run '
@@ -223,7 +217,6 @@
                     yield _
             else:
                 raise AssertionError("Unknown knit kind %r" % knit_kind)
-        self.count_copied += len(revs)
 
     def get_stream_for_missing_keys(self, missing_keys):
         # missing keys can only occur when we are byte copying and not

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2009-02-27 01:02:40 +0000
+++ b/bzrlib/repository.py	2009-03-02 04:45:02 +0000
@@ -2571,8 +2571,7 @@
                             content is copied.
         :param pb: optional progress bar to use for progress reports. If not
                    provided a default one will be created.
-
-        :returns: (copied_revision_count, failures).
+        :return: None.
         """
         # Normally we should find a specific InterRepository subclass to do
         # the fetch; if nothing else then at least InterSameDataRepository.
@@ -2761,7 +2760,6 @@
                                from_repository=self.source,
                                last_revision=revision_id,
                                pb=pb, find_ghosts=find_ghosts)
-        return f.count_copied, f.failed_revisions
 
 
 class InterWeaveRepo(InterSameDataRepository):
@@ -2841,7 +2839,6 @@
                                from_repository=self.source,
                                last_revision=revision_id,
                                pb=pb, find_ghosts=find_ghosts)
-        return f.count_copied, f.failed_revisions
 
     @needs_read_lock
     def search_missing_revision_ids(self, revision_id=None, find_ghosts=True):
@@ -2922,7 +2919,6 @@
                             from_repository=self.source,
                             last_revision=revision_id,
                             pb=pb, find_ghosts=find_ghosts)
-        return f.count_copied, f.failed_revisions
 
     @needs_read_lock
     def search_missing_revision_ids(self, revision_id=None, find_ghosts=True):
@@ -2994,7 +2990,6 @@
             from bzrlib.fetch import RepoFetcher
             fetcher = RepoFetcher(self.target, self.source, revision_id,
                                   pb, find_ghosts)
-            return fetcher.count_copied, fetcher.failed_revisions
         mutter("Using fetch logic to copy between %s(%s) and %s(%s)",
                self.source, self.source._format, self.target, self.target._format)
         self.count_copied = 0
@@ -3117,7 +3112,6 @@
                                  from_repository=self.source,
                                  last_revision=revision_id,
                                  pb=pb, find_ghosts=find_ghosts)
-        return f.count_copied, f.failed_revisions
 
     @needs_write_lock
     def copy_content(self, revision_id=None):
@@ -3210,7 +3204,6 @@
                             from_repository=self.source,
                             last_revision=revision_id,
                             pb=pb, find_ghosts=find_ghosts)
-        return f.count_copied, f.failed_revisions
 
 
 class InterDifferingSerializer(InterKnitRepo):
@@ -3494,7 +3487,6 @@
         from bzrlib.fetch import RepoFetcher
         fetcher = RepoFetcher(self.target, self.source, revision_id,
                               pb, find_ghosts)
-        return fetcher.count_copied, fetcher.failed_revisions
 
     def _get_target_pack_collection(self):
         return self.target._real_repository._pack_collection

=== modified file 'bzrlib/tests/branch_implementations/test_branch.py'
--- a/bzrlib/tests/branch_implementations/test_branch.py	2009-02-24 08:09:17 +0000
+++ b/bzrlib/tests/branch_implementations/test_branch.py	2009-03-02 04:45:02 +0000
@@ -109,7 +109,7 @@
         wt.commit('lala!', rev_id='revision-1', allow_pointless=False)
 
         b2 = self.make_branch('b2')
-        self.assertEqual((1, []), b2.fetch(b1))
+        b2.fetch(b1)
 
         rev = b2.repository.get_revision('revision-1')
         tree = b2.repository.revision_tree('revision-1')

=== modified file 'bzrlib/tests/test_fetch.py'
--- a/bzrlib/tests/test_fetch.py	2009-02-27 01:02:40 +0000
+++ b/bzrlib/tests/test_fetch.py	2009-03-02 04:45:02 +0000
@@ -53,13 +53,13 @@
     self.assertFalse(repo_b.has_revision(br_a.revision_history()[3]))
     self.assertTrue(repo_b.has_revision(br_a.revision_history()[2]))
     self.assertEquals(len(br_b.revision_history()), 7)
-    self.assertEquals(br_b.fetch(br_a, br_a.revision_history()[2])[0], 0)
+    br_b.fetch(br_a, br_a.revision_history()[2])
     # branch.fetch is not supposed to alter the revision history
     self.assertEquals(len(br_b.revision_history()), 7)
     self.assertFalse(repo_b.has_revision(br_a.revision_history()[3]))
 
     # fetching the next revision up in sample data copies one revision
-    self.assertEquals(br_b.fetch(br_a, br_a.revision_history()[3])[0], 1)
+    br_b.fetch(br_a, br_a.revision_history()[3])
     self.assertTrue(repo_b.has_revision(br_a.revision_history()[3]))
     self.assertFalse(has_revision(br_a, br_b.revision_history()[6]))
     self.assertTrue(br_a.repository.has_revision(br_b.revision_history()[5]))
@@ -67,22 +67,20 @@
     # When a non-branch ancestor is missing, it should be unlisted...
     # as its not reference from the inventory weave.
     br_b4 = self.make_branch('br_4')
-    count, failures = br_b4.fetch(br_b)
-    self.assertEqual(count, 7)
-    self.assertEqual(failures, [])
+    br_b4.fetch(br_b)
 
-    self.assertEqual(writable_a.fetch(br_b)[0], 1)
+    writable_a.fetch(br_b)
     self.assertTrue(has_revision(br_a, br_b.revision_history()[3]))
     self.assertTrue(has_revision(br_a, br_b.revision_history()[4]))
 
     br_b2 = self.make_branch('br_b2')
-    self.assertEquals(br_b2.fetch(br_b)[0], 7)
+    br_b2.fetch(br_b)
     self.assertTrue(has_revision(br_b2, br_b.revision_history()[4]))
     self.assertTrue(has_revision(br_b2, br_a.revision_history()[2]))
     self.assertFalse(has_revision(br_b2, br_a.revision_history()[3]))
 
     br_a2 = self.make_branch('br_a2')
-    self.assertEquals(br_a2.fetch(br_a)[0], 9)
+    br_a2.fetch(br_a)
     self.assertTrue(has_revision(br_a2, br_b.revision_history()[4]))
     self.assertTrue(has_revision(br_a2, br_a.revision_history()[3]))
     self.assertTrue(has_revision(br_a2, br_a.revision_history()[2]))
@@ -90,14 +88,13 @@
     br_a3 = self.make_branch('br_a3')
     # pulling a branch with no revisions grabs nothing, regardless of
     # whats in the inventory.
-    self.assertEquals(br_a3.fetch(br_a2)[0], 0)
+    br_a3.fetch(br_a2)
     for revno in range(4):
         self.assertFalse(
             br_a3.repository.has_revision(br_a.revision_history()[revno]))
-    self.assertEqual(br_a3.fetch(br_a2, br_a.revision_history()[2])[0], 3)
+    br_a3.fetch(br_a2, br_a.revision_history()[2])
     # pull the 3 revisions introduced by a at u-0-3
-    fetched = br_a3.fetch(br_a2, br_a.revision_history()[3])[0]
-    self.assertEquals(fetched, 3, "fetched %d instead of 3" % fetched)
+    br_a3.fetch(br_a2, br_a.revision_history()[3])
     # InstallFailed should be raised if the branch is missing the revision
     # that was requested.
     self.assertRaises(errors.InstallFailed, br_a3.fetch, br_a2, 'pizza')
@@ -122,7 +119,7 @@
 
     def test_fetch_self(self):
         wt = self.make_branch_and_tree('br')
-        self.assertEqual(wt.branch.fetch(wt.branch), (0, []))
+        wt.branch.fetch(wt.branch)
 
     def test_fetch_root_knit(self):
         """Ensure that knit2.fetch() updates the root knit
@@ -284,7 +281,7 @@
         wt.commit("changed file")
         target = BzrDir.create_branch_and_repo("target/")
         source = Branch.open(self.get_readonly_url("source/"))
-        self.assertEqual(target.fetch(source), (2, []))
+        target.fetch(source)
         # this is the path to the literal file. As format changes
         # occur it needs to be updated. FIXME: ask the store for the
         # path.
@@ -315,7 +312,7 @@
         source = Branch.open(
             self.get_readonly_url("source/"),
             possible_transports=[source.bzrdir.root_transport])
-        self.assertEqual(target.fetch(source), (0, []))
+        target.fetch(source)
         # should make just two requests
         http_logs = self.get_readonly_server().logs
         self.log("web server logs are:")




More information about the bazaar-commits mailing list