Merge lp:~spiv/bzr/stacking-friendly-revision-history-verb into lp:~bzr/bzr/trunk-old

[Andrew Bennetts]

[Robert Collins (lifeless)]

Looks conceptually fine. Some thoughts
 - rather than using a special revision marker, use the presence/absence
of null: at the start of the history. This is unambiguous and doesn't
require taking up another special revid.
 - update the version numbers.

 review +1

review: Approve

[Andrew Bennetts (spiv)]

Robert Collins wrote:
> Review: Approve
> Looks conceptually fine. Some thoughts
> - rather than using a special revision marker, use the presence/absence
> of null: at the start of the history. This is unambiguous and doesn't
> require taking up another special revid.

There's no special revids here. There's an explicit slot in the protocol
(and Python API) for saying “yep found it” vs. “ran out of history in this
repo”. So it is unambiguous already, so I'll land that as is.

> - update the version numbers.

Oops, done. Thanks.

[Robert Collins (lifeless)]

Ah, I see, I misread.

I guess what I was trying to get at is that that isn't really needed
with null: being unique anyhow.

Still, it looks ok - go ahead.

-Rob

=== modified file 'NEWS'
--- NEWS	2009-06-15 15:20:24 +0000
+++ NEWS	2009-06-16 02:35:31 +0000
@@ -26,6 +26,14 @@
   rather than requiring all command names be known a-priori.
   (Robert Collins)
 
+Improvements
+************
+
+* Resolving a revno to a revision id on a branch accessed via ``bzr://``
+  or ``bzr+ssh://`` is now much faster and involves no VFS operations.
+  This speeds up commands like ``bzr pull -r 123``.  (Andrew Bennetts)
+
+
 bzr 1.16rc1 "It's yesterday in California" 2009-06-11
 #####################################################
 :Codename: yesterday-in-california

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2009-06-10 03:56:49 +0000
+++ bzrlib/branch.py	2009-06-16 02:35:31 +0000
@@ -91,6 +91,7 @@
         self._revision_history_cache = None
         self._revision_id_to_revno_cache = None
         self._partial_revision_id_to_revno_cache = {}
+        self._partial_revision_history_cache = []
         self._last_revision_info_cache = None
         self._merge_sorted_revisions_cache = None
         self._open_hook()
@@ -125,6 +126,27 @@
             raise errors.UnstackableRepositoryFormat(self.repository._format,
                 self.repository.base)
 
+    def _extend_partial_history(self, stop_index=None, stop_revision=None):
+        """Extend the partial history to include a given index
+
+        If a stop_index is supplied, stop when that index has been reached.
+        If a stop_revision is supplied, stop when that revision is
+        encountered.  Otherwise, stop when the beginning of history is
+        reached.
+
+        :param stop_index: The index which should be present.  When it is
+            present, history extension will stop.
+        :param stop_revision: The revision id which should be present.  When
+            it is encountered, history extension will stop.
+        """
+        if len(self._partial_revision_history_cache) == 0:
+            self._partial_revision_history_cache = [self.last_revision()]
+        repository._iter_for_revno(
+            self.repository, self._partial_revision_history_cache,
+            stop_index=stop_index, stop_revision=stop_revision)
+        if self._partial_revision_history_cache[-1] == _mod_revision.NULL_REVISION:
+            self._partial_revision_history_cache.pop()
+
     @staticmethod
     def open(base, _unsupported=False, possible_transports=None):
         """Open the branch rooted at base.
@@ -698,6 +720,8 @@
         self._revision_id_to_revno_cache = None
         self._last_revision_info_cache = None
         self._merge_sorted_revisions_cache = None
+        self._partial_revision_history_cache = []
+        self._partial_revision_id_to_revno_cache = {}
 
     def _gen_revision_history(self):
         """Return sequence of revision hashes on to this branch.
@@ -831,15 +855,20 @@
         except ValueError:
             raise errors.NoSuchRevision(self, revision_id)
 
