Merge lp:~jelmer/brz/mainline-ghosts into lp:brz

[Jelmer Vernooij]

Don't apply matcher logic unless --match is specified in ``bzr log``.

Quick performance fix for ``bzr log`` - don't run regexes over all logs that are printed *unless* one of the --match options is specified.

=== modified file 'breezy/annotate.py'
--- breezy/annotate.py	2017-06-04 18:09:30 +0000
+++ breezy/annotate.py	2017-06-22 01:54:53 +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 01:54:53 +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 01:54:53 +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 01:54:53 +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 01:54:53 +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/log.py'
--- breezy/log.py	2017-06-05 20:48:31 +0000
+++ breezy/log.py	2017-06-22 01:54:53 +0000
@@ -409,8 +409,12 @@
 
         # Find and print the interesting revisions
         generator = self._generator_factory(self.branch, rqst)
-        for lr in generator.iter_log_revisions():
-            lf.log_revision(lr)
+        try:
+            for lr in generator.iter_log_revisions():
+                lf.log_revision(lr)
+        except errors.GhostRevisionUnusableHere:
+            raise errors.BzrCommandError(
+                    gettext('Further revision history missing.'))
         lf.show_advice()
 
     def _generator_factory(self, branch, rqst):
@@ -456,6 +460,8 @@
                     continue
                 if omit_merges and len(rev.parent_ids) > 1:
                     continue
+                if rev is None:
+                    raise errors.GhostRevisionUnusableHere(rev_id)
                 if diff_type is None:
                     diff = None
                 else:
@@ -723,6 +729,7 @@
     :param exclude_common_ancestry: Whether the start_rev_id should be part of
         the iterated revisions.
     :return: An iterator of (revision_id, dotted_revno, merge_depth) tuples.
+        dotted_revno will be None for ghosts
     :raises _StartNotLinearAncestor: if a start_rev_id is specified but
         is not found walking the left-hand history
     """
@@ -731,27 +738,46 @@
     graph = repo.get_graph()
     if start_rev_id is None and end_rev_id is None:
         cur_revno = br_revno
-        for revision_id in graph.iter_lefthand_ancestry(br_rev_id,
-            (_mod_revision.NULL_REVISION,)):
-            yield revision_id, str(cur_revno), 0
-            cur_revno -= 1
+        graph_iter = graph.iter_lefthand_ancestry(br_rev_id,
+            (_mod_revision.NULL_REVISION,))
+        while True:
+            try:
+                revision_id = graph_iter.next()
+            except StopIteration:
+                raise
+            except errors.RevisionNotPresent as e:
+                # Oops, a ghost.
+                yield e.revision_id, None, None
+                break
+            else:
+                yield revision_id, str(cur_revno), 0
+                cur_revno -= 1
     else:
         if end_rev_id is None:
             end_rev_id = br_rev_id
         found_start = start_rev_id is None
-        for revision_id in graph.iter_lefthand_ancestry(end_rev_id,
-                (_mod_revision.NULL_REVISION,)):
-            revno_str = _compute_revno_str(branch, revision_id)
-            if not found_start and revision_id == start_rev_id:
-                if not exclude_common_ancestry:
+        graph_iter = graph.iter_lefthand_ancestry(end_rev_id,
+            (_mod_revision.NULL_REVISION,))
+        while True:
+            try:
+                revision_id = graph_iter.next()
+            except StopIteration:
+                break
+            except errors.RevisionNotPresent as e:
+                # Oops, a ghost.
+                yield e.revision_id, None, None
+                break
+            else:
+                revno_str = _compute_revno_str(branch, revision_id)
+                if not found_start and revision_id == start_rev_id:
+                    if not exclude_common_ancestry:
+                        yield revision_id, revno_str, 0
+                    found_start = True
+                    break
+                else:
                     yield revision_id, revno_str, 0
-                found_start = True
-                break
-            else:
-                yield revision_id, revno_str, 0
-        else:
-            if not found_start:
-                raise _StartNotLinearAncestor()
+        if not found_start:
+            raise _StartNotLinearAncestor()
 
 
 def _graph_view_revisions(branch, start_rev_id, end_rev_id,
@@ -861,7 +887,7 @@
     :return: An iterator over lists of ((rev_id, revno, merge_depth), rev,
         delta).
     """
-    if match is None:
+    if not match:
         return log_rev_iterator
     searchRE = [(k, [re.compile(x, re.IGNORECASE) for x in v])
                 for k, v in match.items()]
@@ -998,10 +1024,8 @@
     for revs in log_rev_iterator:
         # r = revision_id, n = revno, d = merge depth
         revision_ids = [view[0] for view, _, _ in revs]
-        revisions = repository.get_revisions(revision_ids)
-        revs = [(rev[0], revision, rev[2]) for rev, revision in
-            zip(revs, revisions)]
-        yield revs
+        revisions = dict(repository.iter_revisions(revision_ids))
+        yield [(rev[0], revisions[rev[0][0]], rev[2]) for rev in revs]
 
 
 def _make_batch_filter(branch, generate_delta, search, log_rev_iterator):

=== 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 01:54:53 +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 01:54:53 +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 01:54:53 +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 01:54:53 +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/blackbox/test_log.py'
--- breezy/tests/blackbox/test_log.py	2017-06-10 00:17:06 +0000
+++ breezy/tests/blackbox/test_log.py	2017-06-22 01:54:53 +0000
@@ -995,16 +995,18 @@
         self.assertLogRevnos([], ["2", "1"])
 
     def test_log_range_open_begin(self):
-        self.knownFailure("log with ghosts fails. bug #726466")
         (stdout, stderr) = self.run_bzr(['log', '-r..2'], retcode=3)
         self.assertEqual(["2", "1"],
                          [r.revno for r in self.get_captured_revisions()])
-        self.assertEqual("brz: ERROR: Further revision history missing.", stderr)
+        self.assertEqual("brz: ERROR: Further revision history missing.\n",
+                stderr)
 
     def test_log_range_open_end(self):
         self.assertLogRevnos(["-r1.."], ["2", "1"])
 
+
 class TestLogMatch(TestLogWithLogCatcher):
+
     def prepare_tree(self):
         tree = self.make_branch_and_tree('')
         self.build_tree(
@@ -1013,7 +1015,7 @@
         tree.commit(message='message1', committer='committer1', authors=['author1'])
         tree.add('goodbye.txt')
         tree.commit(message='message2', committer='committer2', authors=['author2'])
-    
+
     def test_message(self):
         self.prepare_tree()
         self.assertLogRevnos(["-m", "message1"], ["1"])

=== 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 01:54:53 +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 01:54:53 +0000
@@ -104,6 +104,9 @@
 * Support ``brz commit -x`` in combination with iter_changes.
   (Jelmer Vernooij, #796582, #403811, #694946, #268135, #299879)
 
+* Print a proper error when encountering ghost revisions in
+  mainline in ``bzr log``. (Jelmer Vernooij, #726466)
+
 Documentation
 *************
 
@@ -151,6 +154,9 @@
  * ``Repository.get_revisions`` no longer accepts ``None`` as
    argument. (Jelmer Vernooij)
 
+ * A new ``Repository.iter_revisions`` method has been added.
+   (Jelmer Vernooij)
+
 Internals
 *********