launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/branch-empty into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #772426 in Launchpad itself: "branch-distro.py crashes on empty branches"
  https://bugs.launchpad.net/launchpad/+bug/772426

For more details, see:
https://code.launchpad.net/~abentley/launchpad/branch-empty/+merge/61132

= Summary =
Fix bug #772426: branch-distro.py crashes on empty branches

== Proposed fix ==
Use the NULL_REVISION when getTipRevision returns None.

== Pre-implementation notes ==
None

== Implementation details ==
Some driveby lint fixes.

== Tests ==
bin/test test_branchdistro -t test_makeOnewNewBranch_empty_branch

== Demo and Q/A ==
None


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/codehosting/branchdistro.py
  lib/lp/codehosting/tests/test_branchdistro.py
-- 
https://code.launchpad.net/~abentley/launchpad/branch-empty/+merge/61132
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/branch-empty into lp:launchpad.

=== modified file 'lib/lp/codehosting/branchdistro.py'
--- lib/lp/codehosting/branchdistro.py	2011-03-03 01:13:47 +0000
+++ lib/lp/codehosting/branchdistro.py	2011-05-16 15:12:38 +0000
@@ -23,6 +23,7 @@
     NotBranchError,
     NotStacked,
     )
+from bzrlib.revision import NULL_REVISION
 import transaction
 from zope.component import getUtility
 
@@ -356,7 +357,8 @@
         switch_branches(
             config.codehosting.mirrored_branches_root,
             'lp-internal', old_db_branch, new_db_branch)
-        # Directly copy the branch revisions from the old branch to the new branch.
+        # Directly copy the branch revisions from the old branch to the new
+        # branch.
         store = IMasterStore(BranchRevision)
         store.execute(
             """
@@ -371,8 +373,11 @@
         tip_revision = old_db_branch.getTipRevision()
         new_db_branch.updateScannedDetails(
             tip_revision, old_db_branch.revision_count)
+        tip_revision_id = (
+            tip_revision.revision_id if tip_revision is not None else
+            NULL_REVISION)
         new_db_branch.branchChanged(
-            '', tip_revision.revision_id,
+            '', tip_revision_id,
             old_db_branch.control_format,
             old_db_branch.branch_format,
             old_db_branch.repository_format)

=== modified file 'lib/lp/codehosting/tests/test_branchdistro.py'
--- lib/lp/codehosting/tests/test_branchdistro.py	2010-12-20 03:21:03 +0000
+++ lib/lp/codehosting/tests/test_branchdistro.py	2011-05-16 15:12:38 +0000
@@ -124,11 +124,13 @@
         TestCaseWithFactory.setUp(self)
         self.useBzrBranches(direct_database=True)
 
-    def makeOfficialPackageBranch(self, distroseries=None):
+    def makeOfficialPackageBranch(self, distroseries=None,
+                                  make_revisions=True):
         """Make an official package branch with an underlying bzr branch."""
         db_branch = self.factory.makePackageBranch(distroseries=distroseries)
         db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
-        self.factory.makeRevisionsForBranch(db_branch, count=1)
+        if make_revisions:
+            self.factory.makeRevisionsForBranch(db_branch, count=1)
 
         transaction.commit()
 
@@ -136,8 +138,9 @@
             tree_location=self.factory.getUniqueString(), db_branch=db_branch)
         # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is
         # required to generate the revision-id.
-        with override_environ(BZR_EMAIL='me@xxxxxxxxxxx'):
-            tree.commit('')
+        if make_revisions:
+            with override_environ(BZR_EMAIL='me@xxxxxxxxxxx'):
+                tree.commit('')
 
         return db_branch
 
@@ -297,6 +300,12 @@
             ['^WARNING .* is not an official branch$',
              '^WARNING Skipping branch$'])
 
+    def test_makeOnewNewBranch_empty_branch(self):
+        # Branches with no commits work.
+        db_branch = self.makeOfficialPackageBranch(make_revisions=False)
+        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
+        brancher.makeOneNewBranch(db_branch)
+
     def test_makeNewBranches(self):
         # makeNewBranches calls makeOneNewBranch for each official branch in
         # the old distroseries.
@@ -638,4 +647,3 @@
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
-