launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/merge-directive-handling-no-request-mirror into lp:launchpad/devel with lp:~thumper/launchpad/bzr-format-matching-refactoring as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #608915 merge directive handling puts branch into permanent scan mode by requesting a mirror
  https://bugs.launchpad.net/bugs/608915


There are a few related changes here.

I filed a bug about how we shouldn't create empty branches from a merge directive if the branch is unstackable, and linked that in a couple of places in the code with an XXX comment.

The processing of the merge directive now uses branchChanged rather than requestMirror.
 - this change also needs the addition of BranchJob insert permissions for the creation of scan jobs.


The factory was told to be quiet around makeMergeDirective as it doesn't need to be proxied.

-- 
https://code.launchpad.net/~thumper/launchpad/merge-directive-handling-no-request-mirror/+merge/31019
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/merge-directive-handling-no-request-mirror into lp:launchpad/devel.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-07-21 09:18:56 +0000
+++ database/schema/security.cfg	2010-07-27 05:21:00 +0000
@@ -1636,7 +1636,7 @@
 public.account                          = SELECT
 public.accountpassword                  = SELECT
 public.branch                           = SELECT, INSERT, UPDATE
-public.branchjob                        = SELECT
+public.branchjob                        = SELECT, INSERT
 public.branchmergeproposal              = SELECT, INSERT, UPDATE
 public.branchmergeproposaljob           = SELECT, INSERT
 public.branchsubscription               = SELECT, INSERT

=== modified file 'lib/lp/code/mail/codehandler.py'
--- lib/lp/code/mail/codehandler.py	2010-04-23 04:11:12 +0000
+++ lib/lp/code/mail/codehandler.py	2010-07-27 05:21:00 +0000
@@ -39,6 +39,7 @@
 from canonical.launchpad.webapp.errorlog import globalErrorUtility
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from lazr.uri import URI
+from lp.code.bzr import get_branch_formats
 from lp.code.enums import BranchType, CodeReviewVote
 from lp.code.errors import BranchMergeProposalExists, UserNotBranchReviewer
 from lp.code.interfaces.branch import BranchCreationException
@@ -516,11 +517,15 @@
                 except NotStacked:
                     # We don't currently support pulling in the revisions if
                     # the source branch exists and isn't stacked.
+                    # XXX Tim Penhey 2010-07-27 bug 610292
+                    # We should fail here and return an oops email to the user.
                     return db_source
                 self._pullRevisionsFromMergeDirectiveIntoSourceBranch(
                     md, target_url, bzr_source)
                 # Get the puller to pull the branch into the mirrored area.
-                db_source.requestMirror()
+                formats = get_branch_formats(bzr_source)
+                db_source.branchChanged(
+                    stacked_url, bzr_source.last_revision(), *formats)
             return db_source
         finally:
             lp_server.stop_server()

=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2010-06-10 20:20:21 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2010-07-27 05:21:00 +0000
@@ -11,9 +11,6 @@
 import unittest
 
 from bzrlib.branch import Branch
-from bzrlib.bzrdir import BzrDir
-from bzrlib import errors as bzr_errors
-from bzrlib.transport import get_transport
 from bzrlib.urlutils import join as urljoin
 from bzrlib.workingtree import WorkingTree
 from storm.store import Store
@@ -893,7 +890,6 @@
         target_tree.branch.set_public_branch(db_target_branch.bzr_identity)
         target_tree.commit('rev1')
         # Make sure that the created branch has been mirrored.
-        db_target_branch.startMirroring()
         removeSecurityProxy(db_target_branch).branchChanged(
             '', 'rev1', None, None, None)
         source_tree = target_tree.bzrdir.sprout('source').open_workingtree()
@@ -931,6 +927,9 @@
         # branch that is created is an empty hosted branch.  The new branch
         # will not have a mirror requested as there are no revisions, and
         # there is no branch created in the hosted area.
+
+        # XXX Tim Penhey 2010-07-27 bug 610292
+        # We should fail here and return an oops email to the user.
         self.useBzrBranches()
         branch, source, message = self._createTargetSourceAndBundle(
             format="pack-0.92")
@@ -964,7 +963,7 @@
         bmp = self._processMergeDirective(message)
         source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
         self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)
-        self.assertIsNot(None, bmp.source_branch.next_mirror_time)
+        self.assertTrue(bmp.source_branch.pending_writes)
         self.assertEqual(
             source.last_revision(), source_bzr_branch.last_revision())
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-07-27 05:20:59 +0000
+++ lib/lp/testing/factory.py	2010-07-27 05:21:00 +0000
@@ -44,6 +44,8 @@
 from zope.security.proxy import (
     builtin_isinstance, Proxy, ProxyFactory, removeSecurityProxy)
 
+from bzrlib.merge_directive import MergeDirective2
+
 from canonical.autodecorate import AutoDecorate
 from canonical.config import config
 from canonical.database.constants import UTC_NOW
@@ -2615,7 +2617,6 @@
             context also contains the password and gpg signing mode.
         :param sender: The `Person` that is sending the email.
         """
-        from bzrlib.merge_directive import MergeDirective2
         md = MergeDirective2.from_objects(
             source_branch.repository, source_branch.last_revision(),
             public_branch=source_branch.get_public_branch(),
@@ -2746,7 +2747,7 @@
 # security wrappers for them, as well as for objects created by
 # other Python libraries.
 unwrapped_types = (
-    DSCFile, InstanceType, Message, datetime, int, str, unicode)
+    DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str, unicode)
 
 def is_security_proxied_or_harmless(obj):
     """Check that the given object is security wrapped or a harmless object."""