Merge lp:~spiv/bzr/stacking-friendly-revision-history-verb into lp:~bzr/bzr/trunk-old
- stacking-friendly-revision-history-verb
- Merge into trunk-old
Proposed by
Andrew Bennetts
| Status: | Merged |
|---|---|
| Merge reported by: | Martin Pool |
| Merged at revision: | not available |
| Proposed branch: | lp:~spiv/bzr/stacking-friendly-revision-history-verb |
| Merge into: | lp:~bzr/bzr/trunk-old |
| Diff against target: | 492 lines |
| To merge this branch: | bzr merge lp:~spiv/bzr/stacking-friendly-revision-history-verb |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Martin Pool | Approve | ||
|
Review via email:
|
|||
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
| Andrew Bennetts (spiv) wrote : | # |
Revision history for this message
| Andrew Bennetts (spiv) wrote : | # |
To clarify: this proposal is only for r4423 of my branch. The diff shown inline on Launchpad has this content. The longer (>400 line diff) you can download via the librarian includes in the (not quite ready) followup work.
Revision history for this message
| Martin Pool (mbp) wrote : | # |
Does this need a NEWS entry?
Please put the bug url in a comment in the tests to give more context.
The rest of these are general comments are needn't delay merging:
65 + # This figure represent the amount of work to perform this use case. It
66 + # is entirely ok to reduce this number if a test fails due to rpc_count
67 + # being too low. If rpc_count increases, more network roundtrips have
68 + # become necessary for this use case. Please do not adjust this number
69 + # upwards without agreement from bzr's network support maintainers.
70 + self.assertLeng
This feels like it should be a specific assertion rather than copy and pasted so many times...
It's unfortunate to hardcode 1.9 in these tests but we should probably scan for them later.
88 + except unstackable_
89 + raise TestNotApplicab
Some people would say you should check at the start of the test whether it promises to support stacking.
Looks good.
review:
Approve
Revision history for this message
| Andrew Bennetts (spiv) wrote : | # |
> Does this need a NEWS entry?
Yes, I'll add one.
> Please put the bug url in a comment in the tests to give more context.
Ok.
> The rest of these are general comments are needn't delay merging:
>
> 65 + # This figure represent the amount of work to perform this use case.
> It
> 66 + # is entirely ok to reduce this number if a test fails due to
> rpc_count
> 67 + # being too low. If rpc_count increases, more network roundtrips
> have
> 68 + # become necessary for this use case. Please do not adjust this
> number
> 69 + # upwards without agreement from bzr's network support maintainers.
> 70 + self.assertLeng
>
> This feels like it should be a specific assertion rather than copy and pasted
> so many times...
Yeah. Another possibility is a centralised list of ratchet values somewhere, and then the scary "DO NOT CHANGE UPWARDS" comment can be at that one place.
> It's unfortunate to hardcode 1.9 in these tests but we should probably scan
> for them later.
Yeah. That's how Robert and I feel about the hardcoded 1.9 in these tests too.
> 88 + except unstackable_
> 89 + raise TestNotApplicab
>
> Some people would say you should check at the start of the test whether it
> promises to support stacking.
Well, I'm doing what nearby tests do. (Also, you can't do it at the very start, because at least for the Remote* tests you can't ask the format if it supports stacking unless there is an underlying object for the Remote*Format to ask).
> Looks good.
Thanks!
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'bzrlib/branch.py' | |||
| 2 | --- bzrlib/branch.py 2009-06-10 03:56:49 +0000 | |||
| 3 | +++ bzrlib/branch.py 2009-06-11 19:35:21 +0000 | |||
| 4 | @@ -91,6 +91,7 @@ | |||
| 5 | 91 | self._revision_history_cache = None | 91 | self._revision_history_cache = None |
| 6 | 92 | self._revision_id_to_revno_cache = None | 92 | self._revision_id_to_revno_cache = None |
| 7 | 93 | self._partial_revision_id_to_revno_cache = {} | 93 | self._partial_revision_id_to_revno_cache = {} |
| 8 | 94 | self._partial_revision_history_cache = [] | ||
| 9 | 94 | self._last_revision_info_cache = None | 95 | self._last_revision_info_cache = None |
| 10 | 95 | self._merge_sorted_revisions_cache = None | 96 | self._merge_sorted_revisions_cache = None |
| 11 | 96 | self._open_hook() | 97 | self._open_hook() |
| 12 | @@ -125,6 +126,27 @@ | |||
| 13 | 125 | raise errors.UnstackableRepositoryFormat(self.repository._format, | 126 | raise errors.UnstackableRepositoryFormat(self.repository._format, |
| 14 | 126 | self.repository.base) | 127 | self.repository.base) |
| 15 | 127 | 128 | ||
| 16 | 129 | def _extend_partial_history(self, stop_index=None, stop_revision=None): | ||
| 17 | 130 | """Extend the partial history to include a given index | ||
| 18 | 131 | |||
| 19 | 132 | If a stop_index is supplied, stop when that index has been reached. | ||
| 20 | 133 | If a stop_revision is supplied, stop when that revision is | ||
| 21 | 134 | encountered. Otherwise, stop when the beginning of history is | ||
| 22 | 135 | reached. | ||
| 23 | 136 | |||
| 24 | 137 | :param stop_index: The index which should be present. When it is | ||
| 25 | 138 | present, history extension will stop. | ||
| 26 | 139 | :param stop_revision: The revision id which should be present. When | ||
| 27 | 140 | it is encountered, history extension will stop. | ||
| 28 | 141 | """ | ||
| 29 | 142 | if len(self._partial_revision_history_cache) == 0: | ||
| 30 | 143 | self._partial_revision_history_cache = [self.last_revision()] | ||
| 31 | 144 | repository._iter_for_revno( | ||
| 32 | 145 | self.repository, self._partial_revision_history_cache, | ||
| 33 | 146 | stop_index=stop_index, stop_revision=stop_revision) | ||
| 34 | 147 | if self._partial_revision_history_cache[-1] == _mod_revision.NULL_REVISION: | ||
| 35 | 148 | self._partial_revision_history_cache.pop() | ||
| 36 | 149 | |||
| 37 | 128 | @staticmethod | 150 | @staticmethod |
| 38 | 129 | def open(base, _unsupported=False, possible_transports=None): | 151 | def open(base, _unsupported=False, possible_transports=None): |
| 39 | 130 | """Open the branch rooted at base. | 152 | """Open the branch rooted at base. |
| 40 | @@ -698,6 +720,8 @@ | |||
| 41 | 698 | self._revision_id_to_revno_cache = None | 720 | self._revision_id_to_revno_cache = None |
| 42 | 699 | self._last_revision_info_cache = None | 721 | self._last_revision_info_cache = None |
| 43 | 700 | self._merge_sorted_revisions_cache = None | 722 | self._merge_sorted_revisions_cache = None |
| 44 | 723 | self._partial_revision_history_cache = [] | ||
| 45 | 724 | self._partial_revision_id_to_revno_cache = {} | ||
| 46 | 701 | 725 | ||
| 47 | 702 | def _gen_revision_history(self): | 726 | def _gen_revision_history(self): |
| 48 | 703 | """Return sequence of revision hashes on to this branch. | 727 | """Return sequence of revision hashes on to this branch. |
| 49 | @@ -831,15 +855,20 @@ | |||
| 50 | 831 | except ValueError: | 855 | except ValueError: |
| 51 | 832 | raise errors.NoSuchRevision(self, revision_id) | 856 | raise errors.NoSuchRevision(self, revision_id) |
| 52 | 833 | 857 | ||
| 53 | 858 | @needs_read_lock | ||
| 54 | 834 | def get_rev_id(self, revno, history=None): | 859 | def get_rev_id(self, revno, history=None): |
| 55 | 835 | """Find the revision id of the specified revno.""" | 860 | """Find the revision id of the specified revno.""" |
| 56 | 836 | if revno == 0: | 861 | if revno == 0: |
| 57 | 837 | return _mod_revision.NULL_REVISION | 862 | return _mod_revision.NULL_REVISION |
| 61 | 838 | if history is None: | 863 | last_revno, last_revid = self.last_revision_info() |
| 62 | 839 | history = self.revision_history() | 864 | if revno == last_revno: |
| 63 | 840 | if revno <= 0 or revno > len(history): | 865 | return last_revid |
| 64 | 866 | if revno <= 0 or revno > last_revno: | ||
| 65 | 841 | raise errors.NoSuchRevision(self, revno) | 867 | raise errors.NoSuchRevision(self, revno) |
| 67 | 842 | return history[revno - 1] | 868 | distance_from_last = last_revno - revno |
| 68 | 869 | if len(self._partial_revision_history_cache) <= distance_from_last: | ||
| 69 | 870 | self._extend_partial_history(revno) | ||
| 70 | 871 | return self._partial_revision_history_cache[revno] | ||
| 71 | 843 | 872 | ||
| 72 | 844 | @needs_write_lock | 873 | @needs_write_lock |
| 73 | 845 | def pull(self, source, overwrite=False, stop_revision=None, | 874 | def pull(self, source, overwrite=False, stop_revision=None, |
| 74 | @@ -2376,13 +2405,11 @@ | |||
| 75 | 2376 | self._ignore_fallbacks = kwargs.get('ignore_fallbacks', False) | 2405 | self._ignore_fallbacks = kwargs.get('ignore_fallbacks', False) |
| 76 | 2377 | super(BzrBranch8, self).__init__(*args, **kwargs) | 2406 | super(BzrBranch8, self).__init__(*args, **kwargs) |
| 77 | 2378 | self._last_revision_info_cache = None | 2407 | self._last_revision_info_cache = None |
| 78 | 2379 | self._partial_revision_history_cache = [] | ||
| 79 | 2380 | self._reference_info = None | 2408 | self._reference_info = None |
| 80 | 2381 | 2409 | ||
| 81 | 2382 | def _clear_cached_state(self): | 2410 | def _clear_cached_state(self): |
| 82 | 2383 | super(BzrBranch8, self)._clear_cached_state() | 2411 | super(BzrBranch8, self)._clear_cached_state() |
| 83 | 2384 | self._last_revision_info_cache = None | 2412 | self._last_revision_info_cache = None |
| 84 | 2385 | self._partial_revision_history_cache = [] | ||
| 85 | 2386 | self._reference_info = None | 2413 | self._reference_info = None |
| 86 | 2387 | 2414 | ||
| 87 | 2388 | def _last_revision_info(self): | 2415 | def _last_revision_info(self): |
| 88 | @@ -2444,35 +2471,6 @@ | |||
| 89 | 2444 | self._extend_partial_history(stop_index=last_revno-1) | 2471 | self._extend_partial_history(stop_index=last_revno-1) |
| 90 | 2445 | return list(reversed(self._partial_revision_history_cache)) | 2472 | return list(reversed(self._partial_revision_history_cache)) |
| 91 | 2446 | 2473 | ||
| 92 | 2447 | def _extend_partial_history(self, stop_index=None, stop_revision=None): | ||
| 93 | 2448 | """Extend the partial history to include a given index | ||
| 94 | 2449 | |||
| 95 | 2450 | If a stop_index is supplied, stop when that index has been reached. | ||
| 96 | 2451 | If a stop_revision is supplied, stop when that revision is | ||
| 97 | 2452 | encountered. Otherwise, stop when the beginning of history is | ||
| 98 | 2453 | reached. | ||
| 99 | 2454 | |||
| 100 | 2455 | :param stop_index: The index which should be present. When it is | ||
| 101 | 2456 | present, history extension will stop. | ||
| 102 | 2457 | :param revision_id: The revision id which should be present. When | ||
| 103 | 2458 | it is encountered, history extension will stop. | ||
| 104 | 2459 | """ | ||
| 105 | 2460 | repo = self.repository | ||
| 106 | 2461 | if len(self._partial_revision_history_cache) == 0: | ||
| 107 | 2462 | iterator = repo.iter_reverse_revision_history(self.last_revision()) | ||
| 108 | 2463 | else: | ||
| 109 | 2464 | start_revision = self._partial_revision_history_cache[-1] | ||
| 110 | 2465 | iterator = repo.iter_reverse_revision_history(start_revision) | ||
| 111 | 2466 | #skip the last revision in the list | ||
| 112 | 2467 | next_revision = iterator.next() | ||
| 113 | 2468 | for revision_id in iterator: | ||
| 114 | 2469 | self._partial_revision_history_cache.append(revision_id) | ||
| 115 | 2470 | if (stop_index is not None and | ||
| 116 | 2471 | len(self._partial_revision_history_cache) > stop_index): | ||
| 117 | 2472 | break | ||
| 118 | 2473 | if revision_id == stop_revision: | ||
| 119 | 2474 | break | ||
| 120 | 2475 | |||
| 121 | 2476 | def _write_revision_history(self, history): | 2474 | def _write_revision_history(self, history): |
| 122 | 2477 | """Factored out of set_revision_history. | 2475 | """Factored out of set_revision_history. |
| 123 | 2478 | 2476 | ||
| 124 | 2479 | 2477 | ||
| 125 | === modified file 'bzrlib/builtins.py' | |||
| 126 | --- bzrlib/builtins.py 2009-06-11 06:54:33 +0000 | |||
| 127 | +++ bzrlib/builtins.py 2009-06-11 19:35:21 +0000 | |||
| 128 | @@ -946,29 +946,36 @@ | |||
| 129 | 946 | if branch_to.get_parent() is None or remember: | 946 | if branch_to.get_parent() is None or remember: |
| 130 | 947 | branch_to.set_parent(branch_from.base) | 947 | branch_to.set_parent(branch_from.base) |
| 131 | 948 | 948 | ||
| 136 | 949 | if revision is not None: | 949 | if branch_from is not branch_to: |
| 137 | 950 | revision_id = revision.as_revision_id(branch_from) | 950 | branch_from.lock_read() |
| 134 | 951 | |||
| 135 | 952 | branch_to.lock_write() | ||
| 138 | 953 | try: | 951 | try: |
| 155 | 954 | if tree_to is not None: | 952 | if revision is not None: |
| 156 | 955 | view_info = _get_view_info_for_change_reporter(tree_to) | 953 | revision_id = revision.as_revision_id(branch_from) |
| 157 | 956 | change_reporter = delta._ChangeReporter( | 954 | |
| 158 | 957 | unversioned_filter=tree_to.is_ignored, view_info=view_info) | 955 | branch_to.lock_write() |
| 159 | 958 | result = tree_to.pull(branch_from, overwrite, revision_id, | 956 | try: |
| 160 | 959 | change_reporter, | 957 | if tree_to is not None: |
| 161 | 960 | possible_transports=possible_transports, | 958 | view_info = _get_view_info_for_change_reporter(tree_to) |
| 162 | 961 | local=local) | 959 | change_reporter = delta._ChangeReporter( |
| 163 | 962 | else: | 960 | unversioned_filter=tree_to.is_ignored, |
| 164 | 963 | result = branch_to.pull(branch_from, overwrite, revision_id, | 961 | view_info=view_info) |
| 165 | 964 | local=local) | 962 | result = tree_to.pull( |
| 166 | 965 | 963 | branch_from, overwrite, revision_id, change_reporter, | |
| 167 | 966 | result.report(self.outf) | 964 | possible_transports=possible_transports, local=local) |
| 168 | 967 | if verbose and result.old_revid != result.new_revid: | 965 | else: |
| 169 | 968 | log.show_branch_change(branch_to, self.outf, result.old_revno, | 966 | result = branch_to.pull( |
| 170 | 969 | result.old_revid) | 967 | branch_from, overwrite, revision_id, local=local) |
| 171 | 968 | |||
| 172 | 969 | result.report(self.outf) | ||
| 173 | 970 | if verbose and result.old_revid != result.new_revid: | ||
| 174 | 971 | log.show_branch_change( | ||
| 175 | 972 | branch_to, self.outf, result.old_revno, | ||
| 176 | 973 | result.old_revid) | ||
| 177 | 974 | finally: | ||
| 178 | 975 | branch_to.unlock() | ||
| 179 | 970 | finally: | 976 | finally: |
| 181 | 971 | branch_to.unlock() | 977 | if branch_from is not branch_to: |
| 182 | 978 | branch_from.unlock() | ||
| 183 | 972 | 979 | ||
| 184 | 973 | 980 | ||
| 185 | 974 | class cmd_push(Command): | 981 | class cmd_push(Command): |
| 186 | 975 | 982 | ||
| 187 | === modified file 'bzrlib/remote.py' | |||
| 188 | --- bzrlib/remote.py 2009-06-11 09:11:21 +0000 | |||
| 189 | +++ bzrlib/remote.py 2009-06-11 19:35:22 +0000 | |||
| 190 | @@ -28,12 +28,10 @@ | |||
| 191 | 28 | errors, | 28 | errors, |
| 192 | 29 | graph, | 29 | graph, |
| 193 | 30 | lockdir, | 30 | lockdir, |
| 194 | 31 | pack, | ||
| 195 | 32 | repository, | 31 | repository, |
| 196 | 33 | revision, | 32 | revision, |
| 197 | 34 | revision as _mod_revision, | 33 | revision as _mod_revision, |
| 198 | 35 | symbol_versioning, | 34 | symbol_versioning, |
| 199 | 36 | urlutils, | ||
| 200 | 37 | ) | 35 | ) |
| 201 | 38 | from bzrlib.branch import BranchReferenceFormat | 36 | from bzrlib.branch import BranchReferenceFormat |
| 202 | 39 | from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat | 37 | from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat |
| 203 | @@ -675,6 +673,24 @@ | |||
| 204 | 675 | return self._real_repository.get_missing_parent_inventories( | 673 | return self._real_repository.get_missing_parent_inventories( |
| 205 | 676 | check_for_missing_texts=check_for_missing_texts) | 674 | check_for_missing_texts=check_for_missing_texts) |
| 206 | 677 | 675 | ||
| 207 | 676 | def get_rev_id_for_revno(self, revno, known_pair): | ||
| 208 | 677 | """See Repository.get_rev_id_for_revno.""" | ||
| 209 | 678 | path = self.bzrdir._path_for_remote_call(self._client) | ||
| 210 | 679 | try: | ||
| 211 | 680 | response = self._call( | ||
| 212 | 681 | 'Repository.get_rev_id_for_revno', path, revno, known_pair) | ||
| 213 | 682 | except errors.UnknownSmartMethod: | ||
| 214 | 683 | self._client.medium._remember_remote_is_before((1, 16)) | ||
| 215 | 684 | self._ensure_real() | ||
| 216 | 685 | return self._real_repository.get_rev_id_for_revno( | ||
| 217 | 686 | revno, known_pair) | ||
| 218 | 687 | if response[0] == 'ok': | ||
| 219 | 688 | return True, response[1] | ||
| 220 | 689 | elif response[0] == 'history-incomplete': | ||
| 221 | 690 | return False, response[1:3] | ||
| 222 | 691 | else: | ||
| 223 | 692 | raise errors.UnexpectedSmartServerResponse(response) | ||
| 224 | 693 | |||
| 225 | 678 | def _ensure_real(self): | 694 | def _ensure_real(self): |
| 226 | 679 | """Ensure that there is a _real_repository set. | 695 | """Ensure that there is a _real_repository set. |
| 227 | 680 | 696 | ||
| 228 | @@ -1919,11 +1935,6 @@ | |||
| 229 | 1919 | # We intentionally don't call the parent class's __init__, because it | 1935 | # We intentionally don't call the parent class's __init__, because it |
| 230 | 1920 | # will try to assign to self.tags, which is a property in this subclass. | 1936 | # will try to assign to self.tags, which is a property in this subclass. |
| 231 | 1921 | # And the parent's __init__ doesn't do much anyway. | 1937 | # And the parent's __init__ doesn't do much anyway. |
| 232 | 1922 | self._revision_id_to_revno_cache = None | ||
| 233 | 1923 | self._partial_revision_id_to_revno_cache = {} | ||
| 234 | 1924 | self._revision_history_cache = None | ||
| 235 | 1925 | self._last_revision_info_cache = None | ||
| 236 | 1926 | self._merge_sorted_revisions_cache = None | ||
| 237 | 1927 | self.bzrdir = remote_bzrdir | 1938 | self.bzrdir = remote_bzrdir |
| 238 | 1928 | if _client is not None: | 1939 | if _client is not None: |
| 239 | 1929 | self._client = _client | 1940 | self._client = _client |
| 240 | @@ -1943,6 +1954,7 @@ | |||
| 241 | 1943 | else: | 1954 | else: |
| 242 | 1944 | self._real_branch = None | 1955 | self._real_branch = None |
| 243 | 1945 | # Fill out expected attributes of branch for bzrlib API users. | 1956 | # Fill out expected attributes of branch for bzrlib API users. |
| 244 | 1957 | self._clear_cached_state() | ||
| 245 | 1946 | self.base = self.bzrdir.root_transport.base | 1958 | self.base = self.bzrdir.root_transport.base |
| 246 | 1947 | self._control_files = None | 1959 | self._control_files = None |
| 247 | 1948 | self._lock_mode = None | 1960 | self._lock_mode = None |
| 248 | @@ -2229,6 +2241,17 @@ | |||
| 249 | 2229 | raise NotImplementedError(self.dont_leave_lock_in_place) | 2241 | raise NotImplementedError(self.dont_leave_lock_in_place) |
| 250 | 2230 | self._leave_lock = False | 2242 | self._leave_lock = False |
| 251 | 2231 | 2243 | ||
| 252 | 2244 | def get_rev_id(self, revno, history=None): | ||
| 253 | 2245 | last_revision_info = self.last_revision_info() | ||
| 254 | 2246 | result = last_revision_info | ||
| 255 | 2247 | for repo in [self.repository] + self.repository._fallback_repositories: | ||
| 256 | 2248 | ok, result = repo.get_rev_id_for_revno(revno, result) | ||
| 257 | 2249 | if ok: | ||
| 258 | 2250 | return result | ||
| 259 | 2251 | closest = result[1] | ||
| 260 | 2252 | missing_parent = self.repository.get_parent_map([closest])[closest][0] | ||
| 261 | 2253 | raise errors.RevisionNotPresent(missing_parent, self.repository) | ||
| 262 | 2254 | |||
| 263 | 2232 | def _last_revision_info(self): | 2255 | def _last_revision_info(self): |
| 264 | 2233 | response = self._call('Branch.last_revision_info', self._remote_path()) | 2256 | response = self._call('Branch.last_revision_info', self._remote_path()) |
| 265 | 2234 | if response[0] != 'ok': | 2257 | if response[0] != 'ok': |
| 266 | 2235 | 2258 | ||
| 267 | === modified file 'bzrlib/repository.py' | |||
| 268 | --- bzrlib/repository.py 2009-06-10 03:56:49 +0000 | |||
| 269 | +++ bzrlib/repository.py 2009-06-11 19:35:22 +0000 | |||
| 270 | @@ -2234,6 +2234,41 @@ | |||
| 271 | 2234 | """ | 2234 | """ |
| 272 | 2235 | return self.get_revision(revision_id).inventory_sha1 | 2235 | return self.get_revision(revision_id).inventory_sha1 |
| 273 | 2236 | 2236 | ||
| 274 | 2237 | def get_rev_id_for_revno(self, revno, known_pair): | ||
| 275 | 2238 | """Return the revision id of a revno, given a later (revno, revid) | ||
| 276 | 2239 | pair in the same history. | ||
| 277 | 2240 | |||
| 278 | 2241 | :return: if found (True, revid). If the available history ran out | ||
| 279 | 2242 | before reaching the revno, then this returns | ||
| 280 | 2243 | (False, (closest_revno, closest_revid)). | ||
| 281 | 2244 | """ | ||
| 282 | 2245 | known_revno, known_revid = known_pair | ||
| 283 | 2246 | partial_history = [known_revid] | ||
| 284 | 2247 | distance_from_known = known_revno - revno | ||
| 285 | 2248 | if distance_from_known < 0: | ||
| 286 | 2249 | raise ValueError( | ||
| 287 | 2250 | 'requested revno (%d) is later than given known revno (%d)' | ||
| 288 | 2251 | % (revno, known_revno)) | ||
| 289 | 2252 | # XXX: handle distance_from_known < 0, etc | ||
| 290 | 2253 | try: | ||
| 291 | 2254 | _iter_for_revno(self, partial_history, stop_index=revno) | ||
| 292 | 2255 | except errors.RevisionNotPresent, err: | ||
| 293 | 2256 | if err.revision_id == known_revid: | ||
| 294 | 2257 | # The start revision (known_revid) wasn't found. | ||
| 295 | 2258 | raise | ||
| 296 | 2259 | # This is a stacked repository with no fallbacks, or a there's a | ||
| 297 | 2260 | # left-hand ghost. Either way, even though the revision named in | ||
| 298 | 2261 | # the error isn't in this repo, we know it's the next step in this | ||
| 299 | 2262 | # left-hand history. | ||
| 300 | 2263 | partial_history.append(err.revision_id) | ||
| 301 | 2264 | if len(partial_history) <= distance_from_known: | ||
| 302 | 2265 | # Didn't find enough history to get a revid for the revno. | ||
| 303 | 2266 | earliest_revno = known_revno - len(partial_history) + 1 | ||
| 304 | 2267 | return (False, (earliest_revno, partial_history[-1])) | ||
| 305 | 2268 | if len(partial_history) - 1 > distance_from_known: | ||
| 306 | 2269 | raise AssertionError('_iter_for_revno returned too much history') | ||
| 307 | 2270 | return (True, partial_history[-1]) | ||
| 308 | 2271 | |||
| 309 | 2237 | def iter_reverse_revision_history(self, revision_id): | 2272 | def iter_reverse_revision_history(self, revision_id): |
| 310 | 2238 | """Iterate backwards through revision ids in the lefthand history | 2273 | """Iterate backwards through revision ids in the lefthand history |
| 311 | 2239 | 2274 | ||
| 312 | @@ -4412,3 +4447,35 @@ | |||
| 313 | 4412 | yield versionedfile.FulltextContentFactory( | 4447 | yield versionedfile.FulltextContentFactory( |
| 314 | 4413 | key, parent_keys, None, as_bytes) | 4448 | key, parent_keys, None, as_bytes) |
| 315 | 4414 | 4449 | ||
| 316 | 4450 | |||
| 317 | 4451 | def _iter_for_revno(repo, partial_history_cache, stop_index=None, | ||
| 318 | 4452 | stop_revision=None): | ||
| 319 | 4453 | """Extend the partial history to include a given index | ||
| 320 | 4454 | |||
| 321 | 4455 | If a stop_index is supplied, stop when that index has been reached. | ||
| 322 | 4456 | If a stop_revision is supplied, stop when that revision is | ||
| 323 | 4457 | encountered. Otherwise, stop when the beginning of history is | ||
| 324 | 4458 | reached. | ||
| 325 | 4459 | |||
| 326 | 4460 | :param stop_index: The index which should be present. When it is | ||
| 327 | 4461 | present, history extension will stop. | ||
| 328 | 4462 | :param stop_revision: The revision id which should be present. When | ||
| 329 | 4463 | it is encountered, history extension will stop. | ||
| 330 | 4464 | """ | ||
| 331 | 4465 | start_revision = partial_history_cache[-1] | ||
| 332 | 4466 | iterator = repo.iter_reverse_revision_history(start_revision) | ||
| 333 | 4467 | try: | ||
| 334 | 4468 | #skip the last revision in the list | ||
| 335 | 4469 | next_revision = iterator.next() | ||
| 336 | 4470 | while True: | ||
| 337 | 4471 | if (stop_index is not None and | ||
| 338 | 4472 | len(partial_history_cache) > stop_index): | ||
| 339 | 4473 | break | ||
| 340 | 4474 | revision_id = iterator.next() | ||
| 341 | 4475 | partial_history_cache.append(revision_id) | ||
| 342 | 4476 | if revision_id == stop_revision: | ||
| 343 | 4477 | break | ||
| 344 | 4478 | except StopIteration: | ||
| 345 | 4479 | # No more history | ||
| 346 | 4480 | return | ||
| 347 | 4481 | |||
| 348 | 4415 | 4482 | ||
| 349 | === modified file 'bzrlib/smart/repository.py' | |||
| 350 | --- bzrlib/smart/repository.py 2009-06-10 03:56:49 +0000 | |||
| 351 | +++ bzrlib/smart/repository.py 2009-06-11 19:35:22 +0000 | |||
| 352 | @@ -19,7 +19,6 @@ | |||
| 353 | 19 | import bz2 | 19 | import bz2 |
| 354 | 20 | import os | 20 | import os |
| 355 | 21 | import Queue | 21 | import Queue |
| 356 | 22 | import struct | ||
| 357 | 23 | import sys | 22 | import sys |
| 358 | 24 | import tarfile | 23 | import tarfile |
| 359 | 25 | import tempfile | 24 | import tempfile |
| 360 | @@ -284,6 +283,31 @@ | |||
| 361 | 284 | return SuccessfulSmartServerResponse(('ok', ), '\n'.join(lines)) | 283 | return SuccessfulSmartServerResponse(('ok', ), '\n'.join(lines)) |
| 362 | 285 | 284 | ||
| 363 | 286 | 285 | ||
| 364 | 286 | class SmartServerRepositoryGetRevIdForRevno(SmartServerRepositoryReadLocked): | ||
| 365 | 287 | |||
| 366 | 288 | def do_readlocked_repository_request(self, repository, revno, | ||
| 367 | 289 | known_pair): | ||
| 368 | 290 | """Find the revid for a given revno, given a known revno/revid pair. | ||
| 369 | 291 | |||
| 370 | 292 | New in 1.16. | ||
| 371 | 293 | """ | ||
| 372 | 294 | try: | ||
| 373 | 295 | found_flag, result = repository.get_rev_id_for_revno(revno, known_pair) | ||
| 374 | 296 | except errors.RevisionNotPresent, err: | ||
| 375 | 297 | if err.revision_id != known_pair[1]: | ||
| 376 | 298 | raise AssertionError( | ||
| 377 | 299 | 'get_rev_id_for_revno raised RevisionNotPresent for ' | ||
| 378 | 300 | 'non-initial revision: ' + err.revision_id) | ||
| 379 | 301 | return FailedSmartServerResponse( | ||
| 380 | 302 | ('nosuchrevision', err.revision_id)) | ||
| 381 | 303 | if found_flag: | ||
| 382 | 304 | return SuccessfulSmartServerResponse(('ok', result)) | ||
| 383 | 305 | else: | ||
| 384 | 306 | earliest_revno, earliest_revid = result | ||
| 385 | 307 | return SuccessfulSmartServerResponse( | ||
| 386 | 308 | ('history-incomplete', earliest_revno, earliest_revid)) | ||
| 387 | 309 | |||
| 388 | 310 | |||
| 389 | 287 | class SmartServerRequestHasRevision(SmartServerRepositoryRequest): | 311 | class SmartServerRequestHasRevision(SmartServerRepositoryRequest): |
| 390 | 288 | 312 | ||
| 391 | 289 | def do_repository_request(self, repository, revision_id): | 313 | def do_repository_request(self, repository, revision_id): |
| 392 | 290 | 314 | ||
| 393 | === modified file 'bzrlib/smart/request.py' | |||
| 394 | --- bzrlib/smart/request.py 2009-04-24 05:08:51 +0000 | |||
| 395 | +++ bzrlib/smart/request.py 2009-06-11 19:35:22 +0000 | |||
| 396 | @@ -555,6 +555,9 @@ | |||
| 397 | 555 | request_handlers.register_lazy( | 555 | request_handlers.register_lazy( |
| 398 | 556 | 'Repository.unlock', 'bzrlib.smart.repository', 'SmartServerRepositoryUnlock') | 556 | 'Repository.unlock', 'bzrlib.smart.repository', 'SmartServerRepositoryUnlock') |
| 399 | 557 | request_handlers.register_lazy( | 557 | request_handlers.register_lazy( |
| 400 | 558 | 'Repository.get_rev_id_for_revno', 'bzrlib.smart.repository', | ||
| 401 | 559 | 'SmartServerRepositoryGetRevIdForRevno') | ||
| 402 | 560 | request_handlers.register_lazy( | ||
| 403 | 558 | 'Repository.get_stream', 'bzrlib.smart.repository', | 561 | 'Repository.get_stream', 'bzrlib.smart.repository', |
| 404 | 559 | 'SmartServerRepositoryGetStream') | 562 | 'SmartServerRepositoryGetStream') |
| 405 | 560 | request_handlers.register_lazy( | 563 | request_handlers.register_lazy( |
| 406 | 561 | 564 | ||
| 407 | === modified file 'bzrlib/tests/blackbox/test_pull.py' | |||
| 408 | --- bzrlib/tests/blackbox/test_pull.py 2009-06-11 07:43:56 +0000 | |||
| 409 | +++ bzrlib/tests/blackbox/test_pull.py 2009-06-11 19:35:22 +0000 | |||
| 410 | @@ -368,8 +368,12 @@ | |||
| 411 | 368 | See <https://launchpad.net/bugs/380314> | 368 | See <https://launchpad.net/bugs/380314> |
| 412 | 369 | """ | 369 | """ |
| 413 | 370 | self.setup_smart_server_with_call_log() | 370 | self.setup_smart_server_with_call_log() |
| 414 | 371 | # Make a stacked-on branch with two commits so that the | ||
| 415 | 372 | # revision-history can't be determined just by looking at the parent | ||
| 416 | 373 | # field in the revision in the stacked repo. | ||
| 417 | 371 | parent = self.make_branch_and_tree('parent', format='1.9') | 374 | parent = self.make_branch_and_tree('parent', format='1.9') |
| 418 | 372 | parent.commit(message='first commit') | 375 | parent.commit(message='first commit') |
| 419 | 376 | parent.commit(message='second commit') | ||
| 420 | 373 | local = parent.bzrdir.sprout('local').open_workingtree() | 377 | local = parent.bzrdir.sprout('local').open_workingtree() |
| 421 | 374 | local.commit(message='local commit') | 378 | local.commit(message='local commit') |
| 422 | 375 | local.branch.create_clone_on_transport( | 379 | local.branch.create_clone_on_transport( |
| 423 | @@ -383,7 +387,7 @@ | |||
| 424 | 383 | # being too low. If rpc_count increases, more network roundtrips have | 387 | # being too low. If rpc_count increases, more network roundtrips have |
| 425 | 384 | # become necessary for this use case. Please do not adjust this number | 388 | # become necessary for this use case. Please do not adjust this number |
| 426 | 385 | # upwards without agreement from bzr's network support maintainers. | 389 | # upwards without agreement from bzr's network support maintainers. |
| 428 | 386 | self.assertLength(43, self.hpss_calls) | 390 | self.assertLength(18, self.hpss_calls) |
| 429 | 387 | remote = Branch.open('stacked') | 391 | remote = Branch.open('stacked') |
| 430 | 388 | self.assertEndsWith(remote.get_stacked_on_url(), '/parent') | 392 | self.assertEndsWith(remote.get_stacked_on_url(), '/parent') |
| 431 | 389 | 393 | ||
| 432 | 390 | 394 | ||
| 433 | === modified file 'bzrlib/tests/test_smart.py' | |||
| 434 | --- bzrlib/tests/test_smart.py 2009-06-11 07:10:44 +0000 | |||
| 435 | +++ bzrlib/tests/test_smart.py 2009-06-11 19:35:22 +0000 | |||
| 436 | @@ -1161,6 +1161,47 @@ | |||
| 437 | 1161 | request.execute('', 'missingrevision')) | 1161 | request.execute('', 'missingrevision')) |
| 438 | 1162 | 1162 | ||
| 439 | 1163 | 1163 | ||
| 440 | 1164 | class TestSmartServerRepositoryGetRevIdForRevno(tests.TestCaseWithMemoryTransport): | ||
| 441 | 1165 | |||
| 442 | 1166 | def test_revno_found(self): | ||
| 443 | 1167 | backing = self.get_transport() | ||
| 444 | 1168 | request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing) | ||
| 445 | 1169 | tree = self.make_branch_and_memory_tree('.') | ||
| 446 | 1170 | tree.lock_write() | ||
| 447 | 1171 | tree.add('') | ||
| 448 | 1172 | rev1_id_utf8 = u'\xc8'.encode('utf-8') | ||
| 449 | 1173 | rev2_id_utf8 = u'\xc9'.encode('utf-8') | ||
| 450 | 1174 | tree.commit('1st commit', rev_id=rev1_id_utf8) | ||
| 451 | 1175 | tree.commit('2nd commit', rev_id=rev2_id_utf8) | ||
| 452 | 1176 | tree.unlock() | ||
| 453 | 1177 | |||
| 454 | 1178 | self.assertEqual(SmartServerResponse(('ok', rev1_id_utf8)), | ||
| 455 | 1179 | request.execute('', 1, (2, rev2_id_utf8))) | ||
| 456 | 1180 | |||
| 457 | 1181 | def test_known_revid_missing(self): | ||
| 458 | 1182 | backing = self.get_transport() | ||
| 459 | 1183 | request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing) | ||
| 460 | 1184 | repo = self.make_repository('.') | ||
| 461 | 1185 | self.assertEqual( | ||
| 462 | 1186 | FailedSmartServerResponse(('nosuchrevision', 'ghost')), | ||
| 463 | 1187 | request.execute('', 1, (2, 'ghost'))) | ||
| 464 | 1188 | |||
| 465 | 1189 | def test_history_incomplete(self): | ||
| 466 | 1190 | backing = self.get_transport() | ||
| 467 | 1191 | request = smart.repository.SmartServerRepositoryGetRevIdForRevno(backing) | ||
| 468 | 1192 | parent = self.make_branch_and_memory_tree('parent', format='1.9') | ||
| 469 | 1193 | r1 = parent.commit(message='first commit') | ||
| 470 | 1194 | r2 = parent.commit(message='second commit') | ||
| 471 | 1195 | local = self.make_branch_and_memory_tree('local', format='1.9') | ||
| 472 | 1196 | local.branch.pull(parent.branch) | ||
| 473 | 1197 | local.set_parent_ids([r2]) | ||
| 474 | 1198 | r3 = local.commit(message='local commit') | ||
| 475 | 1199 | local.branch.create_clone_on_transport( | ||
| 476 | 1200 | self.get_transport('stacked'), stacked_on=self.get_url('parent')) | ||
| 477 | 1201 | self.assertEqual( | ||
| 478 | 1202 | SmartServerResponse(('history-incomplete', 2, r2)), | ||
| 479 | 1203 | request.execute('stacked', 1, (3, r3))) | ||
| 480 | 1204 | |||
| 481 | 1164 | class TestSmartServerRepositoryGetStream(tests.TestCaseWithMemoryTransport): | 1205 | class TestSmartServerRepositoryGetStream(tests.TestCaseWithMemoryTransport): |
| 482 | 1165 | 1206 | ||
| 483 | 1166 | def make_two_commit_repo(self): | 1207 | def make_two_commit_repo(self): |
| 484 | @@ -1576,6 +1617,8 @@ | |||
| 485 | 1576 | smart.repository.SmartServerRepositoryGatherStats) | 1617 | smart.repository.SmartServerRepositoryGatherStats) |
| 486 | 1577 | self.assertHandlerEqual('Repository.get_parent_map', | 1618 | self.assertHandlerEqual('Repository.get_parent_map', |
| 487 | 1578 | smart.repository.SmartServerRepositoryGetParentMap) | 1619 | smart.repository.SmartServerRepositoryGetParentMap) |
| 488 | 1620 | self.assertHandlerEqual('Repository.get_rev_id_for_revno', | ||
| 489 | 1621 | smart.repository.SmartServerRepositoryGetRevIdForRevno) | ||
| 490 | 1579 | self.assertHandlerEqual('Repository.get_revision_graph', | 1622 | self.assertHandlerEqual('Repository.get_revision_graph', |
| 491 | 1580 | smart.repository.SmartServerRepositoryGetRevisionGraph) | 1623 | smart.repository.SmartServerRepositoryGetRevisionGraph) |
| 492 | 1581 | self.assertHandlerEqual('Repository.get_stream', | 1624 | self.assertHandlerEqual('Repository.get_stream', |
This fixes bug 380314. It is a cheap fix that simply falls back to VFS for
RemoteBranch.
for both the blackbox case of "'bzr pull -r 123' from stacked" and a per-branch
test for .revision_history when there is stacking.
I have a followup patch in the works that defines a new RPC that avoids VFS
entirely in this case, but it's close to the release and getting the correctness
fix merged is more urgent.