launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/branch-distro-avoid-scan into lp:launchpad/devel with lp:~thumper/launchpad/fix-branchChanged-scan-request as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #574083 branch-distro doesn't trigger a scanner run
  https://bugs.launchpad.net/bugs/574083


When branching the distro, we don't want to tie up the scanner for three days, like we have in the past.  Given that we are just copying the details of one branch to the other, we can do that as we create the branches.

tests:
  lp.codehosting.tests.test_branchdistro
-- 
https://code.launchpad.net/~thumper/launchpad/branch-distro-avoid-scan/+merge/36103
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/branch-distro-avoid-scan into lp:launchpad/devel.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-09-21 05:23:45 +0000
+++ database/schema/security.cfg	2010-09-21 05:23:46 +0000
@@ -629,6 +629,7 @@
 [branch-distro]
 type=user
 public.branch                    = SELECT, INSERT, UPDATE
+public.branchrevision            = SELECT, INSERT
 public.branchsubscription        = SELECT, INSERT
 public.distribution              = SELECT
 public.distroseries              = SELECT
@@ -636,6 +637,7 @@
 public.karmaaction               = SELECT
 public.person                    = SELECT
 public.product                   = SELECT
+public.revision                  = SELECT
 public.seriessourcepackagebranch = SELECT, INSERT, DELETE
 public.sourcepackagename         = SELECT
 public.teamparticipation         = SELECT

=== modified file 'lib/lp/codehosting/branchdistro.py'
--- lib/lp/codehosting/branchdistro.py	2010-09-21 05:23:45 +0000
+++ lib/lp/codehosting/branchdistro.py	2010-09-21 05:23:46 +0000
@@ -27,7 +27,7 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from canonical.launchpad.interfaces import ILaunchpadCelebrities
+from canonical.launchpad.interfaces import ILaunchpadCelebrities, IMasterStore
 from lp.code.enums import BranchLifecycleStatus, BranchType
 from lp.code.errors import BranchExists
 from lp.code.interfaces.branchcollection import IAllBranches
@@ -35,6 +35,7 @@
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks,
     )
+from lp.code.model.branchrevision import BranchRevision
 from lp.codehosting.vfs import branch_id_to_path
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -350,4 +351,25 @@
         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.
+        store = IMasterStore(BranchRevision)
+        store.execute(
+            """
+            INSERT INTO BranchRevision (branch, revision, sequence)
+            SELECT %s, BranchRevision.revision, BranchRevision.sequence
+            FROM BranchRevision
+            WHERE branch = %s
+            """ % (new_db_branch.id, old_db_branch.id))
+
+        # Update the scanned details first, that way when hooking into
+        # branchChanged, it won't try to create a new scan job.
+        tip_revision = old_db_branch.getTipRevision()
+        new_db_branch.updateScannedDetails(
+            tip_revision, old_db_branch.revision_count)
+        new_db_branch.branchChanged(
+            '', tip_revision.revision_id,
+            old_db_branch.control_format,
+            old_db_branch.branch_format,
+            old_db_branch.repository_format)
+        transaction.commit()
         return new_db_branch

=== modified file 'lib/lp/codehosting/tests/test_branchdistro.py'
--- lib/lp/codehosting/tests/test_branchdistro.py	2010-09-21 05:23:45 +0000
+++ lib/lp/codehosting/tests/test_branchdistro.py	2010-09-21 05:23:46 +0000
@@ -27,6 +27,8 @@
 from bzrlib.transport.chroot import ChrootServer
 from lazr.uri import URI
 import transaction
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
 from canonical.launchpad.scripts.logger import (
@@ -35,6 +37,7 @@
     )
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.code.enums import BranchLifecycleStatus
+from lp.code.interfaces.branchjob import IBranchScanJobSource
 from lp.codehosting.branchdistro import (
     DistroBrancher,
     switch_branches,
@@ -127,6 +130,7 @@
         """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)
 
         transaction.commit()
 
@@ -241,6 +245,42 @@
         self.assertEqual(
             BranchLifecycleStatus.MATURE, db_branch.lifecycle_status)
 
+    def test_makeOneNewBranch_avoids_need_for_scan(self):
+        # makeOneNewBranch sets the appropriate properties of the new branch
+        # so a scan is unnecessary.  This can be done because we are making a
+        # copy of the source branch.
+        db_branch = self.makeOfficialPackageBranch()
+        self.factory.makeRevisionsForBranch(db_branch, count=10)
+        tip_revision_id = db_branch.last_mirrored_id
+        self.assertIsNot(None, tip_revision_id)
+        # The makeRevisionsForBranch will create a scan job for the db_branch.
+        # We don't really care about that, but what we do care about is that
+        # no new jobs are created.
+        existing_scan_job_count = len(
+            list(getUtility(IBranchScanJobSource).iterReady()))
+
+        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
+        brancher.makeOneNewBranch(db_branch)
+        new_branch = brancher.new_distroseries.getSourcePackage(
+            db_branch.sourcepackage.name).getBranch(RELEASE)
+
+        self.assertEqual(tip_revision_id, new_branch.last_mirrored_id)
+        self.assertEqual(tip_revision_id, new_branch.last_scanned_id)
+        # Make sure that the branch revisions have been copied.
+        old_ancestry, old_history = removeSecurityProxy(
+            db_branch).getScannerData()
+        new_ancestry, new_history = removeSecurityProxy(
+            new_branch).getScannerData()
+        self.assertEqual(old_ancestry, new_ancestry)
+        self.assertEqual(old_history, new_history)
+        self.assertFalse(new_branch.pending_writes)
+        # The script doesn't have permission to create branch jobs, but just
+        # to be insanely paradoid.
+        transaction.commit()
+        self.layer.switchDbUser('launchpad')
+        scan_jobs = list(getUtility(IBranchScanJobSource).iterReady())
+        self.assertEqual(existing_scan_job_count, len(scan_jobs))
+
     def test_makeOneNewBranch_inconsistent_branch(self):
         # makeOneNewBranch skips over an inconsistent official package branch
         # (see `checkConsistentOfficialPackageBranch` for precisely what an