+    @needs_read_lock
     def get_rev_id(self, revno, history=None):
         """Find the revision id of the specified revno."""
         if revno == 0:
             return _mod_revision.NULL_REVISION
-        if history is None:
-            history = self.revision_history()
-        if revno <= 0 or revno > len(history):
+        last_revno, last_revid = self.last_revision_info()
+        if revno == last_revno:
+            return last_revid
+        if revno <= 0 or revno > last_revno:
             raise errors.NoSuchRevision(self, revno)
-        return history[revno - 1]
+        distance_from_last = last_revno - revno
+        if len(self._partial_revision_history_cache) <= distance_from_last:
+            self._extend_partial_history(revno)
+        return self._partial_revision_history_cache[revno]
 
     @needs_write_lock
     def pull(self, source, overwrite=False, stop_revision=None,
@@ -2376,13 +2405,11 @@
         self._ignore_fallbacks = kwargs.get('ignore_fallbacks', False)
         super(BzrBranch8, self).__init__(*args, **kwargs)
         self._last_revision_info_cache = None
-        self._partial_revision_history_cache = []
         self._reference_info = None
 
     def _clear_cached_state(self):
         super(BzrBranch8, self)._clear_cached_state()
         self._last_revision_info_cache = None
-        self._partial_revision_history_cache = []
         self._reference_info = None
 
     def _last_revision_info(self):
@@ -2444,35 +2471,6 @@
         self._extend_partial_history(stop_index=last_revno-1)
         return list(reversed(self._partial_revision_history_cache))
 
-    def _extend_partial_history(self, stop_index=None, stop_revision=None):
-        """Extend the partial history to include a given index
-
-        If a stop_index is supplied, stop when that index has been reached.
-        If a stop_revision is supplied, stop when that revision is
-        encountered.  Otherwise, stop when the beginning of history is
-        reached.
-
-        :param stop_index: The index which should be present.  When it is
-            present, history extension will stop.
-        :param revision_id: The revision id which should be present.  When
-            it is encountered, history extension will stop.
-        """
-        repo = self.repository
-        if len(self._partial_revision_history_cache) == 0:
-            iterator = repo.iter_reverse_revision_history(self.last_revision())
-        else:
-            start_revision = self._partial_revision_history_cache[-1]
-            iterator = repo.iter_reverse_revision_history(start_revision)
-            #skip the last revision in the list
-            next_revision = iterator.next()
-        for revision_id in iterator:
-            self._partial_revision_history_cache.append(revision_id)
-            if (stop_index is not None and
-                len(self._partial_revision_history_cache) > stop_index):
-                break
-            if revision_id == stop_revision:
-                break
-
     def _write_revision_history(self, history):
         """Factored out of set_revision_history.
 

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-06-11 06:54:33 +0000
+++ bzrlib/builtins.py	2009-06-16 02:35:31 +0000
@@ -946,29 +946,36 @@
             if branch_to.get_parent() is None or remember:
                 branch_to.set_parent(branch_from.base)
 
-        if revision is not None:
-            revision_id = revision.as_revision_id(branch_from)
-
-        branch_to.lock_write()
+        if branch_from is not branch_to:
+            branch_from.lock_read()
         try:
-            if tree_to is not None:
-                view_info = _get_view_info_for_change_reporter(tree_to)
-                change_reporter = delta._ChangeReporter(
-                    unversioned_filter=tree_to.is_ignored, view_info=view_info)
-                result = tree_to.pull(branch_from, overwrite, revision_id,
-                                      change_reporter,
-                                      possible_transports=possible_transports,
-                                      local=local)
-            else:
-                result = branch_to.pull(branch_from, overwrite, revision_id,
-                                      local=local)
-
-            result.report(self.outf)
-            if verbose and result.old_revid != result.new_revid:
-                log.show_branch_change(branch_to, self.outf, result.old_revno,
-                                       result.old_revid)
+            if revision is not None:
+                revision_id = revision.as_revision_id(branch_from)
+
+            branch_to.lock_write()
+            try:
+                if tree_to is not None:
+                    view_info = _get_view_info_for_change_reporter(tree_to)
+                    change_reporter = delta._ChangeReporter(
+                        unversioned_filter=tree_to.is_ignored,
+                        view_info=view_info)
+                    result = tree_to.pull(
+                        branch_from, overwrite, revision_id, change_reporter,
+                        possible_transports=possible_transports, local=local)
+                else:
+                    result = branch_to.pull(
+                        branch_from, overwrite, revision_id, local=local)
+
+                result.report(self.outf)
+                if verbose and result.old_revid != result.new_revid:
+                    log.show_branch_change(
+                        branch_to, self.outf, result.old_revno,
+                        result.old_revid)
+            finally:
+                branch_to.unlock()
         finally:
-            branch_to.unlock()
+            if branch_from is not branch_to:
+                branch_from.unlock()
 
 
 class cmd_push(Command):

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2009-06-11 09:11:21 +0000
+++ bzrlib/remote.py	2009-06-16 02:35:31 +0000
@@ -28,12 +28,10 @@
     errors,
     graph,
     lockdir,
-    pack,
     repository,
     revision,
     revision as _mod_revision,
     symbol_versioning,
-    urlutils,
 )
 from bzrlib.branch import BranchReferenceFormat
 from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat
@@ -675,6 +673,37 @@
         return self._real_repository.get_missing_parent_inventories(
             check_for_missing_texts=check_for_missing_texts)
 
+    def _get_rev_id_for_revno_vfs(self, revno, known_pair):
+        self._ensure_real()
+        return self._real_repository.get_rev_id_for_revno(
+            revno, known_pair)
+
+    def get_rev_id_for_revno(self, revno, known_pair):
+        """See Repository.get_rev_id_for_revno."""
+        path = self.bzrdir._path_for_remote_call(self._client)
+        try:
+            if self._client._medium._is_remote_before((1, 16)):
+                return self._get_rev_id_for_revno_vfs(revno, known_pair)
+            response = self._call(
+                'Repository.get_rev_id_for_revno', path, revno, known_pair)
+        except errors.UnknownSmartMethod:
+            self._client._medium._remember_remote_is_before((1, 16))
+            return self._get_rev_id_for_revno_vfs(revno, known_pair)
+        if response[0] == 'ok':
+            return True, response[1]
+        elif response[0] == 'history-incomplete':
+            known_pair = response[1:3]
+            for fallback in self._fallback_repositories:
+                found, result = fallback.get_rev_id_for_revno(revno, known_pair)
+                if found:
+                    return True, result
+                else:
+                    known_pair = result
+            # Not found in any fallbacks
+            return False, known_pair
+        else:
+            raise errors.UnexpectedSmartServerResponse(response)
+
     def _ensure_real(self):
         """Ensure that there is a _real_repository set.
 
@@ -1919,11 +1948,6 @@
         # We intentionally don't call the parent class's __init__, because it
         # will try to assign to self.tags, which is a property in this subclass.
         # And the parent's __init__ doesn't do much anyway.
-        self._revision_id_to_revno_cache = None
-        self._partial_revision_id_to_revno_cache = {}
-        self._revision_history_cache = None
-        self._last_revision_info_cache = None
-        self._merge_sorted_revisions_cache = None
         self.bzrdir = remote_bzrdir
         if _client is not None:
             self._client = _client
@@ -1943,6 +1967,7 @@
         else:
             self._real_branch = None
         # Fill out expected attributes of branch for bzrlib API users.
+        self._clear_cached_state()
         self.base = self.bzrdir.root_transport.base
         self._control_files = None
         self._lock_mode = None
@@ -2229,6 +2254,15 @@
             raise NotImplementedError(self.dont_leave_lock_in_place)
         self._leave_lock = False
 
+    def get_rev_id(self, revno, history=None):
+        last_revision_info = self.last_revision_info()
+        ok, result = self.repository.get_rev_id_for_revno(
+            revno, last_revision_info)
+        if ok:
+            return result
+        missing_parent = result[1]
+        raise errors.RevisionNotPresent(missing_parent, self.repository)
+
     def _last_revision_info(self):
         response = self._call('Branch.last_revision_info', self._remote_path())
         if response[0] != 'ok':

=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py	2009-06-12 01:11:00 +0000
+++ bzrlib/repository.py	2009-06-16 02:35:31 +0000
@@ -2234,6 +2234,41 @@
         """
         return self.get_revision(revision_id).inventory_sha1
 
+    def get_rev_id_for_revno(self, revno, known_pair):
+        """Return the revision id of a revno, given a later (revno, revid)
+        pair in the same history.
+
+        :return: if found (True, revid).  If the available history ran out
+            before reaching the revno, then this returns
+            (False, (closest_revno, closest_revid)).
+        """
+        known_revno, known_revid = known_pair
+        partial_history = [known_revid]
+        distance_from_known = known_revno - revno
+        if distance_from_known < 0:
+            raise ValueError(
+                'requested revno (%d) is later than given known revno (%d)'
+                % (revno, known_revno))
+        try:
+            _iter_for_revno(
+                self, partial_history, stop_index=distance_from_known)
+        except errors.RevisionNotPresent, err:
+            if err.revision_id == known_revid:
+                # The start revision (known_revid) wasn't found.
+                raise
+            # This is a stacked repository with no fallbacks, or a there's a
+            # left-hand ghost.  Either way, even though the revision named in
+            # the error isn't in this repo, we know it's the next step in this
+            # left-hand history.
+            partial_history.append(err.revision_id)
+        if len(partial_history) <= distance_from_known:
+            # Didn't find enough history to get a revid for the revno.
+            earliest_revno = known_revno - len(partial_history) + 1
+            return (False, (earliest_revno, partial_history[-1]))
+        if len(partial_history) - 1 > distance_from_known:
+            raise AssertionError('_iter_for_revno returned too much history')
+        return (True, partial_history[-1])
+
     def iter_reverse_revision_history(self, revision_id):
         """Iterate backwards through revision ids in the lefthand history
 
@@ -4417,3 +4452,35 @@
             yield versionedfile.FulltextContentFactory(
                 key, parent_keys, None, as_bytes)
 
+
+def _iter_for_revno(repo, partial_history_cache, stop_index=None,
+                    stop_revision=None):
+    """Extend the partial history to include a given index
+
+    If a stop_index is supplied, stop when that index has been reached.
+    If a stop_revision is supplied, stop when that revision is
+    encountered.  Otherwise, stop when the beginning of history is
+    reached.
+
+    :param stop_index: The index which should be present.  When it is
+        present, history extension will stop.
+    :param stop_revision: The revision id which should be present.  When
+        it is encountered, history extension will stop.
+    """
+    start_revision = partial_history_cache[-1]
+    iterator = repo.iter_reverse_revision_history(start_revision)
+    try:
+        #skip the last revision in the list
+        iterator.next()
+        while True:
+            if (stop_index is not None and
+                len(partial_history_cache) > stop_index):
+                break
+            if partial_history_cache[-1] == stop_revision:
+                break
+            revision_id = iterator.next()
+            partial_history_cache.append(revision_id)
+    except StopIteration:
+        # No more history
+        return
+

=== modified file 'bzrlib/smart/repository.py'
--- bzrlib/smart/repository.py	2009-06-10 03:56:49 +0000
+++ bzrlib/smart/repository.py	2009-06-16 02:35:31 +0000
@@ -19,7 +19,6 @@
 import bz2
 import os
 import Queue
-import struct
 import sys
 import tarfile
 import tempfile
@@ -284,6 +283,31 @@
         return SuccessfulSmartServerResponse(('ok', ), '\n'.join(lines))
 
 
+class SmartServerRepositoryGetRevIdForRevno(SmartServerRepositoryReadLocked):
+
+    def do_readlocked_repository_request(self, repository, revno,
+            known_pair):
+        """Find the revid for a given revno, given a known revno/revid pair.
+        
+        New in 1.16.
+        """
+        try:
+            found_flag, result = repository.get_rev_id_for_revno(revno, known_pair)
+        except errors.RevisionNotPresent, err:
+            if err.revision_id != known_pair[1]:
+                raise AssertionError(
+                    'get_rev_id_for_revno raised RevisionNotPresent for '
+                    'non-initial revision: ' + err.revision_id)
+            return FailedSmartServerResponse(
+                ('nosuchrevision', err.revision_id))
+        if found_flag:
+            return SuccessfulSmartServerResponse(('ok', result))
+        else:
+            earliest_revno, earliest_revid = result
+            return SuccessfulSmartServerResponse(
+                ('history-incomplete', earliest_revno, earliest_revid))
+
+
 class SmartServerRequestHasRevision(SmartServerRepositoryRequest):
 
     def do_repository_request(self, repository, revision_id):

=== modified file 'bzrlib/smart/request.py'
--- bzrlib/smart/request.py	2009-06-12 01:48:43 +0000
+++ bzrlib/smart/request.py	2009-06-16 02:35:31 +0000
@@ -555,6 +555,9 @@
 request_handlers.register_lazy(
     'Repository.unlock', 'bzrlib.smart.repository', 'SmartServerRepositoryUnlock')
 request_handlers.register_lazy(
+    'Repository.get_rev_id_for_revno', 'bzrlib.smart.repository',
+    'SmartServerRepositoryGetRevIdForRevno')
+request_handlers.register_lazy(
     'Repository.get_stream', 'bzrlib.smart.repository',
     'SmartServerRepositoryGetStream')
 request_handlers.register_lazy(

=== modified file 'bzrlib/tests/blackbox/test_pull.py'
--- bzrlib/tests/blackbox/test_pull.py	2009-06-11 07:43:56 +0000
+++ bzrlib/tests/blackbox/test_pull.py	2009-06-16 02:35:32 +0000
@@ -368,8 +368,12 @@
         See <https://launchpad.net/bugs/380314>
         """
         self.setup_smart_server_with_call_log()
+        # Make a stacked-on branch with two commits so that the
+        # revision-history can't be determined just by looking at the parent
+        # field in the revision in the stacked repo.
         parent = self.make_branch_and_tree('parent', format='1.9')
         parent.commit(message='first commit')
+        parent.commit(message='second commit')
         local = parent.bzrdir.sprout('local').open_workingtree()
         local.commit(message='local commit')
         local.branch.create_clone_on_transport(
@@ -383,7 +387,7 @@
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertLength(43, self.hpss_calls)
+        self.assertLength(18, self.hpss_calls)
         remote = Branch.open('stacked')
         self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
 

=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
--- bzrlib/tests/per_repository_reference/__init__.py	2009-06-10 03:56:49 +0000
+++ bzrlib/tests/per_repository_reference/__init__.py	2009-06-16 02:35:32 +0000
@@ -98,6 +98,7 @@
         'bzrlib.tests.per_repository_reference.test_check',
         'bzrlib.tests.per_repository_reference.test_default_stacking',
         'bzrlib.tests.per_repository_reference.test_fetch',
+        'bzrlib.tests.per_repository_reference.test_get_rev_id_for_revno',
         'bzrlib.tests.per_repository_reference.test_initialize',
         'bzrlib.tests.per_repository_reference.test_unlock',
         ]

=== added file 'bzrlib/tests/per_repository_reference/test_get_rev_id_for_revno.py'
--- bzrlib/tests/per_repository_reference/test_get_rev_id_for_revno.py	1970-01-01 00:00:00 +0000
+++ bzrlib/tests/per_repository_reference/test_get_rev_id_for_revno.py	2009-06-16 02:35:32 +0000
@@ -0,0 +1,46 @@
+# Copyright (C) 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
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tests for get_rev_id_for_revno on a repository with external references."""
+
+from bzrlib import errors
+from bzrlib.tests.per_repository_reference import (
+    TestCaseWithExternalReferenceRepository,
+    )
+
+class TestGetRevIdForRevno(TestCaseWithExternalReferenceRepository):
+
+    def test_uses_fallback(self):
+        tree = self.make_branch_and_tree('base')
+        base = tree.branch.repository
+        revid = tree.commit('one')
+        revid2 = tree.commit('two')
+        spare_tree = tree.bzrdir.sprout('spare').open_workingtree()
+        revid3 = spare_tree.commit('three')
+        branch = spare_tree.branch.create_clone_on_transport(
+            self.get_transport('referring'), stacked_on=self.get_url('base'))
+        repo = branch.repository
+        # Sanity check: now repo has 'revid3', and base has 'revid' + 'revid2'
+        self.assertEqual(set([revid3]),
+                set(repo.bzrdir.open_repository().all_revision_ids()))
+        self.assertEqual(set([revid2, revid]),
+                set(base.bzrdir.open_repository().all_revision_ids()))
+        # get_rev_id_for_revno will find revno 1 == 'revid', even though
+        # that revision can only be found in the fallback.
+        repo.lock_read()
+        self.addCleanup(repo.unlock)
+        self.assertEqual(
+            (True, revid), repo.get_rev_id_for_revno(1, (3, revid3)))

=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py	2009-06-12 01:48:43 +0000
+++ bzrlib/tests/test_remote.py	2009-06-16 02:35:32 +0000
@@ -2018,6 +2018,65 @@
         self.assertEqual(('AnUnexpectedError',), e.error_tuple)
 
 
+class TestRepositoryGetRevIdForRevno(TestRemoteRepository):
+
+    def test_ok(self):
+        repo, client = self.setup_fake_client_and_repository('quack')
+        client.add_expected_call(
+            'Repository.get_rev_id_for_revno', ('quack/', 5, (42, 'rev-foo')),
+            'success', ('ok', 'rev-five'))
+        result = repo.get_rev_id_for_revno(5, (42, 'rev-foo'))
+        self.assertEqual((True, 'rev-five'), result)
+        client.finished_test()
+
+    def test_history_incomplete(self):
+        repo, client = self.setup_fake_client_and_repository('quack')
+        client.add_expected_call(
+            'Repository.get_rev_id_for_revno', ('quack/', 5, (42, 'rev-foo')),
+            'success', ('history-incomplete', 10, 'rev-ten'))
+        result = repo.get_rev_id_for_revno(5, (42, 'rev-foo'))
+        self.assertEqual((False, (10, 'rev-ten')), result)
+        client.finished_test()
+
+    def test_history_incomplete_with_fallback(self):
+        """A 'history-incomplete' response causes the fallback repository to be
+        queried too, if one is set.
+        """
+        # Make a repo with a fallback repo, both using a FakeClient.
+        format = remote.response_tuple_to_repo_format(
+            ('yes', 'no', 'yes', 'fake-network-name'))
+        repo, client = self.setup_fake_client_and_repository('quack')
+        repo._format = format
+        fallback_repo, ignored = self.setup_fake_client_and_repository(
+            'fallback')
+        fallback_repo._client = client
+        repo.add_fallback_repository(fallback_repo)
+        # First the client should ask the primary repo
+        client.add_expected_call(
+            'Repository.get_rev_id_for_revno', ('quack/', 1, (42, 'rev-foo')),
+            'success', ('history-incomplete', 2, 'rev-two'))
+        # Then it should ask the fallback, using revno/revid from the
+        # history-incomplete response as the known revno/revid.
+        client.add_expected_call(
+            'Repository.get_rev_id_for_revno',('fallback/', 1, (2, 'rev-two')),
+            'success', ('ok', 'rev-one'))
+        result = repo.get_rev_id_for_revno(1, (42, 'rev-foo'))
+        self.assertEqual((True, 'rev-one'), result)
+        client.finished_test()
+
+    def test_nosuchrevision(self):
+        # 'nosuchrevision' is returned when the known-revid is not found in the
+        # remote repo.  The client translates that response to NoSuchRevision.
+        repo, client = self.setup_fake_client_and_repository('quack')
+        client.add_expected_call(
+            'Repository.get_rev_id_for_revno', ('quack/', 5, (42, 'rev-foo')),
+            'error', ('nosuchrevision', 'rev-foo'))
+        self.assertRaises(
+            errors.NoSuchRevision,
+            repo.get_rev_id_for_revno, 5, (42, 'rev-foo'))
+        client.finished_test()
+
+
 class TestRepositoryIsShared(TestRemoteRepository):
 
     def test_is_shared(self):

=== modified file 'bzrlib/tests/test_smart.py'
--- bzrlib/tests/test_smart.py	2009-06-12 01:48:43 +0000
+++ bzrlib/tests/test_smart.py	2009-06-16 02:35:32 +0000
@@ -1161,6 +1161,47 @@
             request.execute('', 'missingrevision'))
 
 
+class TestSmartServerRepositoryGetRevIdForRevno(tests.TestCaseWithMemoryTransport):
+
+    def test_revno_found(self):
+        backing = self.get_transport()
+        request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing)
+        tree = self.make_branch_and_memory_tree('.')
+        tree.lock_write()
+        tree.add('')
+        rev1_id_utf8 = u'\xc8'.encode('utf-8')
+        rev2_id_utf8 = u'\xc9'.encode('utf-8')
+        tree.commit('1st commit', rev_id=rev1_id_utf8)
+        tree.commit('2nd commit', rev_id=rev2_id_utf8)
+        tree.unlock()
+
+        self.assertEqual(SmartServerResponse(('ok', rev1_id_utf8)),
+            request.execute('', 1, (2, rev2_id_utf8)))
+
+    def test_known_revid_missing(self):
+        backing = self.get_transport()
+        request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing)
+        repo = self.make_repository('.')
+        self.assertEqual(
+            FailedSmartServerResponse(('nosuchrevision', 'ghost')),
+            request.execute('', 1, (2, 'ghost')))
+
+    def test_history_incomplete(self):
+        backing = self.get_transport()
+        request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing)
+        parent = self.make_branch_and_memory_tree('parent', format='1.9')
+        r1 = parent.commit(message='first commit')
+        r2 = parent.commit(message='second commit')
+        local = self.make_branch_and_memory_tree('local', format='1.9')
+        local.branch.pull(parent.branch)
+        local.set_parent_ids([r2])
+        r3 = local.commit(message='local commit')
+        local.branch.create_clone_on_transport(
+            self.get_transport('stacked'), stacked_on=self.get_url('parent'))
+        self.assertEqual(
+            SmartServerResponse(('history-incomplete', 2, r2)),
+            request.execute('stacked', 1, (3, r3)))
+
 class TestSmartServerRepositoryGetStream(tests.TestCaseWithMemoryTransport):
 
     def make_two_commit_repo(self):
@@ -1576,6 +1617,8 @@
             smart.repository.SmartServerRepositoryGatherStats)
         self.assertHandlerEqual('Repository.get_parent_map',
             smart.repository.SmartServerRepositoryGetParentMap)
+        self.assertHandlerEqual('Repository.get_rev_id_for_revno',
+            smart.repository.SmartServerRepositoryGetRevIdForRevno)
         self.assertHandlerEqual('Repository.get_revision_graph',
             smart.repository.SmartServerRepositoryGetRevisionGraph)
         self.assertHandlerEqual('Repository.get_stream',