Merge lp:~abentley/launchpad/partial-ancestry-scanner into lp:launchpad
- partial-ancestry-scanner
- Merge into devel
Proposed by
Aaron Bentley
| Status: | Merged |
|---|---|
| Approved by: | Paul Hummer |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 11757 |
| Proposed branch: | lp:~abentley/launchpad/partial-ancestry-scanner |
| Merge into: | lp:launchpad |
| Diff against target: |
553 lines (+193/-117) 7 files modified
lib/lp/codehosting/bzrutils.py (+12/-0) lib/lp/codehosting/scanner/bzrsync.py (+84/-57) lib/lp/codehosting/scanner/events.py (+3/-2) lib/lp/codehosting/scanner/mergedetection.py (+4/-4) lib/lp/codehosting/scanner/tests/test_bzrsync.py (+83/-33) lib/lp/codehosting/scanner/tests/test_mergedetection.py (+3/-3) lib/lp/testing/factory.py (+4/-18) |
| To merge this branch: | bzr merge lp:~abentley/launchpad/partial-ancestry-scanner |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Paul Hummer (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Stop loading the entire ancestry of a branch during the scan
Description of the change
= Summary =
Fix bug #638637: Stop loading the entire ancestry of a branch during the scan
== Proposed fix ==
Use the set of revisions added to the ancestry, in most cases.
== Pre-implementation notes ==
Pre- and mid- implementation with thumper.
== Implementation details ==
I took the opportunity to refactor how things work a lot. Removing
whole-ancestry traversal made retrieveBranchD
with Branch.
I also replaced BzrSync.
can use a partial ancestry and last-revno to generate the required info.
I also factored code out, e.g. getHistoryDelta.
Along the way, I added a write_locked Context Manager and added parent_ids to
makeBranchRevision so I could test getAncestryDelta.
_getRevisionGraph is designed to handle the case where not only has the tip
changed, but the old tip isn't present in the new repository. This requires
generating a Graph using the Revisions associated with the branch. However,
it's an extremely rare case, so I didn't optimize it.
== Tests ==
bin/test -vv scanner
== Demo and Q/A ==
None.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
374: E231 missing whitespace after ','
121: Line exceeds 78 characters.
To post a comment you must log in.
Revision history for this message
| Paul Hummer (rockstar) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'lib/lp/codehosting/bzrutils.py' | |||
| 2 | --- lib/lp/codehosting/bzrutils.py 2010-09-21 14:39:59 +0000 | |||
| 3 | +++ lib/lp/codehosting/bzrutils.py 2010-10-19 20:46:20 +0000 | |||
| 4 | @@ -162,6 +162,7 @@ | |||
| 5 | 162 | 162 | ||
| 6 | 163 | def make_oops_logging_exception_hook(error_utility, request): | 163 | def make_oops_logging_exception_hook(error_utility, request): |
| 7 | 164 | """Make a hook for logging OOPSes.""" | 164 | """Make a hook for logging OOPSes.""" |
| 8 | 165 | |||
| 9 | 165 | def log_oops(): | 166 | def log_oops(): |
| 10 | 166 | error_utility.raising(sys.exc_info(), request) | 167 | error_utility.raising(sys.exc_info(), request) |
| 11 | 167 | return log_oops | 168 | return log_oops |
| 12 | @@ -340,6 +341,7 @@ | |||
| 13 | 340 | 341 | ||
| 14 | 341 | def makeURLChecker(allowed_scheme): | 342 | def makeURLChecker(allowed_scheme): |
| 15 | 342 | """Make a callable that rejects URLs not on the given scheme.""" | 343 | """Make a callable that rejects URLs not on the given scheme.""" |
| 16 | 344 | |||
| 17 | 343 | def checkURL(url): | 345 | def checkURL(url): |
| 18 | 344 | """Check that `url` is safe to open.""" | 346 | """Check that `url` is safe to open.""" |
| 19 | 345 | if URI(url).scheme != allowed_scheme: | 347 | if URI(url).scheme != allowed_scheme: |
| 20 | @@ -374,3 +376,13 @@ | |||
| 21 | 374 | yield | 376 | yield |
| 22 | 375 | finally: | 377 | finally: |
| 23 | 376 | branch.unlock() | 378 | branch.unlock() |
| 24 | 379 | |||
| 25 | 380 | |||
| 26 | 381 | @contextmanager | ||
| 27 | 382 | def write_locked(branch): | ||
| 28 | 383 | """Provide a context in which the branch is write-locked.""" | ||
| 29 | 384 | branch.lock_write() | ||
| 30 | 385 | try: | ||
| 31 | 386 | yield | ||
| 32 | 387 | finally: | ||
| 33 | 388 | branch.unlock() | ||
| 34 | 377 | 389 | ||
| 35 | === modified file 'lib/lp/codehosting/scanner/bzrsync.py' | |||
| 36 | --- lib/lp/codehosting/scanner/bzrsync.py 2010-09-09 02:26:42 +0000 | |||
| 37 | +++ lib/lp/codehosting/scanner/bzrsync.py 2010-10-19 20:46:20 +0000 | |||
| 38 | @@ -16,8 +16,11 @@ | |||
| 39 | 16 | 16 | ||
| 40 | 17 | import logging | 17 | import logging |
| 41 | 18 | 18 | ||
| 42 | 19 | from bzrlib.graph import DictParentsProvider | ||
| 43 | 20 | from bzrlib.revision import NULL_REVISION | ||
| 44 | 19 | import pytz | 21 | import pytz |
| 45 | 20 | import transaction | 22 | import transaction |
| 46 | 23 | from storm.locals import Store | ||
| 47 | 21 | from zope.component import getUtility | 24 | from zope.component import getUtility |
| 48 | 22 | from zope.event import notify | 25 | from zope.event import notify |
| 49 | 23 | 26 | ||
| 50 | @@ -25,6 +28,8 @@ | |||
| 51 | 25 | 28 | ||
| 52 | 26 | from lp.code.interfaces.branchjob import IRosettaUploadJobSource | 29 | from lp.code.interfaces.branchjob import IRosettaUploadJobSource |
| 53 | 27 | from lp.code.interfaces.revision import IRevisionSet | 30 | from lp.code.interfaces.revision import IRevisionSet |
| 54 | 31 | from lp.code.model.branchrevision import (BranchRevision) | ||
| 55 | 32 | from lp.code.model.revision import Revision | ||
| 56 | 28 | from lp.codehosting import iter_list_chunks | 33 | from lp.codehosting import iter_list_chunks |
| 57 | 29 | from lp.codehosting.scanner import events | 34 | from lp.codehosting.scanner import events |
| 58 | 30 | from lp.translations.interfaces.translationtemplatesbuildjob import ( | 35 | from lp.translations.interfaces.translationtemplatesbuildjob import ( |
| 59 | @@ -65,7 +70,8 @@ | |||
| 60 | 65 | 70 | ||
| 61 | 66 | * Revision: there must be one Revision row for each revision in the | 71 | * Revision: there must be one Revision row for each revision in the |
| 62 | 67 | branch ancestry. If the row for a revision that has just been added | 72 | branch ancestry. If the row for a revision that has just been added |
| 64 | 68 | to the branch is already present, it must be checked for consistency. | 73 | to the branch is already present, it must be checked for |
| 65 | 74 | consistency. | ||
| 66 | 69 | 75 | ||
| 67 | 70 | * BranchRevision: there must be one BrancheRevision row for each | 76 | * BranchRevision: there must be one BrancheRevision row for each |
| 68 | 71 | revision in the branch ancestry. If history revisions became merged | 77 | revision in the branch ancestry. If history revisions became merged |
| 69 | @@ -78,20 +84,21 @@ | |||
| 70 | 78 | self.logger.info(" from %s", bzr_branch.base) | 84 | self.logger.info(" from %s", bzr_branch.base) |
| 71 | 79 | # Get the history and ancestry from the branch first, to fail early | 85 | # Get the history and ancestry from the branch first, to fail early |
| 72 | 80 | # if something is wrong with the branch. | 86 | # if something is wrong with the branch. |
| 74 | 81 | bzr_ancestry, bzr_history = self.retrieveBranchDetails(bzr_branch) | 87 | self.logger.info("Retrieving history from bzrlib.") |
| 75 | 88 | bzr_history = bzr_branch.revision_history() | ||
| 76 | 82 | # The BranchRevision, Revision and RevisionParent tables are only | 89 | # The BranchRevision, Revision and RevisionParent tables are only |
| 77 | 83 | # written to by the branch-scanner, so they are not subject to | 90 | # written to by the branch-scanner, so they are not subject to |
| 78 | 84 | # write-lock contention. Update them all in a single transaction to | 91 | # write-lock contention. Update them all in a single transaction to |
| 79 | 85 | # improve the performance and allow garbage collection in the future. | 92 | # improve the performance and allow garbage collection in the future. |
| 80 | 86 | db_ancestry, db_history = self.retrieveDatabaseAncestry() | 93 | db_ancestry, db_history = self.retrieveDatabaseAncestry() |
| 81 | 87 | 94 | ||
| 83 | 88 | (added_ancestry, branchrevisions_to_delete, | 95 | (new_ancestry, branchrevisions_to_delete, |
| 84 | 89 | revids_to_insert) = self.planDatabaseChanges( | 96 | revids_to_insert) = self.planDatabaseChanges( |
| 90 | 90 | bzr_branch, bzr_ancestry, bzr_history, db_ancestry, db_history) | 97 | bzr_branch, bzr_history, db_ancestry, db_history) |
| 91 | 91 | added_ancestry.difference_update( | 98 | new_db_revs = ( |
| 92 | 92 | getUtility(IRevisionSet).onlyPresent(added_ancestry)) | 99 | new_ancestry - getUtility(IRevisionSet).onlyPresent(new_ancestry)) |
| 93 | 93 | self.logger.info("Adding %s new revisions.", len(added_ancestry)) | 100 | self.logger.info("Adding %s new revisions.", len(new_db_revs)) |
| 94 | 94 | for revids in iter_list_chunks(list(added_ancestry), 1000): | 101 | for revids in iter_list_chunks(list(new_db_revs), 1000): |
| 95 | 95 | revisions = self.getBazaarRevisions(bzr_branch, revids) | 102 | revisions = self.getBazaarRevisions(bzr_branch, revids) |
| 96 | 96 | for revision in revisions: | 103 | for revision in revisions: |
| 97 | 97 | # This would probably go much faster if we found some way to | 104 | # This would probably go much faster if we found some way to |
| 98 | @@ -122,7 +129,7 @@ | |||
| 99 | 122 | self.updateBranchStatus(bzr_history) | 129 | self.updateBranchStatus(bzr_history) |
| 100 | 123 | notify( | 130 | notify( |
| 101 | 124 | events.ScanCompleted( | 131 | events.ScanCompleted( |
| 103 | 125 | self.db_branch, bzr_branch, bzr_ancestry, self.logger)) | 132 | self.db_branch, bzr_branch, self.logger, new_ancestry)) |
| 104 | 126 | transaction.commit() | 133 | transaction.commit() |
| 105 | 127 | 134 | ||
| 106 | 128 | def retrieveDatabaseAncestry(self): | 135 | def retrieveDatabaseAncestry(self): |
| 107 | @@ -131,28 +138,40 @@ | |||
| 108 | 131 | db_ancestry, db_history = self.db_branch.getScannerData() | 138 | db_ancestry, db_history = self.db_branch.getScannerData() |
| 109 | 132 | return db_ancestry, db_history | 139 | return db_ancestry, db_history |
| 110 | 133 | 140 | ||
| 133 | 134 | def retrieveBranchDetails(self, bzr_branch): | 141 | def _getRevisionGraph(self, bzr_branch, db_last): |
| 134 | 135 | """Retrieve ancestry from the the bzr branch on disk.""" | 142 | if bzr_branch.repository.has_revision(db_last): |
| 135 | 136 | self.logger.info("Retrieving ancestry from bzrlib.") | 143 | return bzr_branch.repository.get_graph() |
| 136 | 137 | last_revision = bzr_branch.last_revision() | 144 | revisions = Store.of(self.db_branch).find(Revision, |
| 137 | 138 | # Make bzr_ancestry a set for consistency with db_ancestry. | 145 | BranchRevision.branch_id == self.db_branch.id, |
| 138 | 139 | bzr_ancestry_ordered = ( | 146 | Revision.id == BranchRevision.revision_id) |
| 139 | 140 | bzr_branch.repository.get_ancestry(last_revision)) | 147 | parent_map = dict( |
| 140 | 141 | first_ancestor = bzr_ancestry_ordered.pop(0) | 148 | (r.revision_id, r.parent_ids) for r in revisions) |
| 141 | 142 | assert first_ancestor is None, 'history horizons are not supported' | 149 | parents_provider = DictParentsProvider(parent_map) |
| 142 | 143 | bzr_ancestry = set(bzr_ancestry_ordered) | 150 | |
| 143 | 144 | bzr_history = bzr_branch.revision_history() | 151 | class PPSource: |
| 144 | 145 | return bzr_ancestry, bzr_history | 152 | |
| 145 | 146 | 153 | @staticmethod | |
| 146 | 147 | def planDatabaseChanges(self, bzr_branch, bzr_ancestry, bzr_history, | 154 | def _make_parents_provider(): |
| 147 | 148 | db_ancestry, db_history): | 155 | return parents_provider |
| 148 | 149 | """Plan database changes to synchronize with bzrlib data. | 156 | |
| 149 | 150 | 157 | return bzr_branch.repository.get_graph(PPSource) | |
| 150 | 151 | Use the data retrieved by `retrieveDatabaseAncestry` and | 158 | |
| 151 | 152 | `retrieveBranchDetails` to plan the changes to apply to the database. | 159 | def getAncestryDelta(self, bzr_branch): |
| 152 | 153 | """ | 160 | bzr_last = bzr_branch.last_revision() |
| 153 | 154 | self.logger.info("Planning changes.") | 161 | db_last = self.db_branch.last_scanned_id |
| 154 | 155 | # Find the length of the common history. | 162 | if db_last is None: |
| 155 | 163 | added_ancestry = set(bzr_branch.repository.get_ancestry(bzr_last)) | ||
| 156 | 164 | added_ancestry.discard(None) | ||
| 157 | 165 | removed_ancestry = set() | ||
| 158 | 166 | else: | ||
| 159 | 167 | graph = self._getRevisionGraph(bzr_branch, db_last) | ||
| 160 | 168 | added_ancestry, removed_ancestry = ( | ||
| 161 | 169 | graph.find_difference(bzr_last, db_last)) | ||
| 162 | 170 | added_ancestry.discard(NULL_REVISION) | ||
| 163 | 171 | return added_ancestry, removed_ancestry | ||
| 164 | 172 | |||
| 165 | 173 | def getHistoryDelta(self, bzr_history, db_history): | ||
| 166 | 174 | self.logger.info("Calculating history delta.") | ||
| 167 | 156 | common_len = min(len(bzr_history), len(db_history)) | 175 | common_len = min(len(bzr_history), len(db_history)) |
| 168 | 157 | while common_len > 0: | 176 | while common_len > 0: |
| 169 | 158 | # The outer conditional improves efficiency. Without it, the | 177 | # The outer conditional improves efficiency. Without it, the |
| 170 | @@ -165,39 +184,44 @@ | |||
| 171 | 165 | if db_history[:common_len] == bzr_history[:common_len]: | 184 | if db_history[:common_len] == bzr_history[:common_len]: |
| 172 | 166 | break | 185 | break |
| 173 | 167 | common_len -= 1 | 186 | common_len -= 1 |
| 174 | 168 | |||
| 175 | 169 | # Revisions added to the branch's ancestry. | ||
| 176 | 170 | added_ancestry = bzr_ancestry.difference(db_ancestry) | ||
| 177 | 171 | |||
| 178 | 172 | # Revision added or removed from the branch's history. These lists may | 187 | # Revision added or removed from the branch's history. These lists may |
| 179 | 173 | # include revisions whose history position has merely changed. | 188 | # include revisions whose history position has merely changed. |
| 180 | 174 | removed_history = db_history[common_len:] | 189 | removed_history = db_history[common_len:] |
| 181 | 175 | added_history = bzr_history[common_len:] | 190 | added_history = bzr_history[common_len:] |
| 182 | 191 | return added_history, removed_history | ||
| 183 | 192 | |||
| 184 | 193 | def planDatabaseChanges(self, bzr_branch, bzr_history, db_ancestry, | ||
| 185 | 194 | db_history): | ||
| 186 | 195 | """Plan database changes to synchronize with bzrlib data. | ||
| 187 | 196 | |||
| 188 | 197 | Use the data retrieved by `retrieveDatabaseAncestry` and | ||
| 189 | 198 | `retrieveBranchDetails` to plan the changes to apply to the database. | ||
| 190 | 199 | """ | ||
| 191 | 200 | self.logger.info("Planning changes.") | ||
| 192 | 201 | # Find the length of the common history. | ||
| 193 | 202 | added_history, removed_history = self.getHistoryDelta( | ||
| 194 | 203 | bzr_history, db_history) | ||
| 195 | 204 | added_ancestry, removed_ancestry = self.getAncestryDelta(bzr_branch) | ||
| 196 | 176 | 205 | ||
| 197 | 177 | notify( | 206 | notify( |
| 198 | 178 | events.RevisionsRemoved( | 207 | events.RevisionsRemoved( |
| 199 | 179 | self.db_branch, bzr_branch, removed_history)) | 208 | self.db_branch, bzr_branch, removed_history)) |
| 200 | 180 | 209 | ||
| 201 | 181 | # Merged (non-history) revisions in the database and the bzr branch. | ||
| 202 | 182 | old_merged = db_ancestry.difference(db_history) | ||
| 203 | 183 | new_merged = bzr_ancestry.difference(bzr_history) | ||
| 204 | 184 | |||
| 205 | 185 | # Revisions added or removed from the set of merged revisions. | ||
| 206 | 186 | removed_merged = old_merged.difference(new_merged) | ||
| 207 | 187 | added_merged = new_merged.difference(old_merged) | ||
| 208 | 188 | |||
| 209 | 189 | # We must delete BranchRevision rows for all revisions which where | 210 | # We must delete BranchRevision rows for all revisions which where |
| 210 | 190 | # removed from the ancestry or whose sequence value has changed. | 211 | # removed from the ancestry or whose sequence value has changed. |
| 213 | 191 | branchrevisions_to_delete = list( | 212 | branchrevisions_to_delete = set(removed_history) |
| 214 | 192 | removed_merged.union(removed_history)) | 213 | branchrevisions_to_delete.update(removed_ancestry) |
| 215 | 214 | branchrevisions_to_delete.update( | ||
| 216 | 215 | set(added_history).difference(added_ancestry)) | ||
| 217 | 193 | 216 | ||
| 218 | 194 | # We must insert BranchRevision rows for all revisions which were | 217 | # We must insert BranchRevision rows for all revisions which were |
| 219 | 195 | # added to the ancestry or whose sequence value has changed. | 218 | # added to the ancestry or whose sequence value has changed. |
| 220 | 219 | last_revno = len(bzr_history) | ||
| 221 | 196 | revids_to_insert = dict( | 220 | revids_to_insert = dict( |
| 224 | 197 | self.getRevisions( | 221 | self.revisionsToInsert( |
| 225 | 198 | bzr_history, added_merged.union(added_history))) | 222 | added_history, last_revno, added_ancestry)) |
| 226 | 199 | 223 | ||
| 228 | 200 | return (added_ancestry, branchrevisions_to_delete, | 224 | return (added_ancestry, list(branchrevisions_to_delete), |
| 229 | 201 | revids_to_insert) | 225 | revids_to_insert) |
| 230 | 202 | 226 | ||
| 231 | 203 | def getBazaarRevisions(self, bzr_branch, revisions): | 227 | def getBazaarRevisions(self, bzr_branch, revisions): |
| 232 | @@ -228,17 +252,20 @@ | |||
| 233 | 228 | self.db_branch, bzr_branch, db_revision, bzr_revision, | 252 | self.db_branch, bzr_branch, db_revision, bzr_revision, |
| 234 | 229 | revids_to_insert[revision_id])) | 253 | revids_to_insert[revision_id])) |
| 235 | 230 | 254 | ||
| 238 | 231 | def getRevisions(self, bzr_history, revision_subset): | 255 | @staticmethod |
| 239 | 232 | """Iterate over '(revid, revno)' pairs in a branch's ancestry. | 256 | def revisionsToInsert(added_history, last_revno, added_ancestry): |
| 240 | 257 | """Calculate the revisions to insert and their revnos. | ||
| 241 | 233 | 258 | ||
| 244 | 234 | Generate a sequence of (revision-id, sequence) pairs to be inserted | 259 | :param added_history: A list of revision ids added to the revision |
| 245 | 235 | into the branchrevision table. | 260 | history in parent-to-child order. |
| 246 | 261 | :param last_revno: The revno of the last revision. | ||
| 247 | 262 | :param added_ancestry: A set of revisions that have been added to the | ||
| 248 | 263 | ancestry of the branch. May overlap with added_history. | ||
| 249 | 236 | """ | 264 | """ |
| 255 | 237 | for (index, revision_id) in enumerate(bzr_history): | 265 | start_revno = last_revno - len(added_history) + 1 |
| 256 | 238 | if revision_id in revision_subset: | 266 | for (revno, revision_id) in enumerate(added_history, start_revno): |
| 257 | 239 | # sequence numbers start from 1 | 267 | yield revision_id, revno |
| 258 | 240 | yield revision_id, index + 1 | 268 | for revision_id in added_ancestry.difference(added_history): |
| 254 | 241 | for revision_id in revision_subset.difference(set(bzr_history)): | ||
| 259 | 242 | yield revision_id, None | 269 | yield revision_id, None |
| 260 | 243 | 270 | ||
| 261 | 244 | def deleteBranchRevisions(self, revision_ids_to_delete): | 271 | def deleteBranchRevisions(self, revision_ids_to_delete): |
| 262 | 245 | 272 | ||
| 263 | === modified file 'lib/lp/codehosting/scanner/events.py' | |||
| 264 | --- lib/lp/codehosting/scanner/events.py 2010-08-20 20:31:18 +0000 | |||
| 265 | +++ lib/lp/codehosting/scanner/events.py 2010-10-19 20:46:20 +0000 | |||
| 266 | @@ -111,6 +111,7 @@ | |||
| 267 | 111 | ScannerEvent.__init__(self, db_branch, bzr_branch) | 111 | ScannerEvent.__init__(self, db_branch, bzr_branch) |
| 268 | 112 | self.removed_history = removed_history | 112 | self.removed_history = removed_history |
| 269 | 113 | 113 | ||
| 270 | 114 | |||
| 271 | 114 | class IScanCompleted(IObjectEvent): | 115 | class IScanCompleted(IObjectEvent): |
| 272 | 115 | """The scan has been completed and the database is up-to-date.""" | 116 | """The scan has been completed and the database is up-to-date.""" |
| 273 | 116 | 117 | ||
| 274 | @@ -120,7 +121,7 @@ | |||
| 275 | 120 | 121 | ||
| 276 | 121 | implements(IScanCompleted) | 122 | implements(IScanCompleted) |
| 277 | 122 | 123 | ||
| 279 | 123 | def __init__(self, db_branch, bzr_branch, bzr_ancestry, logger): | 124 | def __init__(self, db_branch, bzr_branch, logger, new_ancestry): |
| 280 | 124 | """Construct a `ScanCompleted` event. | 125 | """Construct a `ScanCompleted` event. |
| 281 | 125 | 126 | ||
| 282 | 126 | :param db_branch: The database branch. | 127 | :param db_branch: The database branch. |
| 283 | @@ -131,7 +132,7 @@ | |||
| 284 | 131 | information, such as merges that we find. | 132 | information, such as merges that we find. |
| 285 | 132 | """ | 133 | """ |
| 286 | 133 | ScannerEvent.__init__(self, db_branch, bzr_branch) | 134 | ScannerEvent.__init__(self, db_branch, bzr_branch) |
| 288 | 134 | self.bzr_ancestry = bzr_ancestry | 135 | self.new_ancestry = new_ancestry |
| 289 | 135 | # This is kind of ick. In a strict Zope sense, the logger should | 136 | # This is kind of ick. In a strict Zope sense, the logger should |
| 290 | 136 | # probably be a registered utility. | 137 | # probably be a registered utility. |
| 291 | 137 | self.logger = logger | 138 | self.logger = logger |
| 292 | 138 | 139 | ||
| 293 | === modified file 'lib/lp/codehosting/scanner/mergedetection.py' | |||
| 294 | --- lib/lp/codehosting/scanner/mergedetection.py 2010-08-20 20:31:18 +0000 | |||
| 295 | +++ lib/lp/codehosting/scanner/mergedetection.py 2010-10-19 20:46:20 +0000 | |||
| 296 | @@ -78,7 +78,7 @@ | |||
| 297 | 78 | determine which other branches this branch has been merged into. | 78 | determine which other branches this branch has been merged into. |
| 298 | 79 | """ | 79 | """ |
| 299 | 80 | db_branch = scan_completed.db_branch | 80 | db_branch = scan_completed.db_branch |
| 301 | 81 | bzr_ancestry = scan_completed.bzr_ancestry | 81 | new_ancestry = scan_completed.new_ancestry |
| 302 | 82 | logger = scan_completed.logger | 82 | logger = scan_completed.logger |
| 303 | 83 | 83 | ||
| 304 | 84 | # XXX: JonathanLange 2009-05-05 spec=package-branches: Yet another thing | 84 | # XXX: JonathanLange 2009-05-05 spec=package-branches: Yet another thing |
| 305 | @@ -112,7 +112,7 @@ | |||
| 306 | 112 | # If the tip revisions are the same, then it is the same | 112 | # If the tip revisions are the same, then it is the same |
| 307 | 113 | # branch, not one merged into the other. | 113 | # branch, not one merged into the other. |
| 308 | 114 | pass | 114 | pass |
| 310 | 115 | elif last_scanned in bzr_ancestry: | 115 | elif last_scanned in new_ancestry: |
| 311 | 116 | merge_detected(logger, branch, db_branch) | 116 | merge_detected(logger, branch, db_branch) |
| 312 | 117 | 117 | ||
| 313 | 118 | 118 | ||
| 314 | @@ -140,7 +140,7 @@ | |||
| 315 | 140 | def auto_merge_proposals(scan_completed): | 140 | def auto_merge_proposals(scan_completed): |
| 316 | 141 | """Detect merged proposals.""" | 141 | """Detect merged proposals.""" |
| 317 | 142 | db_branch = scan_completed.db_branch | 142 | db_branch = scan_completed.db_branch |
| 319 | 143 | bzr_ancestry = scan_completed.bzr_ancestry | 143 | new_ancestry = scan_completed.new_ancestry |
| 320 | 144 | logger = scan_completed.logger | 144 | logger = scan_completed.logger |
| 321 | 145 | 145 | ||
| 322 | 146 | # Check landing candidates in non-terminal states to see if their tip | 146 | # Check landing candidates in non-terminal states to see if their tip |
| 323 | @@ -159,7 +159,7 @@ | |||
| 324 | 159 | scan_completed.bzr_branch.iter_merge_sorted_revisions()) | 159 | scan_completed.bzr_branch.iter_merge_sorted_revisions()) |
| 325 | 160 | for proposal in db_branch.landing_candidates: | 160 | for proposal in db_branch.landing_candidates: |
| 326 | 161 | tip_rev_id = proposal.source_branch.last_scanned_id | 161 | tip_rev_id = proposal.source_branch.last_scanned_id |
| 328 | 162 | if tip_rev_id in bzr_ancestry: | 162 | if tip_rev_id in new_ancestry: |
| 329 | 163 | merged_revno = find_merged_revno(merge_sorted, tip_rev_id) | 163 | merged_revno = find_merged_revno(merge_sorted, tip_rev_id) |
| 330 | 164 | # Remember so we can find the merged revision number. | 164 | # Remember so we can find the merged revision number. |
| 331 | 165 | merge_detected( | 165 | merge_detected( |
| 332 | 166 | 166 | ||
| 333 | === modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py' | |||
| 334 | --- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-10-04 19:50:45 +0000 | |||
| 335 | +++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-10-19 20:46:20 +0000 | |||
| 336 | @@ -38,9 +38,10 @@ | |||
| 337 | 38 | RevisionAuthor, | 38 | RevisionAuthor, |
| 338 | 39 | RevisionParent, | 39 | RevisionParent, |
| 339 | 40 | ) | 40 | ) |
| 340 | 41 | from lp.codehosting.bzrutils import write_locked | ||
| 341 | 41 | from lp.codehosting.scanner.bzrsync import BzrSync | 42 | from lp.codehosting.scanner.bzrsync import BzrSync |
| 342 | 42 | from lp.services.osutils import override_environ | 43 | from lp.services.osutils import override_environ |
| 344 | 43 | from lp.testing import TestCaseWithFactory | 44 | from lp.testing import TestCaseWithFactory, temp_dir |
| 345 | 44 | from lp.translations.interfaces.translations import ( | 45 | from lp.translations.interfaces.translations import ( |
| 346 | 45 | TranslationsBranchImportMode, | 46 | TranslationsBranchImportMode, |
| 347 | 46 | ) | 47 | ) |
| 348 | @@ -391,37 +392,99 @@ | |||
| 349 | 391 | self.assertEqual(rev_1.revision_date, dt) | 392 | self.assertEqual(rev_1.revision_date, dt) |
| 350 | 392 | self.assertEqual(rev_2.revision_date, dt) | 393 | self.assertEqual(rev_2.revision_date, dt) |
| 351 | 393 | 394 | ||
| 353 | 394 | def test_get_revisions_empty(self): | 395 | def getAncestryDelta_test(self, clean_repository=False): |
| 354 | 396 | """"Test various ancestry delta calculations. | ||
| 355 | 397 | |||
| 356 | 398 | :param clean_repository: If True, perform calculations with a branch | ||
| 357 | 399 | whose repository contains only revisions in the ancestry of the | ||
| 358 | 400 | tip. | ||
| 359 | 401 | """ | ||
| 360 | 402 | (db_branch, bzr_tree), ignored = self.makeBranchWithMerge( | ||
| 361 | 403 | 'base', 'trunk', 'branch', 'merge') | ||
| 362 | 404 | bzr_branch = bzr_tree.branch | ||
| 363 | 405 | self.factory.makeBranchRevision(db_branch, 'base', 0) | ||
| 364 | 406 | self.factory.makeBranchRevision( | ||
| 365 | 407 | db_branch, 'trunk', 1, parent_ids=['base']) | ||
| 366 | 408 | self.factory.makeBranchRevision( | ||
| 367 | 409 | db_branch, 'branch', None, parent_ids=['base']) | ||
| 368 | 410 | self.factory.makeBranchRevision( | ||
| 369 | 411 | db_branch, 'merge', 2, parent_ids=['trunk', 'branch']) | ||
| 370 | 412 | sync = self.makeBzrSync(db_branch) | ||
| 371 | 413 | self.useContext(write_locked(bzr_branch)) | ||
| 372 | 414 | |||
| 373 | 415 | def get_delta(bzr_rev, db_rev): | ||
| 374 | 416 | db_branch.last_scanned_id = db_rev | ||
| 375 | 417 | graph = bzr_branch.repository.get_graph() | ||
| 376 | 418 | revno = graph.find_distance_to_null(bzr_rev, []) | ||
| 377 | 419 | if clean_repository: | ||
| 378 | 420 | tempdir = self.useContext(temp_dir()) | ||
| 379 | 421 | delta_branch = self.createBranchAtURL(tempdir) | ||
| 380 | 422 | self.useContext(write_locked(delta_branch)) | ||
| 381 | 423 | delta_branch.pull(bzr_branch, stop_revision=bzr_rev) | ||
| 382 | 424 | else: | ||
| 383 | 425 | bzr_branch.set_last_revision_info(revno, bzr_rev) | ||
| 384 | 426 | delta_branch = bzr_branch | ||
| 385 | 427 | return sync.getAncestryDelta(delta_branch) | ||
| 386 | 428 | |||
| 387 | 429 | added_ancestry, removed_ancestry = get_delta('merge', None) | ||
| 388 | 430 | # All revisions are new for an unscanned branch | ||
| 389 | 431 | self.assertEqual( | ||
| 390 | 432 | set(['base', 'trunk', 'branch', 'merge']), added_ancestry) | ||
| 391 | 433 | self.assertEqual(set(), removed_ancestry) | ||
| 392 | 434 | added_ancestry, removed_ancestry = get_delta('merge', 'base') | ||
| 393 | 435 | self.assertEqual( | ||
| 394 | 436 | set(['trunk', 'branch', 'merge']), added_ancestry) | ||
| 395 | 437 | self.assertEqual(set(), removed_ancestry) | ||
| 396 | 438 | added_ancestry, removed_ancestry = get_delta(NULL_REVISION, 'merge') | ||
| 397 | 439 | self.assertEqual( | ||
| 398 | 440 | set(), added_ancestry) | ||
| 399 | 441 | self.assertEqual( | ||
| 400 | 442 | set(['base', 'trunk', 'branch', 'merge']), removed_ancestry) | ||
| 401 | 443 | added_ancestry, removed_ancestry = get_delta('base', 'merge') | ||
| 402 | 444 | self.assertEqual( | ||
| 403 | 445 | set(), added_ancestry) | ||
| 404 | 446 | self.assertEqual( | ||
| 405 | 447 | set(['trunk', 'branch', 'merge']), removed_ancestry) | ||
| 406 | 448 | added_ancestry, removed_ancestry = get_delta('trunk', 'branch') | ||
| 407 | 449 | self.assertEqual(set(['trunk']), added_ancestry) | ||
| 408 | 450 | self.assertEqual(set(['branch']), removed_ancestry) | ||
| 409 | 451 | |||
| 410 | 452 | def test_getAncestryDelta(self): | ||
| 411 | 453 | """"Test ancestry delta calculations with a dirty repository.""" | ||
| 412 | 454 | return self.getAncestryDelta_test() | ||
| 413 | 455 | |||
| 414 | 456 | def test_getAncestryDelta_clean_repository(self): | ||
| 415 | 457 | """"Test ancestry delta calculations with a clean repository.""" | ||
| 416 | 458 | return self.getAncestryDelta_test(clean_repository=True) | ||
| 417 | 459 | |||
| 418 | 460 | def test_revisionsToInsert_empty(self): | ||
| 419 | 395 | # An empty branch should have no revisions. | 461 | # An empty branch should have no revisions. |
| 420 | 396 | bzrsync = self.makeBzrSync(self.db_branch) | ||
| 421 | 397 | bzr_ancestry, bzr_history = ( | ||
| 422 | 398 | bzrsync.retrieveBranchDetails(self.bzr_branch)) | ||
| 423 | 399 | self.assertEqual( | 462 | self.assertEqual( |
| 425 | 400 | [], list(bzrsync.getRevisions(bzr_history, bzr_ancestry))) | 463 | [], list(BzrSync.revisionsToInsert([], 0, set()))) |
| 426 | 401 | 464 | ||
| 430 | 402 | def test_get_revisions_linear(self): | 465 | def test_revisionsToInsert_linear(self): |
| 431 | 403 | # If the branch has a linear ancestry, getRevisions() should yield | 466 | # If the branch has a linear ancestry, revisionsToInsert() should |
| 432 | 404 | # each revision along with a sequence number, starting at 1. | 467 | # yield each revision along with a sequence number, starting at 1. |
| 433 | 405 | self.commitRevision(rev_id='rev-1') | 468 | self.commitRevision(rev_id='rev-1') |
| 434 | 406 | bzrsync = self.makeBzrSync(self.db_branch) | 469 | bzrsync = self.makeBzrSync(self.db_branch) |
| 440 | 407 | bzr_ancestry, bzr_history = ( | 470 | bzr_history = self.bzr_branch.revision_history() |
| 441 | 408 | bzrsync.retrieveBranchDetails(self.bzr_branch)) | 471 | added_ancestry = bzrsync.getAncestryDelta(self.bzr_branch)[0] |
| 442 | 409 | self.assertEqual( | 472 | result = bzrsync.revisionsToInsert( |
| 443 | 410 | [('rev-1', 1)], | 473 | bzr_history, self.bzr_branch.revno(), added_ancestry) |
| 444 | 411 | list(bzrsync.getRevisions(bzr_history, bzr_ancestry))) | 474 | self.assertEqual({'rev-1': 1}, dict(result)) |
| 445 | 412 | 475 | ||
| 447 | 413 | def test_get_revisions_branched(self): | 476 | def test_revisionsToInsert_branched(self): |
| 448 | 414 | # Confirm that these revisions are generated by getRevisions with None | 477 | # Confirm that these revisions are generated by getRevisions with None |
| 449 | 415 | # as the sequence 'number'. | 478 | # as the sequence 'number'. |
| 450 | 416 | (db_branch, bzr_tree), ignored = self.makeBranchWithMerge( | 479 | (db_branch, bzr_tree), ignored = self.makeBranchWithMerge( |
| 451 | 417 | 'base', 'trunk', 'branch', 'merge') | 480 | 'base', 'trunk', 'branch', 'merge') |
| 452 | 418 | bzrsync = self.makeBzrSync(db_branch) | 481 | bzrsync = self.makeBzrSync(db_branch) |
| 457 | 419 | bzr_ancestry, bzr_history = ( | 482 | bzr_history = bzr_tree.branch.revision_history() |
| 458 | 420 | bzrsync.retrieveBranchDetails(bzr_tree.branch)) | 483 | added_ancestry = bzrsync.getAncestryDelta(bzr_tree.branch)[0] |
| 459 | 421 | expected = set( | 484 | expected = {'base': 1, 'trunk': 2, 'merge': 3, 'branch': None} |
| 456 | 422 | [('base', 1), ('trunk', 2), ('merge', 3), ('branch', None)]) | ||
| 460 | 423 | self.assertEqual( | 485 | self.assertEqual( |
| 462 | 424 | expected, set(bzrsync.getRevisions(bzr_history, bzr_ancestry))) | 486 | expected, dict(bzrsync.revisionsToInsert(bzr_history, |
| 463 | 487 | bzr_tree.branch.revno(), added_ancestry))) | ||
| 464 | 425 | 488 | ||
| 465 | 426 | def test_sync_with_merged_branches(self): | 489 | def test_sync_with_merged_branches(self): |
| 466 | 427 | # Confirm that when we syncHistory, all of the revisions are included | 490 | # Confirm that when we syncHistory, all of the revisions are included |
| 467 | @@ -460,19 +523,6 @@ | |||
| 468 | 460 | expected = set([(1, 'base'), (2, 'branch')]) | 523 | expected = set([(1, 'base'), (2, 'branch')]) |
| 469 | 461 | self.assertEqual(self.getBranchRevisions(db_trunk), expected) | 524 | self.assertEqual(self.getBranchRevisions(db_trunk), expected) |
| 470 | 462 | 525 | ||
| 471 | 463 | def test_retrieveBranchDetails(self): | ||
| 472 | 464 | # retrieveBranchDetails should set last_revision, bzr_ancestry and | ||
| 473 | 465 | # bzr_history on the BzrSync instance to match the information in the | ||
| 474 | 466 | # Bazaar branch. | ||
| 475 | 467 | (db_trunk, trunk_tree), ignored = self.makeBranchWithMerge( | ||
| 476 | 468 | 'base', 'trunk', 'branch', 'merge') | ||
| 477 | 469 | bzrsync = self.makeBzrSync(db_trunk) | ||
| 478 | 470 | bzr_ancestry, bzr_history = ( | ||
| 479 | 471 | bzrsync.retrieveBranchDetails(trunk_tree.branch)) | ||
| 480 | 472 | expected_ancestry = set(['base', 'trunk', 'branch', 'merge']) | ||
| 481 | 473 | self.assertEqual(expected_ancestry, bzr_ancestry) | ||
| 482 | 474 | self.assertEqual(['base', 'trunk', 'merge'], bzr_history) | ||
| 483 | 475 | |||
| 484 | 476 | def test_retrieveDatabaseAncestry(self): | 526 | def test_retrieveDatabaseAncestry(self): |
| 485 | 477 | # retrieveDatabaseAncestry should set db_ancestry and db_history to | 527 | # retrieveDatabaseAncestry should set db_ancestry and db_history to |
| 486 | 478 | # Launchpad's current understanding of the branch state. | 528 | # Launchpad's current understanding of the branch state. |
| 487 | 479 | 529 | ||
| 488 | === modified file 'lib/lp/codehosting/scanner/tests/test_mergedetection.py' | |||
| 489 | --- lib/lp/codehosting/scanner/tests/test_mergedetection.py 2010-10-04 19:50:45 +0000 | |||
| 490 | +++ lib/lp/codehosting/scanner/tests/test_mergedetection.py 2010-10-19 20:46:20 +0000 | |||
| 491 | @@ -197,11 +197,11 @@ | |||
| 492 | 197 | mergedetection.merge_detected = self._original_merge_detected | 197 | mergedetection.merge_detected = self._original_merge_detected |
| 493 | 198 | TestCaseWithFactory.tearDown(self) | 198 | TestCaseWithFactory.tearDown(self) |
| 494 | 199 | 199 | ||
| 496 | 200 | def autoMergeBranches(self, db_branch, bzr_ancestry): | 200 | def autoMergeBranches(self, db_branch, new_ancestry): |
| 497 | 201 | mergedetection.auto_merge_branches( | 201 | mergedetection.auto_merge_branches( |
| 498 | 202 | events.ScanCompleted( | 202 | events.ScanCompleted( |
| 499 | 203 | db_branch=db_branch, bzr_branch=None, | 203 | db_branch=db_branch, bzr_branch=None, |
| 501 | 204 | bzr_ancestry=bzr_ancestry, logger=None)) | 204 | logger=None, new_ancestry=new_ancestry)) |
| 502 | 205 | 205 | ||
| 503 | 206 | def mergeDetected(self, logger, source, target): | 206 | def mergeDetected(self, logger, source, target): |
| 504 | 207 | # Record the merged branches | 207 | # Record the merged branches |
| 505 | @@ -360,7 +360,7 @@ | |||
| 506 | 360 | target = self.factory.makeBranchTargetBranch(source.target) | 360 | target = self.factory.makeBranchTargetBranch(source.target) |
| 507 | 361 | target.product.development_focus.branch = target | 361 | target.product.development_focus.branch = target |
| 508 | 362 | logger = logging.getLogger('test') | 362 | logger = logging.getLogger('test') |
| 510 | 363 | notify(events.ScanCompleted(target, None, ['23foo'], logger)) | 363 | notify(events.ScanCompleted(target, None, logger, ['23foo'])) |
| 511 | 364 | self.assertEqual( | 364 | self.assertEqual( |
| 512 | 365 | BranchLifecycleStatus.MERGED, source.lifecycle_status) | 365 | BranchLifecycleStatus.MERGED, source.lifecycle_status) |
| 513 | 366 | 366 | ||
| 514 | 367 | 367 | ||
| 515 | === modified file 'lib/lp/testing/factory.py' | |||
| 516 | --- lib/lp/testing/factory.py 2010-10-18 10:19:56 +0000 | |||
| 517 | +++ lib/lp/testing/factory.py 2010-10-19 20:46:20 +0000 | |||
| 518 | @@ -1312,8 +1312,10 @@ | |||
| 519 | 1312 | '', parent.revision_id, None, None, None) | 1312 | '', parent.revision_id, None, None, None) |
| 520 | 1313 | branch.updateScannedDetails(parent, sequence) | 1313 | branch.updateScannedDetails(parent, sequence) |
| 521 | 1314 | 1314 | ||
| 524 | 1315 | def makeBranchRevision(self, branch, revision_id, sequence=None): | 1315 | def makeBranchRevision(self, branch, revision_id, sequence=None, |
| 525 | 1316 | revision = self.makeRevision(rev_id=revision_id) | 1316 | parent_ids=None): |
| 526 | 1317 | revision = self.makeRevision( | ||
| 527 | 1318 | rev_id=revision_id, parent_ids=parent_ids) | ||
| 528 | 1317 | return branch.createBranchRevision(sequence, revision) | 1319 | return branch.createBranchRevision(sequence, revision) |
| 529 | 1318 | 1320 | ||
| 530 | 1319 | def makeBug(self, product=None, owner=None, bug_watch_url=None, | 1321 | def makeBug(self, product=None, owner=None, bug_watch_url=None, |
| 531 | @@ -1846,22 +1848,6 @@ | |||
| 532 | 1846 | expires=expires, restricted=restricted) | 1848 | expires=expires, restricted=restricted) |
| 533 | 1847 | return library_file_alias | 1849 | return library_file_alias |
| 534 | 1848 | 1850 | ||
| 535 | 1849 | def makePackageDiff(self, from_spr=None, to_spr=None): | ||
| 536 | 1850 | """Make a completed package diff.""" | ||
| 537 | 1851 | if from_spr is None: | ||
| 538 | 1852 | from_spr = self.makeSourcePackageRelease() | ||
| 539 | 1853 | if to_spr is None: | ||
| 540 | 1854 | to_spr = self.makeSourcePackageRelease() | ||
| 541 | 1855 | |||
| 542 | 1856 | diff = from_spr.requestDiffTo( | ||
| 543 | 1857 | from_spr.creator, to_spr) | ||
| 544 | 1858 | |||
| 545 | 1859 | naked_diff = removeSecurityProxy(diff) | ||
| 546 | 1860 | naked_diff.status = PackageDiffStatus.COMPLETED | ||
| 547 | 1861 | naked_diff.diff_content = self.makeLibraryFileAlias() | ||
| 548 | 1862 | naked_diff.date_fulfilled = UTC_NOW | ||
| 549 | 1863 | return diff | ||
| 550 | 1864 | |||
| 551 | 1865 | def makeDistribution(self, name=None, displayname=None, owner=None, | 1851 | def makeDistribution(self, name=None, displayname=None, owner=None, |
| 552 | 1866 | members=None, title=None, aliases=None): | 1852 | members=None, title=None, aliases=None): |
| 553 | 1867 | """Make a new distribution.""" | 1853 | """Make a new distribution.""" |
