Merge lp:~jelmer/brz/iter-revisions into lp:brz

[Jelmer Vernooij]

Add Repository.iter_revisions.

Add Repository.iter_revisions.

This functionality already existed, but was mostly private. It's like Repository.get_revisions, but it can cope with missing revisions and doesn't necessarily yield revisions in order.

[Martin Packman (gz)]

I'm missing some of the subtleties here, but the changes look okay to me. We're currently forcing passed revision_ids to be lists everywhere, right? That could be loosened later maybe.

review: Approve

=== modified file 'breezy/annotate.py'
--- breezy/annotate.py	2017-06-04 18:09:30 +0000
+++ breezy/annotate.py	2017-06-22 22:20:42 +0000
@@ -188,10 +188,10 @@
         revision_id_to_revno[CURRENT_REVISION] = (
             "%d?" % (branch.revno() + 1),)
         revisions[CURRENT_REVISION] = current_rev
-    revision_ids = [o for o in revision_ids if
-                    repository.has_revision(o)]
-    revisions.update((r.revision_id, r) for r in
-                     repository.get_revisions(revision_ids))
+    revisions.update(
+            entry for entry in
+            repository.iter_revisions(revision_ids)
+            if entry[1] is not None)
     for origin, text in annotations:
         text = text.rstrip('\r\n')
         if origin == last_origin:

=== modified file 'breezy/bzr/groupcompress_repo.py'
--- breezy/bzr/groupcompress_repo.py	2017-06-22 00:21:13 +0000
+++ breezy/bzr/groupcompress_repo.py	2017-06-22 22:20:42 +0000
@@ -1136,7 +1136,7 @@
             raise AssertionError()
         vf = self.revisions
         if revisions_iterator is None:
-            revisions_iterator = self._iter_revisions(self.all_revision_ids())
+            revisions_iterator = self.iter_revisions(self.all_revision_ids())
         for revid, revision in revisions_iterator:
             if revision is None:
                 pass

=== modified file 'breezy/bzr/remote.py'
--- breezy/bzr/remote.py	2017-06-22 00:23:56 +0000
+++ breezy/bzr/remote.py	2017-06-22 22:20:42 +0000
@@ -2658,39 +2658,29 @@
             yield serializer.read_revision_from_string("".join(chunks))
 
     @needs_read_lock
-    def get_revisions(self, revision_ids):
+    def iter_revisions(self, revision_ids):
         for rev_id in revision_ids:
             if not rev_id or not isinstance(rev_id, bytes):
                 raise errors.InvalidRevisionId(
                     revision_id=rev_id, branch=self)
         try:
             missing = set(revision_ids)
-            revs = {}
             for rev in self._iter_revisions_rpc(revision_ids):
                 missing.remove(rev.revision_id)
-                revs[rev.revision_id] = rev
+                yield rev.revision_id, rev
+            for fallback in self._fallback_repositories:
+                if not missing:
+                    break
+                for revid, rev in fallback.iter_revisions(missing):
+                    if rev is not None:
+                        yield revid, rev
+                        missing.remove(revid)
+            for revid in missing:
+                yield revid, None
         except errors.UnknownSmartMethod:
             self._ensure_real()
-            return self._real_repository.get_revisions(revision_ids)
-        for fallback in self._fallback_repositories:
-            if not missing:
-                break
-            for revid in list(missing):
-                # XXX JRV 2011-11-20: It would be nice if there was a
-                # public method on Repository that could be used to query
-                # for revision objects *without* failing completely if one
-                # was missing. There is VersionedFileRepository._iter_revisions,
-                # but unfortunately that's private and not provided by
-                # all repository implementations.
-                try:
-                    revs[revid] = fallback.get_revision(revid)
-                except errors.NoSuchRevision:
-                    pass
-                else:
-                    missing.remove(revid)
-        if missing:
-            raise errors.NoSuchRevision(self, list(missing)[0])
-        return [revs[revid] for revid in revision_ids]
+            for entry in self._real_repository.iter_revisions(revision_ids):
+                yield entry
 
     def supports_rich_root(self):
         return self._format.rich_root_data

=== modified file 'breezy/bzr/vf_repository.py'
--- breezy/bzr/vf_repository.py	2017-06-22 00:21:13 +0000
+++ breezy/bzr/vf_repository.py	2017-06-22 22:20:42 +0000
@@ -1102,28 +1102,9 @@
         be used by reconcile, or reconcile-alike commands that are correcting
         or testing the revision graph.
         """
-        return self._get_revisions([revision_id])[0]
-
-    @needs_read_lock
-    def get_revisions(self, revision_ids):
-        """Get many revisions at once.
-        
-        Repositories that need to check data on every revision read should 
-        subclass this method.
-        """
-        return self._get_revisions(revision_ids)
-
-    @needs_read_lock
-    def _get_revisions(self, revision_ids):
-        """Core work logic to get many revisions without sanity checks."""
-        revs = {}
-        for revid, rev in self._iter_revisions(revision_ids):
-            if rev is None:
-                raise errors.NoSuchRevision(self, revid)
-            revs[revid] = rev
-        return [revs[revid] for revid in revision_ids]
-
-    def _iter_revisions(self, revision_ids):
+        return self.get_revisions([revision_id])[0]
+
+    def iter_revisions(self, revision_ids):
         """Iterate over revision objects.
 
         :param revision_ids: An iterable of revisions to examine. None may be
@@ -1133,19 +1114,23 @@
         :return: An iterator of (revid, revision) tuples. Absent revisions (
             those asked for but not available) are returned as (revid, None).
         """
-        for rev_id in revision_ids:
-            if not rev_id or not isinstance(rev_id, bytes):
-                raise errors.InvalidRevisionId(revision_id=rev_id, branch=self)
-        keys = [(key,) for key in revision_ids]
-        stream = self.revisions.get_record_stream(keys, 'unordered', True)
-        for record in stream:
-            revid = record.key[0]
-            if record.storage_kind == 'absent':
-                yield (revid, None)
-            else:
-                text = record.get_bytes_as('fulltext')
-                rev = self._serializer.read_revision_from_string(text)
-                yield (revid, rev)
+        self.lock_read()
+        try:
+            for rev_id in revision_ids:
+                if not rev_id or not isinstance(rev_id, bytes):
+                    raise errors.InvalidRevisionId(revision_id=rev_id, branch=self)
+            keys = [(key,) for key in revision_ids]
+            stream = self.revisions.get_record_stream(keys, 'unordered', True)
+            for record in stream:
+                revid = record.key[0]
+                if record.storage_kind == 'absent':
+                    yield (revid, None)
+                else:
+                    text = record.get_bytes_as('fulltext')
+                    rev = self._serializer.read_revision_from_string(text)
+                    yield (revid, rev)
+        finally:
+            self.unlock()
 
     @needs_write_lock
     def add_signature_text(self, revision_id, signature):
@@ -1677,7 +1662,7 @@
             raise AssertionError()
         vf = self.revisions
         if revisions_iterator is None:
-            revisions_iterator = self._iter_revisions(self.all_revision_ids())
+            revisions_iterator = self.iter_revisions(self.all_revision_ids())
         for revid, revision in revisions_iterator:
             if revision is None:
                 pass

=== modified file 'breezy/check.py'
--- breezy/check.py	2017-06-22 00:21:13 +0000
+++ breezy/check.py	2017-06-22 22:20:42 +0000
@@ -181,7 +181,7 @@
 
     def check_revisions(self):
         """Scan revisions, checking data directly available as we go."""
-        revision_iterator = self.repository._iter_revisions(
+        revision_iterator = self.repository.iter_revisions(
             self.repository.all_revision_ids())
         revision_iterator = self._check_revisions(revision_iterator)
         # We read the all revisions here:

=== modified file 'breezy/plugins/stats/cmds.py'
--- breezy/plugins/stats/cmds.py	2017-06-05 21:24:34 +0000
+++ breezy/plugins/stats/cmds.py	2017-06-22 22:20:42 +0000
@@ -148,8 +148,8 @@
     pb = ui.ui_factory.nested_progress_bar()
     try:
         trace.note('getting revisions')
-        revisions = a_repo.get_revisions(revids)
-        for count, rev in enumerate(revisions):
+        revisions = a_repo.iter_revisions(revids)
+        for count, (revid, rev) in enumerate(revisions):
             pb.update('checking', count, len(revids))
             for author in rev.get_apparent_authors():
                 # XXX: There is a chance sometimes with svn imports that the

=== modified file 'breezy/plugins/weave_fmt/repository.py'
--- breezy/plugins/weave_fmt/repository.py	2017-06-14 23:29:06 +0000
+++ breezy/plugins/weave_fmt/repository.py	2017-06-22 22:20:42 +0000
@@ -166,11 +166,6 @@
         self.start_write_group()
         return result
 
-    @needs_read_lock
-    def get_revisions(self, revision_ids):
-        revs = self._get_revisions(revision_ids)
-        return revs
-
     def _inventory_add_lines(self, revision_id, parents, lines,
         check_content=True):
         """Store lines in inv_vf and return the sha1 of the inventory."""

=== modified file 'breezy/repository.py'
--- breezy/repository.py	2017-06-20 01:35:59 +0000
+++ breezy/repository.py	2017-06-22 22:20:42 +0000
@@ -822,11 +822,29 @@
 
     def get_revisions(self, revision_ids):
         """Get many revisions at once.
-        
-        Repositories that need to check data on every revision read should 
+
+        Repositories that need to check data on every revision read should
         subclass this method.
         """
-        raise NotImplementedError(self.get_revisions)
+        revs = {}
+        for revid, rev in self.iter_revisions(revision_ids):
+            if rev is None:
+                raise errors.NoSuchRevision(self, revid)
+            revs[revid] = rev
+        return [revs[revid] for revid in revision_ids]
+
+    def iter_revisions(self, revision_ids):
+        """Iterate over revision objects.
+
+        :param revision_ids: An iterable of revisions to examine. None may be
+            passed to request all revisions known to the repository. Note that
+            not all repositories can find unreferenced revisions; for those
+            repositories only referenced ones will be returned.
+        :return: An iterator of (revid, revision) tuples. Absent revisions (
+            those asked for but not available) are returned as (revid, None).
+            N.B.: Revisions are not necessarily yielded in order.
+        """
+        raise NotImplementedError(self.iter_revisions)
 
     def get_deltas_for_revisions(self, revisions, specific_fileids=None):
         """Produce a generator of revision deltas.

=== modified file 'breezy/status.py'
--- breezy/status.py	2017-05-25 01:35:55 +0000
+++ breezy/status.py	2017-06-22 22:20:42 +0000
@@ -297,7 +297,7 @@
     log_formatter = log.LineLogFormatter(to_file)
     for merge in pending:
         try:
-            rev = branch.repository.get_revisions([merge])[0]
+            rev = branch.repository.get_revision(merge)
         except errors.NoSuchRevision:
             # If we are missing a revision, just print out the revision id
             to_file.write(first_prefix + '(ghost) ' + merge + '\n')
@@ -316,19 +316,7 @@
         merge_extra.discard(_mod_revision.NULL_REVISION)
 
         # Get a handle to all of the revisions we will need
-        try:
-            revisions = dict((rev.revision_id, rev) for rev in
-                             branch.repository.get_revisions(merge_extra))
-        except errors.NoSuchRevision:
-            # One of the sub nodes is a ghost, check each one
-            revisions = {}
-            for revision_id in merge_extra:
-                try:
-                    rev = branch.repository.get_revisions([revision_id])[0]
-                except errors.NoSuchRevision:
-                    revisions[revision_id] = None
-                else:
-                    revisions[revision_id] = rev
+        revisions = dict(branch.repository.iter_revisions(merge_extra))
 
         # Display the revisions brought in by this merge.
         rev_id_iterator = _get_sorted_revisions(merge, merge_extra,

=== modified file 'breezy/tests/per_repository/test_repository.py'
--- breezy/tests/per_repository/test_repository.py	2017-06-11 14:07:05 +0000
+++ breezy/tests/per_repository/test_repository.py	2017-06-22 22:20:42 +0000
@@ -424,6 +424,25 @@
             self.assertEqual(revision.revision_id, revision_id)
             self.assertEqual(revision, repo.get_revision(revision_id))
 
+    def test_iter_revisions(self):
+        tree = self.make_branch_and_tree('.')
+        tree.commit('initial empty commit', rev_id='a-rev',
+                    allow_pointless=True)
+        tree.commit('second empty commit', rev_id='b-rev',
+                    allow_pointless=True)
+        tree.commit('third empty commit', rev_id='c-rev',
+                    allow_pointless=True)
+        repo = tree.branch.repository
+        revision_ids = ['a-rev', 'c-rev', 'b-rev', 'd-rev']
+        revid_with_rev = repo.iter_revisions(revision_ids)
+        self.assertEqual(
+            set((revid, rev.revision_id if rev is not None else None)
+                for (revid, rev) in revid_with_rev),
+            {('a-rev', 'a-rev'),
+             ('b-rev', 'b-rev'),
+             ('c-rev', 'c-rev'),
+             ('d-rev', None)})
+
     def test_root_entry_has_revision(self):
         tree = self.make_branch_and_tree('.')
         tree.commit('message', rev_id='rev_id')

=== modified file 'doc/en/release-notes/brz-3.0.txt'
--- doc/en/release-notes/brz-3.0.txt	2017-06-22 00:21:13 +0000
+++ doc/en/release-notes/brz-3.0.txt	2017-06-22 22:20:42 +0000
@@ -151,6 +151,9 @@
  * ``Repository.get_revisions`` no longer accepts ``None`` as
    argument. (Jelmer Vernooij)
 
+ * A new ``Repository.iter_revisions`` method has been added.
+   (Jelmer Vernooij)
+
 Internals
 *********