Launchpad itself

Merge lp:~mwhudson/launchpad/branching-ubuntu into lp:launchpad

branching-ubuntu
Merge into devel

Proposed by Michael Hudson-Doyle on 2009-10-13

Status:	Merged
Merged at revision:	not available
Proposed branch:	lp:~mwhudson/launchpad/branching-ubuntu
Merge into:	lp:launchpad
Diff against target:	1129 lines 4 files modified database/schema/security.cfg (+16/-0) lib/lp/codehosting/branchdistro.py (+364/-0) lib/lp/codehosting/tests/test_branchdistro.py (+685/-0) scripts/branch-distro.py (+39/-0)
To merge this branch:	bzr merge lp:~mwhudson/launchpad/branching-ubuntu
Related bugs:	Link a bug report
Related blueprints:	Branching Ubuntu on release (Essential)

Reviewer	Review Type	Date Requested	Status
Jonathan Lange (community)		2009-10-13	Approve on 2009-10-13
Review via email: mp+13269@code.launchpad.net

Commit message

A script to open a new distribution for branch-based development -- see https://dev.launchpad.net/BranchingUbuntu.

Revision history for this message

Michael Hudson-Doyle (mwhudson) wrote on 2009-10-13:

Hey Jono,

Finally I think this beast of a branch is ready (actually it's "only" 1123 lines, not too far over the limit :-).

It adds a script that carries out the procedure on https://dev.launchpad.net/BranchingUbuntu. It also adds a --check flag that verifies that a previous run of the script went ok. This was the only way I could convince myself to stop worrying about what would happen if the script crashed or was interrupted -- the idea is that if the script is interrupted we'll run the script again until it completes, then run branch-distro --check to find any damage that was left behind by the interrupted run.

I hope it makes sense, we've talked about this enough!

Cheers,
mwh

Revision history for this message

Michael Hudson-Doyle (mwhudson) wrote on 2009-10-13:

Oh, I guess I should say that we need to test this as much as we can on staging, I think this should be possible.

Revision history for this message

Jonathan Lange (jml) wrote on 2009-10-13:

Download full text (53.2 KiB)

Hey Michael,

Not much to say here, given that we've already talked about it. I look forward to seeing this QA'd on staging.

I have a few comments, one or two of which must be fixed before landing. Other than that, looks good. Thanks for getting this done.

jml

> === modified file 'database/schema/security.cfg'
> --- database/schema/security.cfg 2009-10-05 15:07:10 +0000
> +++ database/schema/security.cfg 2009-10-13 09:11:15 +0000
> @@ -641,6 +641,22 @@
> # Merge notifications
> public.codereviewvote = SELECT
>
> +[branch-distro]
> +type=user
> +public.branch = SELECT, INSERT
> +public.branchsubscription = SELECT, INSERT
> +public.distribution = SELECT
> +public.distroseries = SELECT
> +public.karma = SELECT, INSERT
> +public.karmaaction = SELECT
> +public.person = SELECT
> +public.product = SELECT
> +public.seriessourcepackagebranch = SELECT, INSERT, DELETE
> +public.sourcepackagename = SELECT
> +public.teamparticipation = SELECT
> +public.validpersoncache = SELECT
> +
> +

How did you determine these? Are you sure you've got everything?

> [targetnamecacheupdater]
> type=user
> groups=script
>

> === added file 'lib/lp/codehosting/branchdistro.py'
> --- lib/lp/codehosting/branchdistro.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/codehosting/branchdistro.py 2009-10-13 09:11:15 +0000
> @@ -0,0 +1,355 @@
> +# Copyright 2009 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Opening a new DistroSeries for branch based development.
> +
> +Intended to be run just after a new distro series has been completed, this
> +script will create an official package branch in the new series for every one
> +in the old. The old branch will become stacked on the new, to avoid a using
> +too much disk space whilst retaining best performance for the new branch.
> +"""
> +
> +import os
> +
> +from bzrlib.branch import Branch
> +from bzrlib.bzrdir import BzrDir
> +from bzrlib.errors import NotBranchError, NotStacked
> +
> +from lazr.uri import URI
> +
> +import transaction
> +
> +from zope.component import getUtility
> +
> +from canonical.launchpad.interfaces import ILaunchpadCelebrities
> +from canonical.config import config
> +
> +from lp.code.interfaces.branchcollection import IAllBranches
> +from lp.code.interfaces.branch import BranchExists
> +from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
> +from lp.code.interfaces.seriessourcepackagebranch import (
> + IFindOfficialBranchLinks)
> +from lp.code.enums import BranchType
> +from lp.codehosting.vfs import branch_id_to_path
> +from lp.registry.interfaces.distribution import IDistributionSet
> +from lp.registry.interfaces.pocket import PackagePublishingPocket
> +
> +
> +__metaclass__ = type
> +__all__ = []
> +

These should be above the imports & below the docstring, IIRC.

> +
> +def switch_branches(prefix, scheme, old_db_branch, new_db_branch):
> + """Move bzr data from an old to a new branch, leaving old stacked on new.
> +
> + T...

Hey Michael,

Not much to say here, given that we've already talked about it. I look forward to seeing this QA'd on staging.

I have a few comments, one or two of which must be fixed before landing. Other than that, looks good. Thanks for getting this done.

jml

> === modified file 'database/schema/security.cfg'
> --- database/schema/security.cfg	2009-10-05 15:07:10 +0000
> +++ database/schema/security.cfg	2009-10-13 09:11:15 +0000
> @@ -641,6 +641,22 @@
>  # Merge notifications
>  public.codereviewvote                   = SELECT
>  
> +[branch-distro]
> +type=user
> +public.branch                    = SELECT, INSERT
> +public.branchsubscription        = SELECT, INSERT
> +public.distribution              = SELECT
> +public.distroseries              = SELECT
> +public.karma                     = SELECT, INSERT
> +public.karmaaction               = SELECT
> +public.person                    = SELECT
> +public.product                   = SELECT
> +public.seriessourcepackagebranch = SELECT, INSERT, DELETE
> +public.sourcepackagename         = SELECT
> +public.teamparticipation         = SELECT
> +public.validpersoncache          = SELECT
> +
> +

How did you determine these? Are you sure you've got everything?

>  [targetnamecacheupdater]
>  type=user
>  groups=script
>

> === added file 'lib/lp/codehosting/branchdistro.py'
> --- lib/lp/codehosting/branchdistro.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/codehosting/branchdistro.py	2009-10-13 09:11:15 +0000
> @@ -0,0 +1,355 @@
> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Opening a new DistroSeries for branch based development.
> +
> +Intended to be run just after a new distro series has been completed, this
> +script will create an official package branch in the new series for every one
> +in the old.  The old branch will become stacked on the new, to avoid a using
> +too much disk space whilst retaining best performance for the new branch.
> +"""
> +
> +import os
> +
> +from bzrlib.branch import Branch
> +from bzrlib.bzrdir import BzrDir
> +from bzrlib.errors import NotBranchError, NotStacked
> +
> +from lazr.uri import URI
> +
> +import transaction
> +
> +from zope.component import getUtility
> +
> +from canonical.launchpad.interfaces import ILaunchpadCelebrities
> +from canonical.config import config
> +
> +from lp.code.interfaces.branchcollection import IAllBranches
> +from lp.code.interfaces.branch import BranchExists
> +from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
> +from lp.code.interfaces.seriessourcepackagebranch import (
> +    IFindOfficialBranchLinks)
> +from lp.code.enums import BranchType
> +from lp.codehosting.vfs import branch_id_to_path
> +from lp.registry.interfaces.distribution import IDistributionSet
> +from lp.registry.interfaces.pocket import PackagePublishingPocket
> +
> +
> +__metaclass__ = type
> +__all__ = []
> +

These should be above the imports & below the docstring, IIRC.

> +
> +def switch_branches(prefix, scheme, old_db_branch, new_db_branch):
> +    """Move bzr data from an old to a new branch, leaving old stacked on new.
> +
> +    This function is intended to be used just after Ubuntu is released to
> +    create (at the bzr level) a new trunk branch for a source package for the
> +    next release of the distribution.  We move the bzr data to the location
> +    for the new branch and replace the trunk branch for the just released
> +    version with a stacked branch pointing at the new branch.
> +
> +    :param prefix: The non-branch id dependent part of the physical path to
> +        the branches on disk.
> +    :param scheme: The branches should be open-able at a URL of the form
> +        ``scheme + :/// + unique_name``.
> +    :param old_db_branch: The branch that currently has the trunk bzr data.
> +    :param old_db_branch: The new trunk branch.  This should not have any
> +        presence on disk yet.
> +    """

Do you think it would be worth documenting failure conditions here, or at
least mentioning something about atomicity?

> +    # Move .bzr directory from old to new location, crashing through the
> +    # abstraction we usually hide our branch locations behind.
> +    old_underlying_path = os.path.join(
> +        prefix, branch_id_to_path(old_db_branch.id))
> +    new_underlying_path = os.path.join(
> +        prefix, branch_id_to_path(new_db_branch.id))
> +    os.makedirs(new_underlying_path)
> +    os.rename(
> +        os.path.join(old_underlying_path, '.bzr'),
> +        os.path.join(new_underlying_path, '.bzr'))
> +
> +    # Create branch at old location -- we use the "clone('null:')" trick to
> +    # preserve the format.  We have to open at the logical, unique_name-based,
> +    # location so that it works to set the stacked on url to '/' + a
> +    # unique_name.
> +    new_location_bzrdir = BzrDir.open(
> +        str(URI(
> +            scheme=scheme, host='', path='/' + new_db_branch.unique_name)))
> +    old_location_bzrdir = new_location_bzrdir.clone(
> +        str(URI(
> +            scheme=scheme, host='', path='/' + old_db_branch.unique_name)),
> +        revision_id='null:')
> +
> +    # Set the stacked on url for old location.
> +    old_location_branch = old_location_bzrdir.open_branch()
> +    old_location_branch.set_stacked_on_url('/' + new_db_branch.unique_name)
> +
> +    # Pull from new location to old -- this won't actually transfer any
> +    # revisions, just update the last revision pointer.
> +    old_location_branch.pull(new_location_bzrdir.open_branch())
> +

In a private email thread, I mentioned that this could well be its own utility
script. I don't know how many people want to move a branch to a new location
and create a new branch at the old location stacked on the old one, but it
seems quite generic.

I don't think it needs to be made into a script in order to land.

> +
> +class DistroBrancher:
> +    """Open a new distroseries for branch based development.
> +
> +    `makeNewBranches` will create an official package branch in the new series
> +    for every one in the old.  `checkNewBranches` will check that a previous
> +    run of this script completed successfully -- this is only likely to be
> +    really useful if a script run died halfway through or had to be killed.
> +    """
> +
> +    def __init__(self, logger, old_distroseries, new_distroseries):
> +        """Construct a `DistroBrancher`.
> +
> +        The old and new distroseries must be from the same distribution, but
> +        not the same distroseries.
> +
> +        :param logger: A Logger.  Problems will be logged to this object at
> +            the WARNING level or higher; progress reports will be logged at
> +            the DEBUG level.
> +        :param old_distroseries: The distroseries that will be examined to
> +            find existing source package branches.
> +        :param new_distroseries: The distroseries that will have new official
> +            source branches made for it.
> +        """
> +        self.logger = logger
> +        if old_distroseries.distribution != new_distroseries.distribution:
> +            raise AssertionError(
> +                "%s and %s are from different distributions!" %
> +                (old_distroseries, new_distroseries))
> +        if old_distroseries == new_distroseries:
> +            raise AssertionError(
> +                "New and old distributions must be different!")
> +        self.old_distroseries = old_distroseries
> +        self.new_distroseries = new_distroseries
> +
> +    @classmethod
> +    def fromNames(cls, logger, distribution_name, old_distroseries_name,
> +                  new_distroseries_name):
> +        """Make a `DistroBrancher` from the names of a distro and two series.
> +        """
> +        distribution = getUtility(IDistributionSet).getByName(
> +            distribution_name)
> +        new_distroseries = distribution.getSeries(new_distroseries_name)
> +        old_distroseries = distribution.getSeries(old_distroseries_name)
> +        return cls(logger, old_distroseries, new_distroseries)
> +
> +    def _existingOfficialBranches(self):
> +        """Return the collection of official branches in the old distroseries.
> +        """
> +        branches = getUtility(IAllBranches)
> +        distroseries_branches = branches.inDistroSeries(self.old_distroseries)
> +        return distroseries_branches.officialBranches().getBranches()
> +
> +    def checkConsistentOfficialPackageBranch(self, db_branch):
> +        """Check that `db_branch` is a consistent official package branch.
> +
> +        'Consistent official package branch' means:
> +
> +         * It's a package branch (rather than a personal or junk branch).
> +         * It's official for its SourcePackage and no other.
> +
> +        This function simply returns True or False -- any problems will be
> +        logged to ``self.logger``.
> +
> +        :param db_branch: The `IBranch` to check.
> +        :return: ``True`` if the branch is a consistent official package
> +            branch, ``False`` otherwise.
> +        """
> +        if db_branch.product:
> +            self.logger.warning(
> +                "Encountered unexpected product branch %r",
> +                db_branch.unique_name)
> +            return False
> +        if not db_branch.distroseries:
> +            self.logger.warning(
> +                "Encountered unexpected personal branch %s",
> +                db_branch.unique_name)
> +            return False
> +        find_branch_links = getUtility(IFindOfficialBranchLinks)
> +        links = list(find_branch_links.findForBranch(db_branch))
> +        if len(links) == 0:
> +            self.logger.warning(
> +                "%s is not an official branch", db_branch.unique_name)
> +            return False
> +        elif len(links) > 1:
> +            series_text = ', '.join([
> +                link.sourcepackage.path for link in links])
> +            self.logger.warning(
> +                "%s is official for multiple series: %s",
> +                db_branch.unique_name, series_text)
> +            return False
> +        elif links[0].sourcepackage != db_branch.sourcepackage:
> +            self.logger.warning(
> +                "%s is the official branch for %s but not its "
> +                "sourcepackage", db_branch.unique_name,
> +                links[0].sourcepackage.path)
> +            return False
> +        return True
> +
> +    def makeNewBranches(self):
> +        """Make official branches in the new distroseries."""
> +        for db_branch in self._existingOfficialBranches():
> +            self.logger.debug("Processing %s" % db_branch.unique_name)
> +            try:
> +                self.makeOneNewBranch(db_branch)
> +            except BranchExists:
> +                # Check here?

Check what here, exactly?

> +                pass
> +
> +    def checkNewBranches(self):
> +        """Check the branches in the new distroseries are present and correct.
> +
> +        This function checks that every official package branch in the old
> +        distroseries has a matching branch in the new distroseries and that
> +        stacking is set up as we expect in both the hosted and mirrored areas
> +        on disk.
> +
> +        This function simply returns True or False -- any problems will be
> +        logged to ``self.logger``.
> +
> +        :return: ``True`` if every branch passes the check, ``False``
> +            otherwise.
> +        """

Perhaps its worth mentioning that it goes through every branch, regardless of
whether any fail.

> +        ok = True
> +        for db_branch in self._existingOfficialBranches():
> +            self.logger.debug("Checking %s" % db_branch.unique_name)
> +            try:
> +                if not self.checkOneBranch(db_branch):
> +                    ok = False
> +            except:
> +                ok = False
> +                self.logger.exception(
> +                    "Unexpected error checking %s!", db_branch)
> +        return ok
> +
> +    def checkOneBranch(self, old_db_branch):
> +        """Check a branch in the old distroseries has been copied to the new.
> +
> +        This function checks that `old_db_branch` has a matching branch in the
> +        new distroseries and that stacking is set up as we expect in both the
> +        hosted and mirrored areas on disk.
> +
> +        This function simply returns True or False -- any problems will be
> +        logged to ``self.logger``.
> +
> +        :param old_db_branch: The branch to check.
> +        :return: ``True`` if the branch passes the check, ``False`` otherwise.
> +        """
> +        ok = self.checkConsistentOfficialPackageBranch(old_db_branch)
> +        if not ok:
> +            return ok
> +        new_sourcepackage = self.new_distroseries.getSourcePackage(
> +            old_db_branch.sourcepackagename)
> +        new_db_branch = new_sourcepackage.getBranch(
> +            PackagePublishingPocket.RELEASE)
> +        if new_db_branch is None:
> +            self.logger.warning(
> +                "No official branch found for %s",
> +                new_sourcepackage.path)
> +            return False
> +        ok = self.checkConsistentOfficialPackageBranch(new_db_branch)
> +        if not ok:
> +            return ok
> +        # for both mirrored and hosted areas:
> +        for scheme in 'lp-mirrored', 'lp-hosted':
> +            # the branch in the new distroseries is unstacked
> +            new_location = str(URI(
> +                scheme=scheme, host='', path='/' + new_db_branch.unique_name))
> +            try:
> +                new_bzr_branch = Branch.open(new_location)
> +            except NotBranchError:
> +                self.logger.warning(
> +                    "No bzr branch at new location %s", new_location)
> +                ok = False
> +            else:
> +                try:
> +                    new_stacked_on_url = new_bzr_branch.get_stacked_on_url()
> +                    ok = False
> +                    self.logger.warning(
> +                        "New branch at %s is stacked on %s, should be "
> +                        "unstacked.", new_location, new_stacked_on_url)
> +                except NotStacked:
> +                    pass
> +            # The branch in the old distroseries is stacked on that in the
> +            # new.
> +            old_location = str(URI(
> +                scheme=scheme, host='', path='/' + old_db_branch.unique_name))
> +            try:
> +                old_bzr_branch = Branch.open(old_location)
> +            except NotBranchError:
> +                self.logger.warning(
> +                    "No bzr branch at old location %s", old_location)
> +                ok = False
> +            else:
> +                try:
> +                    old_stacked_on_url = old_bzr_branch.get_stacked_on_url()
> +                    if old_stacked_on_url != '/' + new_db_branch.unique_name:
> +                        self.logger.warning(
> +                            "Old branch at %s is stacked on %s, should be "
> +                            "stacked on %s", old_location, old_stacked_on_url,
> +                            '/' + new_db_branch.unique_name)
> +                        ok = False
> +                except NotStacked:
> +                    self.logger.warning(
> +                        "Old branch at %s is not stacked, should be stacked "
> +                        "on %s", old_location,
> +                        '/' + new_db_branch.unique_name)
> +                    ok = False
> +                # The branch in the old distroseries has no revisions in its
> +                # repository.  We open the repository independently of the
> +                # branch because the branch's repository has had its fallback
> +                # location activated. Note that this check might fail if new
> +                # revisions get pushed to the branch in the old distroseries,
> +                # which shouldn't happen but isn't totally impossible.
> +                old_repo = BzrDir.open(old_location).open_repository()
> +                if len(old_repo.all_revision_ids()) > 0:
> +                    self.logger.warning(
> +                        "Repository at %s has %s revisions.",
> +                        old_location, len(old_repo.all_revision_ids()))
> +                    ok = False
> +                # The branch in the old distroseries has at least some
> +                # history.  (We can't check that the tips are the same because
> +                # the branch in the new distroseries might have new revisons).
> +                if old_bzr_branch.last_revision() == 'null:':
> +                    self.logger.warning(
> +                        "Old branch at %s has null tip revision.",
> +                        old_location)
> +                    ok = False
> +        return ok
> +
> +    def makeOneNewBranch(self, old_db_branch):
> +        """Copy a branch to the new distroseries.
> +
> +        This function makes a new database branch for the same source package
> +        as old_db_branch but in the new distroseries and then uses
> +        `switch_branches` to move the underlying bzr branch to the new series
> +        and replace the old branch with a branch stacked on the new series'
> +        branch.
> +
> +        :param old_db_branch: The branch to copy into the new distroseries.
> +        :raises BranchExists: This will be raised if old_db_branch has already
> +            been copied to the new distroseries (in the database, at least).
> +        """
> +        if not self.checkConsistentOfficialPackageBranch(old_db_branch):
> +            self.logger.warning("Skipping branch")
> +            return
> +        new_namespace = getUtility(IBranchNamespaceSet).get(
> +            person=old_db_branch.owner, product=None,
> +            distroseries=self.new_distroseries,
> +            sourcepackagename=old_db_branch.sourcepackagename)
> +        new_db_branch = new_namespace.createBranch(
> +            BranchType.HOSTED, old_db_branch.name, old_db_branch.registrant)
> +        new_db_branch.sourcepackage.setBranch(
> +            PackagePublishingPocket.RELEASE, new_db_branch,
> +            getUtility(ILaunchpadCelebrities).ubuntu_branches.teamowner)
> +        # Commit now because switch_branches *moves* the data to locations
> +        # dependent on the new_branch's id, so if the transaction doesn't get
> +        # committed we won't know where it's gone.

I don't quite follow this. Why won't we know where it has gone?

> +        transaction.commit()
> +        switch_branches(
> +            config.codehosting.hosted_branches_root,
> +            'lp-hosted', old_db_branch, new_db_branch)
> +        switch_branches(
> +            config.codehosting.mirrored_branches_root,
> +            'lp-mirrored', old_db_branch, new_db_branch)
> +        return new_db_branch
>

> === added file 'lib/lp/codehosting/tests/test_branchdistro.py'
> --- lib/lp/codehosting/tests/test_branchdistro.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/codehosting/tests/test_branchdistro.py	2009-10-13 09:11:15 +0000
> @@ -0,0 +1,690 @@
> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Tests for making new source package branches just after a distro release.
> +"""
> +
> +__metaclass__ = type
> +
> +import os
> +import re
> +from StringIO import StringIO
> +from subprocess import PIPE, Popen, STDOUT
> +import textwrap
> +import unittest
> +
> +from bzrlib.branch import Branch
> +from bzrlib.bzrdir import BzrDir
> +from bzrlib.errors import NotStacked
> +from bzrlib.tests import TestCaseWithTransport
> +from bzrlib.transport import get_transport
> +from bzrlib.transport.chroot import ChrootServer
> +
> +from lazr.uri import URI
> +
> +import transaction
> +
> +from canonical.config import config
> +from canonical.testing.layers import ZopelessAppServerLayer
> +from canonical.launchpad.scripts.logger import FakeLogger, QuietFakeLogger
> +
> +from lp.codehosting.branchdistro import DistroBrancher, switch_branches
> +from lp.codehosting.vfs import branch_id_to_path
> +from lp.registry.interfaces.pocket import PackagePublishingPocket
> +from lp.testing import TestCaseWithFactory
> +
> +
> +# We say "RELEASE" often enough to not want to say "PackagePublishingPocket."
> +# each time.
> +RELEASE = PackagePublishingPocket.RELEASE
> +
> +
> +class FakeBranch:
> +    """Just enough of a Branch to pass `test_switch_branches`."""
> +
> +    def __init__(self, id):
> +        self.id = id
> +
> +    @property
> +    def unique_name(self):
> +        return branch_id_to_path(self.id)
> +
> +
> +class TestSwitchBranches(TestCaseWithTransport):
> +    """Tests for `switch_branches`."""
> +
> +    def test_switch_branches(self):
> +        # switch_branches moves a branch to the new location and places a
> +        # branch (with no revisions) stacked on the new branch in the old
> +        # location.
> +
> +        chroot_server = ChrootServer(self.get_transport())
> +        chroot_server.setUp()
> +        self.addCleanup(chroot_server.tearDown)
> +        scheme = chroot_server.get_url().rstrip('/:')
> +
> +        old_branch = FakeBranch(1)
> +        self.get_transport(old_branch.unique_name).create_prefix()
> +        tree = self.make_branch_and_tree(old_branch.unique_name)
> +        tree.commit(message='.')
> +
> +        new_branch = FakeBranch(2)
> +
> +        switch_branches('.', scheme, old_branch, new_branch)
> +
> +        # Post conditions:
> +        # 1. unstacked branch in new_branch's location
> +        # 2. stacked branch with no revisions in repo at old_branch
> +        # 3. last_revision() the same for two branches
> +
> +        old_location_bzrdir = BzrDir.open(str(URI(
> +            scheme=scheme, host='', path='/' + old_branch.unique_name)))
> +        new_location_bzrdir = BzrDir.open(str(URI(
> +            scheme=scheme, host='', path='/' + new_branch.unique_name)))
> +
> +        old_location_branch = old_location_bzrdir.open_branch()
> +        new_location_branch = new_location_bzrdir.open_branch()
> +
> +        # 1. unstacked branch in new_branch's location
> +        self.assertRaises(NotStacked, new_location_branch.get_stacked_on_url)
> +
> +        # 2. stacked branch with no revisions in repo at old_branch
> +        self.assertEqual(
> +            '/' + new_branch.unique_name,
> +            old_location_branch.get_stacked_on_url())
> +        self.assertEqual(
> +            [], old_location_bzrdir.open_repository().all_revision_ids())
> +
> +        # 3. last_revision() the same for two branches
> +        self.assertEqual(
> +            old_location_branch.last_revision(),
> +            new_location_branch.last_revision())
> +
> +
> +class TestDistroBrancher(TestCaseWithFactory):
> +    """Tests for `DistroBrancher`."""
> +
> +    layer = ZopelessAppServerLayer
> +
> +    def setUp(self):
> +        TestCaseWithFactory.setUp(self)
> +        self.useBzrBranches(real_server=True)
> +
> +    def makeOfficialPackageBranch(self, distroseries=None):
> +        """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)
> +
> +        transaction.commit()
> +
> +        _, tree = self.create_branch_and_tree(
> +            tree_location=self.factory.getUniqueString(), db_branch=db_branch,
> +            hosted=True)
> +        tree.commit('')
> +        mirrored_branch = BzrDir.create_branch_convenience(
> +            db_branch.warehouse_url)
> +        mirrored_branch.pull(tree.branch)
> +
> +        return db_branch
> +
> +    def makeNewSeriesAndBrancher(self, distroseries=None):
> +        """Make a DistroBrancher.
> +
> +        Any messages logged by this DistroBrancher can be checked by calling
> +        `assertLogMessages` below.
> +        """
> +        if distroseries is None:
> +            distroseries = self.factory.makeDistroRelease()
> +        self._log_file = StringIO()
> +        new_distroseries = self.factory.makeDistroRelease(
> +            distribution=distroseries.distribution, name='new')
> +        transaction.commit()
> +        self.layer.switchDbUser('branch-distro')
> +        return DistroBrancher(
> +            FakeLogger(self._log_file), distroseries, new_distroseries)
> +
> +    def clearLogMessages(self):
> +        """Forget about all logged messages seen so far."""
> +        self._log_file.seek(0, 0)
> +        self._log_file.truncate()
> +
> +    def assertLogMessages(self, patterns):
> +        """Assert that the messages logged meet expectations.
> +
> +        :param patterns: A list of regular expressions.  The length must match
> +            the number of messages logged, and then each pattern must match
> +            the messages logged in order.
> +        """
> +        log_messages = self._log_file.getvalue().splitlines()
> +        if len(log_messages) > len(patterns):
> +            self.fail(
> +                "More log messages (%s) than expected (%s)" %
> +                (log_messages, patterns))
> +        elif len(log_messages) < len(patterns):
> +            self.fail(
> +                "Fewer log messages (%s) than expected (%s)" %
> +                (log_messages, patterns))
> +        for pattern, message in zip(patterns, log_messages):
> +            if not re.match(pattern, message):
> +                self.fail("%r does not match %r" % (pattern, message))
> +
> +    def test_DistroBrancher_same_distro_check(self):
> +        # DistroBrancher.__init__ raises AssertionError if the two
> +        # distroseries passed are not from the same distribution.
> +        self.assertRaises(
> +            AssertionError, DistroBrancher, None,
> +            self.factory.makeDistroRelease(),
> +            self.factory.makeDistroRelease())
> +
> +    def test_DistroBrancher_same_distroseries_check(self):
> +        # DistroBrancher.__init__ raises AssertionError if passed the same
> +        # distroseries twice.
> +        distroseries = self.factory.makeDistroRelease()
> +        self.assertRaises(
> +            AssertionError, DistroBrancher, None, distroseries, distroseries)
> +
> +    def test_fromNames(self):
> +        # DistroBrancher.__init__ raises AssertionError if passed the same
> +        # distroseries twice.

The comment here is incorrect.

> +        distribution = self.factory.makeDistribution()
> +        distroseries1 = self.factory.makeDistroRelease(
> +            distribution=distribution)
> +        distroseries2 = self.factory.makeDistroRelease(
> +            distribution=distribution)
> +        brancher = DistroBrancher.fromNames(
> +            None, distribution.name, distroseries1.name, distroseries2.name)
> +        self.assertEqual(
> +            [distroseries1, distroseries2],
> +            [brancher.old_distroseries, brancher.new_distroseries])
> +
> +    # A word on testing strategy: we don't directly test the post conditions
> +    # of makeOneNewBranch, but we do test that it satisfies checkOneBranch and
> +    # the tests for checkOneBranch verify that this function rejects various
> +    # ways in which makeOneNewBranch could conceivably fail.
> +

Thanks, this is a helpful comment.

> +    def test_makeOneNewBranch(self):
> +        # makeOneNewBranch creates an official package branch in the new
> +        # distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +
> +        new_branch = brancher.new_distroseries.getSourcePackage(
> +            db_branch.sourcepackage.name).getBranch(RELEASE)
> +
> +        self.assertIsNot(new_branch, None)
> +

Wouldn't it be nice if there were an explicit ordering for 'expected' and
'observed' :)

I notice you don't make any assertions about the name, owner or target of the
branch. I wonder if these are checked later...

> +    def test_makeOneNewBranch_inconsistent_branch(self):
> +        # makeOneNewBranch skips over an inconsistent official package branch
> +        # (see `checkConsistentOfficialPackageBranch` for precisely what an
> +        # "inconsistent official package branch" is).
> +        unofficial_branch = self.factory.makePackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(
> +            unofficial_branch.distroseries)
> +        brancher.makeOneNewBranch(unofficial_branch)
> +
> +        new_branch = brancher.new_distroseries.getSourcePackage(
> +            unofficial_branch.sourcepackage.name).getBranch(RELEASE)
> +        self.assertIs(new_branch, None)
> +        self.assertLogMessages(
> +            ['^WARNING .*',
> +             '^WARNING Skipping branch'])
> +

What's the first warning for?

> +    def test_makeNewBranches(self):
> +        # makeNewBranches calls makeOneNewBranch for each official branch in
> +        # the old distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        db_branch2 = self.makeOfficialPackageBranch(
> +            distroseries=db_branch.distroseries)
> +
> +        new_distroseries = self.factory.makeDistroRelease(
> +            distribution=db_branch.distribution)
> +
> +        brancher = DistroBrancher(
> +            QuietFakeLogger(), db_branch.distroseries, new_distroseries)
> +
> +        brancher.makeNewBranches()
> +
> +        new_sourcepackage = new_distroseries.getSourcePackage(
> +            db_branch.sourcepackage.name)
> +        new_branch = new_sourcepackage.getBranch(RELEASE)
> +        new_sourcepackage2 = new_distroseries.getSourcePackage(
> +            db_branch2.sourcepackage.name)
> +        new_branch2 = new_sourcepackage2.getBranch(RELEASE)
> +
> +        self.assertIsNot(new_branch, None)
> +        self.assertIsNot(new_branch2, None)
> +
> +    def test_makeNewBranches_idempotent(self):
> +        # makeNewBranches is idempotent in the sense that if a branch in the
> +        # old distroseries already has a counterpart in the new distroseries,
> +        # it is silently ignored.
> +        db_branch = self.makeOfficialPackageBranch()
> +
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeNewBranches()
> +        brancher.makeNewBranches()
> +
> +        new_branch = brancher.new_distroseries.getSourcePackage(
> +            db_branch.sourcepackage.name).getBranch(RELEASE)
> +
> +        self.assertIsNot(new_branch, None)
> +
> +    def test_makeOneNewBranch_checks_ok(self):
> +        # After calling makeOneNewBranch for a branch, calling checkOneBranch
> +        # returns True for that branch.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        self.clearLogMessages()
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([])
> +        self.assertTrue(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_product_branch(self):
> +        # checkConsistentOfficialPackageBranch returns False when passed a
> +        # product branch.
> +        db_branch = self.factory.makeProductBranch()
> +        brancher = self.makeNewSeriesAndBrancher()
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Encountered unexpected product branch .*/.*/.*$'])
> +        self.assertFalse(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_personal_branch(self):
> +        # checkConsistentOfficialPackageBranch returns False when passed a
> +        # personal branch.
> +        db_branch = self.factory.makePersonalBranch()
> +        brancher = self.makeNewSeriesAndBrancher()
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Encountered unexpected personal branch .*/.*/.*$'])
> +        self.assertFalse(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_no_official_branch(self):
> +        # checkConsistentOfficialPackageBranch returns False when passed a
> +        # branch which is not official for any package.
> +        db_branch = self.factory.makePackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages(
> +            ['^WARNING .*/.*/.*\ is not an official branch$'])
> +        self.assertFalse(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_official_elsewhere(self):
> +        # checkConsistentOfficialPackageBranch returns False when passed a
> +        # branch which is official for a sourcepackage that it is not a branch
> +        # for.
> +        db_branch = self.factory.makePackageBranch()
> +        self.factory.makeSourcePackage().setBranch(
> +            RELEASE, db_branch, db_branch.owner)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages(
> +            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
> +             'its sourcepackage$'])
> +        self.assertFalse(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_official_twice(self):
> +        # checkConsistentOfficialPackageBranch returns False when passed a
> +        # branch that is official for two sourcepackages.
> +        db_branch = self.factory.makePackageBranch()
> +        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
> +        self.factory.makeSourcePackage().setBranch(
> +            RELEASE, db_branch, db_branch.owner)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING .*/.*/.* is official for multiple series: .*/.*/.*, '
> +            '.*/.*/.*$'])
> +        self.assertFalse(ok)
> +
> +    def test_checkConsistentOfficialPackageBranch_ok(self):
> +        # checkConsistentOfficialPackageBranch returns True when passed a
> +        # branch that is official for its sourcepackage and no other.
> +        db_branch = self.factory.makePackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
> +        self.assertLogMessages([])
> +        self.assertTrue(ok)
> +
> +    def test_checkOneBranch_inconsistent_old_package_branch(self):
> +        # checkOneBranch returns False when passed a branch that is not a
> +        # consistent official package branch.
> +        db_branch = self.factory.makePackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher()
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages(
> +            ['^WARNING .*/.*/.* is not an official branch$'])
> +
> +    def test_checkOneBranch_no_new_official_branch(self):
> +        # checkOneBranch returns False when there is no corresponding official
> +        # package branch in the new distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages(
> +            ['^WARNING No official branch found for .*/.*/.*$'])
> +
> +    def test_checkOneBranch_inconsistent_new_package_branch(self):
> +        # checkOneBranch returns False when the corresponding official package
> +        # branch in the new distroseries is not consistent.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
> +        self.layer.switchDbUser('launchpad')
> +        new_db_branch.setTarget(
> +            new_db_branch.owner,
> +            source_package=self.factory.makeSourcePackage())
> +        transaction.commit()
> +        self.layer.switchDbUser('branch-distro')
> +        ok = brancher.checkOneBranch(new_db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages(
> +            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
> +             'its sourcepackage$'])
> +
> +    # All these hosted/mirrored tests are very repetitive, perhaps some meta
> +    # programming would reduce LOC count, but maybe make things harder to
> +    # understand.
> +

It's not just the LOC count, it's the duplicated logic. We never want hosted &
mirrored to differ in behaviour here.

Did you try a metaprogramming approach? Was it really that much harder to
follow?

> +    def test_checkOneBranch_new_hosted_branch_missing(self):
> +        # checkOneBranch returns False when there is no bzr branch in the
> +        # hosted area for the database branch in the new distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
> +        get_transport(new_db_branch.getPullURL()).delete_tree('.bzr')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        # Deleting the new hosted branch will break the old branch, as that's
> +        # stacked on the new one.
> +        self.assertLogMessages([
> +            '^WARNING No bzr branch at new location '
> +            'lp-hosted:///.*/.*/.*/.*$',
> +            '^WARNING No bzr branch at old location '
> +            'lp-hosted:///.*/.*/.*/.*$',
> +            ])
> +
> +    def test_checkOneBranch_new_mirrored_branch_missing(self):
> +        # checkOneBranch returns False when there is no bzr branch in the
> +        # mirrored area for the database branch in the new distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
> +        get_transport(new_db_branch.warehouse_url).delete_tree('.bzr')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        # Deleting the new mirrored branch will break the old branch, as
> +        # that's stacked on the new one.
> +        self.assertLogMessages([
> +            '^WARNING No bzr branch at new location '
> +            'lp-mirrored:///.*/.*/.*/.*$',
> +            '^WARNING No bzr branch at old location '
> +            'lp-mirrored:///.*/.*/.*/.*$',
> +            ])
> +
> +    def test_checkOneBranch_old_hosted_branch_missing(self):
> +        # checkOneBranch returns False when there is no bzr branch in the
> +        # hosted area for the database branch in old distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        get_transport(db_branch.getPullURL()).delete_tree('.bzr')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages([
> +            '^WARNING No bzr branch at old location '
> +            'lp-hosted:///.*/.*/.*/.*$',
> +            ])
> +
> +    def test_checkOneBranch_old_mirrored_branch_missing(self):
> +        # checkOneBranch returns False when there is no bzr branch in the
> +        # mirrored area for the database branch in old distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        get_transport(db_branch.warehouse_url).delete_tree('.bzr')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages([
> +            '^WARNING No bzr branch at old location '
> +            'lp-mirrored:///.*/.*/.*/.*$',
> +            ])
> +
> +    def test_checkOneBranch_new_hosted_stacked(self):
> +        # checkOneBranch returns False when the bzr branch in the hosted area
> +        # for the database branch in new distroseries is stacked.
> +        db_branch = self.makeOfficialPackageBranch()
> +        b, _ = self.create_branch_and_tree(
> +            self.factory.getUniqueString(), hosted=True)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
> +        Branch.open(new_db_branch.getPullURL()).set_stacked_on_url(
> +            '/' + b.unique_name)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages([
> +            '^WARNING New branch at lp-hosted:///.*/.*/.*/.* is stacked on '
> +            '/.*/.*/.*, should be unstacked.$',
> +            ])
> +
> +    def test_checkOneBranch_new_mirrored_stacked(self):
> +        # checkOneBranch returns False when the bzr branch in the mirrored
> +        # area for the database branch in new distroseries is stacked.
> +        db_branch = self.makeOfficialPackageBranch()
> +        b, _ = self.create_branch_and_tree(
> +            self.factory.getUniqueString(), hosted=False)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
> +        Branch.open(new_db_branch.warehouse_url).set_stacked_on_url(
> +            '/' + b.unique_name)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertFalse(ok)
> +        self.assertLogMessages([
> +            '^WARNING New branch at lp-mirrored:///.*/.*/.*/.* is stacked on '
> +            '/.*/.*/.*, should be unstacked.$',
> +            ])
> +
> +    def test_checkOneBranch_old_hosted_unstacked(self):
> +        # checkOneBranch returns False when the bzr branch in the hosted area
> +        # for the database branch in old distroseries is not stacked.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
> +        old_hosted_bzr_branch.set_stacked_on_url(None)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* is not '
> +            'stacked, should be stacked on /.*/.*/.*.$',
> +            '^.*has .* revisions.*$',
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_mirrored_unstacked(self):
> +        # checkOneBranch returns False when the bzr branch in the mirrored
> +        # area for the database branch in old distroseries is not stacked.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
> +        old_mirrored_bzr_branch.set_stacked_on_url(None)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* is not '
> +            'stacked, should be stacked on /.*/.*/.*.$',
> +            '^.*has .* revisions.*$',
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_hosted_misstacked(self):
> +        # checkOneBranch returns False when the bzr branch in the hosted area
> +        # for the database branch in old distroseries stacked on some other
> +        # branch than the branch in the new distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        b, _ = self.create_branch_and_tree(
> +            self.factory.getUniqueString(), hosted=True)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        Branch.open(db_branch.getPullURL()).set_stacked_on_url(
> +            '/' + b.unique_name)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* is stacked on '
> +            '/.*/.*/.*, should be stacked on /.*/.*/.*.$',
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_mirrored_misstacked(self):
> +        # checkOneBranch returns False when the bzr branch in the mirrored
> +        # area for the database branch in old distroseries stacked on some
> +        # other branch than the branch in the new distroseries.
> +        db_branch = self.makeOfficialPackageBranch()
> +        b, _ = self.create_branch_and_tree(
> +            self.factory.getUniqueString(), hosted=False)
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        Branch.open(db_branch.warehouse_url).set_stacked_on_url(
> +            '/' + b.unique_name)
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* is stacked on '
> +            '/.*/.*/.*, should be stacked on /.*/.*/.*.$',
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_hosted_has_revisions(self):
> +        # checkOneBranch returns False when the bzr branch in the hosted area
> +        # for the database branch in old distroseries has a repository that
> +        # contains revisions.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
> +        old_hosted_bzr_branch.create_checkout(
> +            self.factory.getUniqueString()).commit('')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Repository at lp-hosted:///.*/.*/.*/.* has 1 revisions.'
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_mirrored_has_revisions(self):
> +        # checkOneBranch returns False when the bzr branch in the mirrored
> +        # area for the database branch in old distroseries has a repository
> +        # that contains revisions.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
> +        old_mirrored_bzr_branch.create_checkout(
> +            self.factory.getUniqueString()).commit('')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Repository at lp-mirrored:///.*/.*/.*/.* has 1 '
> +            'revisions.'
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_hosted_has_null_tip(self):
> +        # checkOneBranch returns False when the bzr branch in the hosted area
> +        # for the database branch in old distroseries has tip revision of
> +        # 'null:'.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
> +        old_hosted_bzr_branch.set_last_revision_info(0, 'null:')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* has null tip '
> +            'revision.'
> +            ])
> +        self.assertFalse(ok)
> +
> +    def test_checkOneBranch_old_mirrored_has_null_tip(self):
> +        # checkOneBranch returns False when the bzr branch in the mirrored
> +        # area for the database branch in old distroseries has tip revision of
> +        # 'null:'.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeOneNewBranch(db_branch)
> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
> +        old_mirrored_bzr_branch.set_last_revision_info(0, 'null:')
> +        ok = brancher.checkOneBranch(db_branch)
> +        self.assertLogMessages([
> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* has null tip '
> +            'revision.'
> +            ])
> +        self.assertFalse(ok)
> +
> +    def runBranchDistroScript(self, args):
> +        """Run the branch-distro.py script with the given arguments.
> +
> +        ;param args: The arguments to pass to the branch-distro.py script.
> +        :return: A tuple (returncode, output).  stderr and stdout are both
> +            contained in the output.
> +        """
> +        script_path = os.path.join(config.root, 'scripts', 'branch-distro.py')
> +        process = Popen([script_path] + args, stdout=PIPE, stderr=STDOUT)
> +        output, error = process.communicate()
> +        return process.returncode, output
> +
> +    def test_makeNewBranches_script(self):
> +        # Running the script with the arguments 'distro old-series new-series'
> +        # makes new branches in the new series.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        returncode, output = self.runBranchDistroScript(
> +            ['-v', db_branch.distribution.name,
> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
> +        self.assertEqual(0, returncode)
> +        self.assertEqual(
> +            'DEBUG   Processing ' + db_branch.unique_name + '\n', output)
> +        brancher.checkOneBranch(db_branch)
> +
> +    def test_checkNewBranches_script_success(self):
> +        # Running the script with the arguments '--check distro old-series
> +        # new-series' checks that the branches in the new series are as
> +        # expected.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        brancher.makeNewBranches()
> +        returncode, output = self.runBranchDistroScript(
> +            ['-v', '--check', db_branch.distribution.name,
> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
> +        self.assertEqual(0, returncode)
> +        self.assertEqual(
> +            'DEBUG   Checking ' + db_branch.unique_name + '\n', output)
> +        brancher.checkOneBranch(db_branch)
> +
> +    def test_checkNewBranches_script_failure(self):
> +        # Running the script with the arguments '--check distro old-series
> +        # new-series' checks that the branches in the new series are as
> +        # expected and logs warnings and exits with code 1 is things are not
> +        # as expected.
> +        db_branch = self.makeOfficialPackageBranch()
> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
> +        returncode, output = self.runBranchDistroScript(
> +            ['-v', '--check', db_branch.distribution.name,
> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
> +        sp_path = brancher.new_distroseries.getSourcePackage(
> +            db_branch.sourcepackagename).path
> +        expected = '''\
> +        DEBUG   Checking %(branch_name)s
> +        WARNING No official branch found for %(sp_path)s
> +        ERROR   Check failed
> +        ''' % {'branch_name': db_branch.unique_name, 'sp_path': sp_path}
> +        self.assertEqual(
> +            textwrap.dedent(expected), output)
> +        self.assertEqual(1, returncode)
> +
> +
> +def test_suite():
> +    return unittest.TestLoader().loadTestsFromName(__name__)
> +
>

That's a lot of test case. :)

> === added file 'scripts/branch-distro.py'
> --- scripts/branch-distro.py	1970-01-01 00:00:00 +0000
> +++ scripts/branch-distro.py	2009-10-13 09:11:15 +0000
> @@ -0,0 +1,38 @@
> +#!/usr/bin/python2.4
> +#
> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +import _pythonpath
> +
> +from lp.codehosting.branchdistro import DistroBrancher
> +from lp.codehosting.vfs import get_multi_server
> +from lp.services.scripts.base import LaunchpadScript, LaunchpadScriptFailure
> +
> +
> +class BranchDistroScript(LaunchpadScript):

Blank line here please.

> +    usage = "%prog distro old-series new-series"
> +
> +    def add_my_options(self):
> +        self.parser.add_option(
> +            '--check', dest="check", action="store_true", default=False,
> +            help=("Check that the new distro series has its official "
> +                  "branches set up correctly."))
> +
> +    def main(self):
> +        if len(self.args) != 3:
> +            self.parser.error("Wrong number of arguments.")
> +        brancher = DistroBrancher.fromNames(self.logger, *self.args)
> +        server = get_multi_server(write_mirrored=True, write_hosted=True)
> +        server.setUp()
> +        try:
> +            if self.options.check:
> +                if not brancher.checkNewBranches():
> +                    raise LaunchpadScriptFailure("Check failed")
> +            else:
> +                brancher.makeNewBranches()
> +        finally:
> +            server.tearDown()
> +
> +if __name__ == '__main__':
> +    BranchDistroScript("branch-distro", dbuser='branch-distro').run()
>

review: Approve

Revision history for this message

Michael Hudson-Doyle (mwhudson) wrote on 2009-10-14:

Download full text (56.0 KiB)

Jonathan Lange wrote:
> Review: Approve
> Hey Michael,
>
> Not much to say here, given that we've already talked about it. I look forward to seeing this QA'd on staging.
>
> I have a few comments, one or two of which must be fixed before landing. Other than that, looks good. Thanks for getting this done.
>
> jml
>
>> === modified file 'database/schema/security.cfg'
>> --- database/schema/security.cfg 2009-10-05 15:07:10 +0000
>> +++ database/schema/security.cfg 2009-10-13 09:11:15 +0000
>> @@ -641,6 +641,22 @@
>> # Merge notifications
>> public.codereviewvote = SELECT
>>
>> +[branch-distro]
>> +type=user
>> +public.branch = SELECT, INSERT
>> +public.branchsubscription = SELECT, INSERT
>> +public.distribution = SELECT
>> +public.distroseries = SELECT
>> +public.karma = SELECT, INSERT
>> +public.karmaaction = SELECT
>> +public.person = SELECT
>> +public.product = SELECT
>> +public.seriessourcepackagebranch = SELECT, INSERT, DELETE
>> +public.sourcepackagename = SELECT
>> +public.teamparticipation = SELECT
>> +public.validpersoncache = SELECT
>> +
>> +
>
> How did you determine these?

The tests run as this user, so I ran make schema over and over and over
again whilst gradually increasing the permissions...

> Are you sure you've got everything?

Reasonably. As you noticed, I spent a long time writing tests...

>> [targetnamecacheupdater]
>> type=user
>> groups=script
>>
>
>
>> === added file 'lib/lp/codehosting/branchdistro.py'
>> --- lib/lp/codehosting/branchdistro.py 1970-01-01 00:00:00 +0000
>> +++ lib/lp/codehosting/branchdistro.py 2009-10-13 09:11:15 +0000
>> @@ -0,0 +1,355 @@
>> +# Copyright 2009 Canonical Ltd. This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Opening a new DistroSeries for branch based development.
>> +
>> +Intended to be run just after a new distro series has been completed, this
>> +script will create an official package branch in the new series for every one
>> +in the old. The old branch will become stacked on the new, to avoid a using
>> +too much disk space whilst retaining best performance for the new branch.
>> +"""
>> +
>> +import os
>> +
>> +from bzrlib.branch import Branch
>> +from bzrlib.bzrdir import BzrDir
>> +from bzrlib.errors import NotBranchError, NotStacked
>> +
>> +from lazr.uri import URI
>> +
>> +import transaction
>> +
>> +from zope.component import getUtility
>> +
>> +from canonical.launchpad.interfaces import ILaunchpadCelebrities
>> +from canonical.config import config
>> +
>> +from lp.code.interfaces.branchcollection import IAllBranches
>> +from lp.code.interfaces.branch import BranchExists
>> +from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
>> +from lp.code.interfaces.seriessourcepackagebranch import (
>> + IFindOfficialBranchLinks)
>> +from lp.code.enums import BranchType
>> +from lp.codehosting.vfs import branch_id_to_path
>> +from lp.registry.interfaces.distribution import IDistributionSet
>> +from lp.registry.interfa...

Jonathan Lange wrote:
> Review: Approve
> Hey Michael,
> 
> Not much to say here, given that we've already talked about it. I look forward to seeing this QA'd on staging.
> 
> I have a few comments, one or two of which must be fixed before landing. Other than that, looks good. Thanks for getting this done.
> 
> jml
> 
>> === modified file 'database/schema/security.cfg'
>> --- database/schema/security.cfg	2009-10-05 15:07:10 +0000
>> +++ database/schema/security.cfg	2009-10-13 09:11:15 +0000
>> @@ -641,6 +641,22 @@
>>  # Merge notifications
>>  public.codereviewvote                   = SELECT
>>  
>> +[branch-distro]
>> +type=user
>> +public.branch                    = SELECT, INSERT
>> +public.branchsubscription        = SELECT, INSERT
>> +public.distribution              = SELECT
>> +public.distroseries              = SELECT
>> +public.karma                     = SELECT, INSERT
>> +public.karmaaction               = SELECT
>> +public.person                    = SELECT
>> +public.product                   = SELECT
>> +public.seriessourcepackagebranch = SELECT, INSERT, DELETE
>> +public.sourcepackagename         = SELECT
>> +public.teamparticipation         = SELECT
>> +public.validpersoncache          = SELECT
>> +
>> +
> 
> How did you determine these?

The tests run as this user, so I ran make schema over and over and over
again whilst gradually increasing the permissions...

> Are you sure you've got everything?

Reasonably.  As you noticed, I spent a long time writing tests...

>>  [targetnamecacheupdater]
>>  type=user
>>  groups=script
>>
> 
> 
>> === added file 'lib/lp/codehosting/branchdistro.py'
>> --- lib/lp/codehosting/branchdistro.py	1970-01-01 00:00:00 +0000
>> +++ lib/lp/codehosting/branchdistro.py	2009-10-13 09:11:15 +0000
>> @@ -0,0 +1,355 @@
>> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Opening a new DistroSeries for branch based development.
>> +
>> +Intended to be run just after a new distro series has been completed, this
>> +script will create an official package branch in the new series for every one
>> +in the old.  The old branch will become stacked on the new, to avoid a using
>> +too much disk space whilst retaining best performance for the new branch.
>> +"""
>> +
>> +import os
>> +
>> +from bzrlib.branch import Branch
>> +from bzrlib.bzrdir import BzrDir
>> +from bzrlib.errors import NotBranchError, NotStacked
>> +
>> +from lazr.uri import URI
>> +
>> +import transaction
>> +
>> +from zope.component import getUtility
>> +
>> +from canonical.launchpad.interfaces import ILaunchpadCelebrities
>> +from canonical.config import config
>> +
>> +from lp.code.interfaces.branchcollection import IAllBranches
>> +from lp.code.interfaces.branch import BranchExists
>> +from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
>> +from lp.code.interfaces.seriessourcepackagebranch import (
>> +    IFindOfficialBranchLinks)
>> +from lp.code.enums import BranchType
>> +from lp.codehosting.vfs import branch_id_to_path
>> +from lp.registry.interfaces.distribution import IDistributionSet
>> +from lp.registry.interfaces.pocket import PackagePublishingPocket
>> +
>> +
>> +__metaclass__ = type
>> +__all__ = []
>> +
> 
> These should be above the imports & below the docstring, IIRC.

Oh yeah.  And there should be something in the __all__ I guess!

>> +
>> +def switch_branches(prefix, scheme, old_db_branch, new_db_branch):
>> +    """Move bzr data from an old to a new branch, leaving old stacked on new.
>> +
>> +    This function is intended to be used just after Ubuntu is released to
>> +    create (at the bzr level) a new trunk branch for a source package for the
>> +    next release of the distribution.  We move the bzr data to the location
>> +    for the new branch and replace the trunk branch for the just released
>> +    version with a stacked branch pointing at the new branch.
>> +
>> +    :param prefix: The non-branch id dependent part of the physical path to
>> +        the branches on disk.
>> +    :param scheme: The branches should be open-able at a URL of the form
>> +        ``scheme + :/// + unique_name``.
>> +    :param old_db_branch: The branch that currently has the trunk bzr data.
>> +    :param old_db_branch: The new trunk branch.  This should not have any
>> +        presence on disk yet.
>> +    """
> 
> Do you think it would be worth documenting failure conditions here, or at
> least mentioning something about atomicity?

Yeah probably.

>> +    # Move .bzr directory from old to new location, crashing through the
>> +    # abstraction we usually hide our branch locations behind.
>> +    old_underlying_path = os.path.join(
>> +        prefix, branch_id_to_path(old_db_branch.id))
>> +    new_underlying_path = os.path.join(
>> +        prefix, branch_id_to_path(new_db_branch.id))
>> +    os.makedirs(new_underlying_path)
>> +    os.rename(
>> +        os.path.join(old_underlying_path, '.bzr'),
>> +        os.path.join(new_underlying_path, '.bzr'))
>> +
>> +    # Create branch at old location -- we use the "clone('null:')" trick to
>> +    # preserve the format.  We have to open at the logical, unique_name-based,
>> +    # location so that it works to set the stacked on url to '/' + a
>> +    # unique_name.
>> +    new_location_bzrdir = BzrDir.open(
>> +        str(URI(
>> +            scheme=scheme, host='', path='/' + new_db_branch.unique_name)))
>> +    old_location_bzrdir = new_location_bzrdir.clone(
>> +        str(URI(
>> +            scheme=scheme, host='', path='/' + old_db_branch.unique_name)),
>> +        revision_id='null:')
>> +
>> +    # Set the stacked on url for old location.
>> +    old_location_branch = old_location_bzrdir.open_branch()
>> +    old_location_branch.set_stacked_on_url('/' + new_db_branch.unique_name)
>> +
>> +    # Pull from new location to old -- this won't actually transfer any
>> +    # revisions, just update the last revision pointer.
>> +    old_location_branch.pull(new_location_bzrdir.open_branch())
>> +
> 
> In a private email thread, I mentioned that this could well be its own utility
> script. I don't know how many people want to move a branch to a new location
> and create a new branch at the old location stacked on the old one, but it
> seems quite generic.

Yeah.  In the "Ubuntu model" of releasing software (where the tip
duplicated and the copy becomes the new line of development), it would
be very generally useful.  Most projects seem to follow the "bzr model"
though, where there is only one trunk series and particular releases are
cloned off that...

In any case I'm not sure that having it as a script in Launchpad is that
useful, I guess I could put it in a bzr plugin... but it won't work as a
user on Launchpad because we don't let rename work between different
branch areas.

> I don't think it needs to be made into a script in order to land.

Oh good :)

>> +
>> +class DistroBrancher:
>> +    """Open a new distroseries for branch based development.
>> +
>> +    `makeNewBranches` will create an official package branch in the new series
>> +    for every one in the old.  `checkNewBranches` will check that a previous
>> +    run of this script completed successfully -- this is only likely to be
>> +    really useful if a script run died halfway through or had to be killed.
>> +    """
>> +
>> +    def __init__(self, logger, old_distroseries, new_distroseries):
>> +        """Construct a `DistroBrancher`.
>> +
>> +        The old and new distroseries must be from the same distribution, but
>> +        not the same distroseries.
>> +
>> +        :param logger: A Logger.  Problems will be logged to this object at
>> +            the WARNING level or higher; progress reports will be logged at
>> +            the DEBUG level.
>> +        :param old_distroseries: The distroseries that will be examined to
>> +            find existing source package branches.
>> +        :param new_distroseries: The distroseries that will have new official
>> +            source branches made for it.
>> +        """
>> +        self.logger = logger
>> +        if old_distroseries.distribution != new_distroseries.distribution:
>> +            raise AssertionError(
>> +                "%s and %s are from different distributions!" %
>> +                (old_distroseries, new_distroseries))
>> +        if old_distroseries == new_distroseries:
>> +            raise AssertionError(
>> +                "New and old distributions must be different!")
>> +        self.old_distroseries = old_distroseries
>> +        self.new_distroseries = new_distroseries
>> +
>> +    @classmethod
>> +    def fromNames(cls, logger, distribution_name, old_distroseries_name,
>> +                  new_distroseries_name):
>> +        """Make a `DistroBrancher` from the names of a distro and two series.
>> +        """
>> +        distribution = getUtility(IDistributionSet).getByName(
>> +            distribution_name)
>> +        new_distroseries = distribution.getSeries(new_distroseries_name)
>> +        old_distroseries = distribution.getSeries(old_distroseries_name)
>> +        return cls(logger, old_distroseries, new_distroseries)
>> +
>> +    def _existingOfficialBranches(self):
>> +        """Return the collection of official branches in the old distroseries.
>> +        """
>> +        branches = getUtility(IAllBranches)
>> +        distroseries_branches = branches.inDistroSeries(self.old_distroseries)
>> +        return distroseries_branches.officialBranches().getBranches()
>> +
>> +    def checkConsistentOfficialPackageBranch(self, db_branch):
>> +        """Check that `db_branch` is a consistent official package branch.
>> +
>> +        'Consistent official package branch' means:
>> +
>> +         * It's a package branch (rather than a personal or junk branch).
>> +         * It's official for its SourcePackage and no other.
>> +
>> +        This function simply returns True or False -- any problems will be
>> +        logged to ``self.logger``.
>> +
>> +        :param db_branch: The `IBranch` to check.
>> +        :return: ``True`` if the branch is a consistent official package
>> +            branch, ``False`` otherwise.
>> +        """
>> +        if db_branch.product:
>> +            self.logger.warning(
>> +                "Encountered unexpected product branch %r",
>> +                db_branch.unique_name)
>> +            return False
>> +        if not db_branch.distroseries:
>> +            self.logger.warning(
>> +                "Encountered unexpected personal branch %s",
>> +                db_branch.unique_name)
>> +            return False
>> +        find_branch_links = getUtility(IFindOfficialBranchLinks)
>> +        links = list(find_branch_links.findForBranch(db_branch))
>> +        if len(links) == 0:
>> +            self.logger.warning(
>> +                "%s is not an official branch", db_branch.unique_name)
>> +            return False
>> +        elif len(links) > 1:
>> +            series_text = ', '.join([
>> +                link.sourcepackage.path for link in links])
>> +            self.logger.warning(
>> +                "%s is official for multiple series: %s",
>> +                db_branch.unique_name, series_text)
>> +            return False
>> +        elif links[0].sourcepackage != db_branch.sourcepackage:
>> +            self.logger.warning(
>> +                "%s is the official branch for %s but not its "
>> +                "sourcepackage", db_branch.unique_name,
>> +                links[0].sourcepackage.path)
>> +            return False
>> +        return True
>> +
>> +    def makeNewBranches(self):
>> +        """Make official branches in the new distroseries."""
>> +        for db_branch in self._existingOfficialBranches():
>> +            self.logger.debug("Processing %s" % db_branch.unique_name)
>> +            try:
>> +                self.makeOneNewBranch(db_branch)
>> +            except BranchExists:
>> +                # Check here?
> 
> Check what here, exactly?

I was thinking that if you find a branch has already been copied into
the new series we could check it -- depends a bit how efficient "check"
turns out to be.  For now, I'll drop it, and delete the comment.

>> +                pass
>> +
>> +    def checkNewBranches(self):
>> +        """Check the branches in the new distroseries are present and correct.
>> +
>> +        This function checks that every official package branch in the old
>> +        distroseries has a matching branch in the new distroseries and that
>> +        stacking is set up as we expect in both the hosted and mirrored areas
>> +        on disk.
>> +
>> +        This function simply returns True or False -- any problems will be
>> +        logged to ``self.logger``.
>> +
>> +        :return: ``True`` if every branch passes the check, ``False``
>> +            otherwise.
>> +        """
> 
> Perhaps its worth mentioning that it goes through every branch, regardless of
> whether any fail.

Make sense, added.

>> +        ok = True
>> +        for db_branch in self._existingOfficialBranches():
>> +            self.logger.debug("Checking %s" % db_branch.unique_name)
>> +            try:
>> +                if not self.checkOneBranch(db_branch):
>> +                    ok = False
>> +            except:
>> +                ok = False
>> +                self.logger.exception(
>> +                    "Unexpected error checking %s!", db_branch)
>> +        return ok
>> +
>> +    def checkOneBranch(self, old_db_branch):
>> +        """Check a branch in the old distroseries has been copied to the new.
>> +
>> +        This function checks that `old_db_branch` has a matching branch in the
>> +        new distroseries and that stacking is set up as we expect in both the
>> +        hosted and mirrored areas on disk.
>> +
>> +        This function simply returns True or False -- any problems will be
>> +        logged to ``self.logger``.
>> +
>> +        :param old_db_branch: The branch to check.
>> +        :return: ``True`` if the branch passes the check, ``False`` otherwise.
>> +        """
>> +        ok = self.checkConsistentOfficialPackageBranch(old_db_branch)
>> +        if not ok:
>> +            return ok
>> +        new_sourcepackage = self.new_distroseries.getSourcePackage(
>> +            old_db_branch.sourcepackagename)
>> +        new_db_branch = new_sourcepackage.getBranch(
>> +            PackagePublishingPocket.RELEASE)
>> +        if new_db_branch is None:
>> +            self.logger.warning(
>> +                "No official branch found for %s",
>> +                new_sourcepackage.path)
>> +            return False
>> +        ok = self.checkConsistentOfficialPackageBranch(new_db_branch)
>> +        if not ok:
>> +            return ok
>> +        # for both mirrored and hosted areas:
>> +        for scheme in 'lp-mirrored', 'lp-hosted':
>> +            # the branch in the new distroseries is unstacked
>> +            new_location = str(URI(
>> +                scheme=scheme, host='', path='/' + new_db_branch.unique_name))
>> +            try:
>> +                new_bzr_branch = Branch.open(new_location)
>> +            except NotBranchError:
>> +                self.logger.warning(
>> +                    "No bzr branch at new location %s", new_location)
>> +                ok = False
>> +            else:
>> +                try:
>> +                    new_stacked_on_url = new_bzr_branch.get_stacked_on_url()
>> +                    ok = False
>> +                    self.logger.warning(
>> +                        "New branch at %s is stacked on %s, should be "
>> +                        "unstacked.", new_location, new_stacked_on_url)
>> +                except NotStacked:
>> +                    pass
>> +            # The branch in the old distroseries is stacked on that in the
>> +            # new.
>> +            old_location = str(URI(
>> +                scheme=scheme, host='', path='/' + old_db_branch.unique_name))
>> +            try:
>> +                old_bzr_branch = Branch.open(old_location)
>> +            except NotBranchError:
>> +                self.logger.warning(
>> +                    "No bzr branch at old location %s", old_location)
>> +                ok = False
>> +            else:
>> +                try:
>> +                    old_stacked_on_url = old_bzr_branch.get_stacked_on_url()
>> +                    if old_stacked_on_url != '/' + new_db_branch.unique_name:
>> +                        self.logger.warning(
>> +                            "Old branch at %s is stacked on %s, should be "
>> +                            "stacked on %s", old_location, old_stacked_on_url,
>> +                            '/' + new_db_branch.unique_name)
>> +                        ok = False
>> +                except NotStacked:
>> +                    self.logger.warning(
>> +                        "Old branch at %s is not stacked, should be stacked "
>> +                        "on %s", old_location,
>> +                        '/' + new_db_branch.unique_name)
>> +                    ok = False
>> +                # The branch in the old distroseries has no revisions in its
>> +                # repository.  We open the repository independently of the
>> +                # branch because the branch's repository has had its fallback
>> +                # location activated. Note that this check might fail if new
>> +                # revisions get pushed to the branch in the old distroseries,
>> +                # which shouldn't happen but isn't totally impossible.
>> +                old_repo = BzrDir.open(old_location).open_repository()
>> +                if len(old_repo.all_revision_ids()) > 0:
>> +                    self.logger.warning(
>> +                        "Repository at %s has %s revisions.",
>> +                        old_location, len(old_repo.all_revision_ids()))
>> +                    ok = False
>> +                # The branch in the old distroseries has at least some
>> +                # history.  (We can't check that the tips are the same because
>> +                # the branch in the new distroseries might have new revisons).
>> +                if old_bzr_branch.last_revision() == 'null:':
>> +                    self.logger.warning(
>> +                        "Old branch at %s has null tip revision.",
>> +                        old_location)
>> +                    ok = False
>> +        return ok
>> +
>> +    def makeOneNewBranch(self, old_db_branch):
>> +        """Copy a branch to the new distroseries.
>> +
>> +        This function makes a new database branch for the same source package
>> +        as old_db_branch but in the new distroseries and then uses
>> +        `switch_branches` to move the underlying bzr branch to the new series
>> +        and replace the old branch with a branch stacked on the new series'
>> +        branch.
>> +
>> +        :param old_db_branch: The branch to copy into the new distroseries.
>> +        :raises BranchExists: This will be raised if old_db_branch has already
>> +            been copied to the new distroseries (in the database, at least).
>> +        """
>> +        if not self.checkConsistentOfficialPackageBranch(old_db_branch):
>> +            self.logger.warning("Skipping branch")
>> +            return
>> +        new_namespace = getUtility(IBranchNamespaceSet).get(
>> +            person=old_db_branch.owner, product=None,
>> +            distroseries=self.new_distroseries,
>> +            sourcepackagename=old_db_branch.sourcepackagename)
>> +        new_db_branch = new_namespace.createBranch(
>> +            BranchType.HOSTED, old_db_branch.name, old_db_branch.registrant)
>> +        new_db_branch.sourcepackage.setBranch(
>> +            PackagePublishingPocket.RELEASE, new_db_branch,
>> +            getUtility(ILaunchpadCelebrities).ubuntu_branches.teamowner)
>> +        # Commit now because switch_branches *moves* the data to locations
>> +        # dependent on the new_branch's id, so if the transaction doesn't get
>> +        # committed we won't know where it's gone.
> 
> I don't quite follow this. Why won't we know where it has gone?

INSERT into branch, get new branch id
move branch data somewhere based on that branch id
ROLLBACK

Now how do you find that branch data again?

I've reworded the comment, hopefully it makes more sense now.

>> +        transaction.commit()
>> +        switch_branches(
>> +            config.codehosting.hosted_branches_root,
>> +            'lp-hosted', old_db_branch, new_db_branch)
>> +        switch_branches(
>> +            config.codehosting.mirrored_branches_root,
>> +            'lp-mirrored', old_db_branch, new_db_branch)
>> +        return new_db_branch
>>
> 
> 
>> === added file 'lib/lp/codehosting/tests/test_branchdistro.py'
>> --- lib/lp/codehosting/tests/test_branchdistro.py	1970-01-01 00:00:00 +0000
>> +++ lib/lp/codehosting/tests/test_branchdistro.py	2009-10-13 09:11:15 +0000
>> @@ -0,0 +1,690 @@
>> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Tests for making new source package branches just after a distro release.
>> +"""
>> +
>> +__metaclass__ = type
>> +
>> +import os
>> +import re
>> +from StringIO import StringIO
>> +from subprocess import PIPE, Popen, STDOUT
>> +import textwrap
>> +import unittest
>> +
>> +from bzrlib.branch import Branch
>> +from bzrlib.bzrdir import BzrDir
>> +from bzrlib.errors import NotStacked
>> +from bzrlib.tests import TestCaseWithTransport
>> +from bzrlib.transport import get_transport
>> +from bzrlib.transport.chroot import ChrootServer
>> +
>> +from lazr.uri import URI
>> +
>> +import transaction
>> +
>> +from canonical.config import config
>> +from canonical.testing.layers import ZopelessAppServerLayer
>> +from canonical.launchpad.scripts.logger import FakeLogger, QuietFakeLogger
>> +
>> +from lp.codehosting.branchdistro import DistroBrancher, switch_branches
>> +from lp.codehosting.vfs import branch_id_to_path
>> +from lp.registry.interfaces.pocket import PackagePublishingPocket
>> +from lp.testing import TestCaseWithFactory
>> +
>> +
>> +# We say "RELEASE" often enough to not want to say "PackagePublishingPocket."
>> +# each time.
>> +RELEASE = PackagePublishingPocket.RELEASE
>> +
>> +
>> +class FakeBranch:
>> +    """Just enough of a Branch to pass `test_switch_branches`."""
>> +
>> +    def __init__(self, id):
>> +        self.id = id
>> +
>> +    @property
>> +    def unique_name(self):
>> +        return branch_id_to_path(self.id)
>> +
>> +
>> +class TestSwitchBranches(TestCaseWithTransport):
>> +    """Tests for `switch_branches`."""
>> +
>> +    def test_switch_branches(self):
>> +        # switch_branches moves a branch to the new location and places a
>> +        # branch (with no revisions) stacked on the new branch in the old
>> +        # location.
>> +
>> +        chroot_server = ChrootServer(self.get_transport())
>> +        chroot_server.setUp()
>> +        self.addCleanup(chroot_server.tearDown)
>> +        scheme = chroot_server.get_url().rstrip('/:')
>> +
>> +        old_branch = FakeBranch(1)
>> +        self.get_transport(old_branch.unique_name).create_prefix()
>> +        tree = self.make_branch_and_tree(old_branch.unique_name)
>> +        tree.commit(message='.')
>> +
>> +        new_branch = FakeBranch(2)
>> +
>> +        switch_branches('.', scheme, old_branch, new_branch)
>> +
>> +        # Post conditions:
>> +        # 1. unstacked branch in new_branch's location
>> +        # 2. stacked branch with no revisions in repo at old_branch
>> +        # 3. last_revision() the same for two branches
>> +
>> +        old_location_bzrdir = BzrDir.open(str(URI(
>> +            scheme=scheme, host='', path='/' + old_branch.unique_name)))
>> +        new_location_bzrdir = BzrDir.open(str(URI(
>> +            scheme=scheme, host='', path='/' + new_branch.unique_name)))
>> +
>> +        old_location_branch = old_location_bzrdir.open_branch()
>> +        new_location_branch = new_location_bzrdir.open_branch()
>> +
>> +        # 1. unstacked branch in new_branch's location
>> +        self.assertRaises(NotStacked, new_location_branch.get_stacked_on_url)
>> +
>> +        # 2. stacked branch with no revisions in repo at old_branch
>> +        self.assertEqual(
>> +            '/' + new_branch.unique_name,
>> +            old_location_branch.get_stacked_on_url())
>> +        self.assertEqual(
>> +            [], old_location_bzrdir.open_repository().all_revision_ids())
>> +
>> +        # 3. last_revision() the same for two branches
>> +        self.assertEqual(
>> +            old_location_branch.last_revision(),
>> +            new_location_branch.last_revision())
>> +
>> +
>> +class TestDistroBrancher(TestCaseWithFactory):
>> +    """Tests for `DistroBrancher`."""
>> +
>> +    layer = ZopelessAppServerLayer
>> +
>> +    def setUp(self):
>> +        TestCaseWithFactory.setUp(self)
>> +        self.useBzrBranches(real_server=True)
>> +
>> +    def makeOfficialPackageBranch(self, distroseries=None):
>> +        """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)
>> +
>> +        transaction.commit()
>> +
>> +        _, tree = self.create_branch_and_tree(
>> +            tree_location=self.factory.getUniqueString(), db_branch=db_branch,
>> +            hosted=True)
>> +        tree.commit('')
>> +        mirrored_branch = BzrDir.create_branch_convenience(
>> +            db_branch.warehouse_url)
>> +        mirrored_branch.pull(tree.branch)
>> +
>> +        return db_branch
>> +
>> +    def makeNewSeriesAndBrancher(self, distroseries=None):
>> +        """Make a DistroBrancher.
>> +
>> +        Any messages logged by this DistroBrancher can be checked by calling
>> +        `assertLogMessages` below.
>> +        """
>> +        if distroseries is None:
>> +            distroseries = self.factory.makeDistroRelease()
>> +        self._log_file = StringIO()
>> +        new_distroseries = self.factory.makeDistroRelease(
>> +            distribution=distroseries.distribution, name='new')
>> +        transaction.commit()
>> +        self.layer.switchDbUser('branch-distro')
>> +        return DistroBrancher(
>> +            FakeLogger(self._log_file), distroseries, new_distroseries)
>> +
>> +    def clearLogMessages(self):
>> +        """Forget about all logged messages seen so far."""
>> +        self._log_file.seek(0, 0)
>> +        self._log_file.truncate()
>> +
>> +    def assertLogMessages(self, patterns):
>> +        """Assert that the messages logged meet expectations.
>> +
>> +        :param patterns: A list of regular expressions.  The length must match
>> +            the number of messages logged, and then each pattern must match
>> +            the messages logged in order.
>> +        """
>> +        log_messages = self._log_file.getvalue().splitlines()
>> +        if len(log_messages) > len(patterns):
>> +            self.fail(
>> +                "More log messages (%s) than expected (%s)" %
>> +                (log_messages, patterns))
>> +        elif len(log_messages) < len(patterns):
>> +            self.fail(
>> +                "Fewer log messages (%s) than expected (%s)" %
>> +                (log_messages, patterns))
>> +        for pattern, message in zip(patterns, log_messages):
>> +            if not re.match(pattern, message):
>> +                self.fail("%r does not match %r" % (pattern, message))
>> +
>> +    def test_DistroBrancher_same_distro_check(self):
>> +        # DistroBrancher.__init__ raises AssertionError if the two
>> +        # distroseries passed are not from the same distribution.
>> +        self.assertRaises(
>> +            AssertionError, DistroBrancher, None,
>> +            self.factory.makeDistroRelease(),
>> +            self.factory.makeDistroRelease())
>> +
>> +    def test_DistroBrancher_same_distroseries_check(self):
>> +        # DistroBrancher.__init__ raises AssertionError if passed the same
>> +        # distroseries twice.
>> +        distroseries = self.factory.makeDistroRelease()
>> +        self.assertRaises(
>> +            AssertionError, DistroBrancher, None, distroseries, distroseries)
>> +
>> +    def test_fromNames(self):
>> +        # DistroBrancher.__init__ raises AssertionError if passed the same
>> +        # distroseries twice.
> 
> The comment here is incorrect.

Good spot, thanks.

>> +        distribution = self.factory.makeDistribution()
>> +        distroseries1 = self.factory.makeDistroRelease(
>> +            distribution=distribution)
>> +        distroseries2 = self.factory.makeDistroRelease(
>> +            distribution=distribution)
>> +        brancher = DistroBrancher.fromNames(
>> +            None, distribution.name, distroseries1.name, distroseries2.name)
>> +        self.assertEqual(
>> +            [distroseries1, distroseries2],
>> +            [brancher.old_distroseries, brancher.new_distroseries])
>> +
>> +    # A word on testing strategy: we don't directly test the post conditions
>> +    # of makeOneNewBranch, but we do test that it satisfies checkOneBranch and
>> +    # the tests for checkOneBranch verify that this function rejects various
>> +    # ways in which makeOneNewBranch could conceivably fail.
>> +
> 
> Thanks, this is a helpful comment.

I do try :)

>> +    def test_makeOneNewBranch(self):
>> +        # makeOneNewBranch creates an official package branch in the new
>> +        # distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +
>> +        new_branch = brancher.new_distroseries.getSourcePackage(
>> +            db_branch.sourcepackage.name).getBranch(RELEASE)
>> +
>> +        self.assertIsNot(new_branch, None)
>> +
> 
> Wouldn't it be nice if there were an explicit ordering for 'expected' and
> 'observed' :)

Sigh.

> I notice you don't make any assertions about the name, owner or target of the
> branch. I wonder if these are checked later...

That's a good point, I don't think that is tested.

>> +    def test_makeOneNewBranch_inconsistent_branch(self):
>> +        # makeOneNewBranch skips over an inconsistent official package branch
>> +        # (see `checkConsistentOfficialPackageBranch` for precisely what an
>> +        # "inconsistent official package branch" is).
>> +        unofficial_branch = self.factory.makePackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(
>> +            unofficial_branch.distroseries)
>> +        brancher.makeOneNewBranch(unofficial_branch)
>> +
>> +        new_branch = brancher.new_distroseries.getSourcePackage(
>> +            unofficial_branch.sourcepackage.name).getBranch(RELEASE)
>> +        self.assertIs(new_branch, None)
>> +        self.assertLogMessages(
>> +            ['^WARNING .*',
>> +             '^WARNING Skipping branch'])
>> +
> 
> What's the first warning for?

It's the warning that the branch is inconsistent.  I'll make the test
less generic...

>> +    def test_makeNewBranches(self):
>> +        # makeNewBranches calls makeOneNewBranch for each official branch in
>> +        # the old distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        db_branch2 = self.makeOfficialPackageBranch(
>> +            distroseries=db_branch.distroseries)
>> +
>> +        new_distroseries = self.factory.makeDistroRelease(
>> +            distribution=db_branch.distribution)
>> +
>> +        brancher = DistroBrancher(
>> +            QuietFakeLogger(), db_branch.distroseries, new_distroseries)
>> +
>> +        brancher.makeNewBranches()
>> +
>> +        new_sourcepackage = new_distroseries.getSourcePackage(
>> +            db_branch.sourcepackage.name)
>> +        new_branch = new_sourcepackage.getBranch(RELEASE)
>> +        new_sourcepackage2 = new_distroseries.getSourcePackage(
>> +            db_branch2.sourcepackage.name)
>> +        new_branch2 = new_sourcepackage2.getBranch(RELEASE)
>> +
>> +        self.assertIsNot(new_branch, None)
>> +        self.assertIsNot(new_branch2, None)
>> +
>> +    def test_makeNewBranches_idempotent(self):
>> +        # makeNewBranches is idempotent in the sense that if a branch in the
>> +        # old distroseries already has a counterpart in the new distroseries,
>> +        # it is silently ignored.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeNewBranches()
>> +        brancher.makeNewBranches()
>> +
>> +        new_branch = brancher.new_distroseries.getSourcePackage(
>> +            db_branch.sourcepackage.name).getBranch(RELEASE)
>> +
>> +        self.assertIsNot(new_branch, None)
>> +
>> +    def test_makeOneNewBranch_checks_ok(self):
>> +        # After calling makeOneNewBranch for a branch, calling checkOneBranch
>> +        # returns True for that branch.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        self.clearLogMessages()
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([])
>> +        self.assertTrue(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_product_branch(self):
>> +        # checkConsistentOfficialPackageBranch returns False when passed a
>> +        # product branch.
>> +        db_branch = self.factory.makeProductBranch()
>> +        brancher = self.makeNewSeriesAndBrancher()
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Encountered unexpected product branch .*/.*/.*$'])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_personal_branch(self):
>> +        # checkConsistentOfficialPackageBranch returns False when passed a
>> +        # personal branch.
>> +        db_branch = self.factory.makePersonalBranch()
>> +        brancher = self.makeNewSeriesAndBrancher()
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Encountered unexpected personal branch .*/.*/.*$'])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_no_official_branch(self):
>> +        # checkConsistentOfficialPackageBranch returns False when passed a
>> +        # branch which is not official for any package.
>> +        db_branch = self.factory.makePackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages(
>> +            ['^WARNING .*/.*/.*\ is not an official branch$'])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_official_elsewhere(self):
>> +        # checkConsistentOfficialPackageBranch returns False when passed a
>> +        # branch which is official for a sourcepackage that it is not a branch
>> +        # for.
>> +        db_branch = self.factory.makePackageBranch()
>> +        self.factory.makeSourcePackage().setBranch(
>> +            RELEASE, db_branch, db_branch.owner)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages(
>> +            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
>> +             'its sourcepackage$'])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_official_twice(self):
>> +        # checkConsistentOfficialPackageBranch returns False when passed a
>> +        # branch that is official for two sourcepackages.
>> +        db_branch = self.factory.makePackageBranch()
>> +        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
>> +        self.factory.makeSourcePackage().setBranch(
>> +            RELEASE, db_branch, db_branch.owner)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING .*/.*/.* is official for multiple series: .*/.*/.*, '
>> +            '.*/.*/.*$'])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkConsistentOfficialPackageBranch_ok(self):
>> +        # checkConsistentOfficialPackageBranch returns True when passed a
>> +        # branch that is official for its sourcepackage and no other.
>> +        db_branch = self.factory.makePackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
>> +        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
>> +        self.assertLogMessages([])
>> +        self.assertTrue(ok)
>> +
>> +    def test_checkOneBranch_inconsistent_old_package_branch(self):
>> +        # checkOneBranch returns False when passed a branch that is not a
>> +        # consistent official package branch.
>> +        db_branch = self.factory.makePackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher()
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages(
>> +            ['^WARNING .*/.*/.* is not an official branch$'])
>> +
>> +    def test_checkOneBranch_no_new_official_branch(self):
>> +        # checkOneBranch returns False when there is no corresponding official
>> +        # package branch in the new distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages(
>> +            ['^WARNING No official branch found for .*/.*/.*$'])
>> +
>> +    def test_checkOneBranch_inconsistent_new_package_branch(self):
>> +        # checkOneBranch returns False when the corresponding official package
>> +        # branch in the new distroseries is not consistent.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
>> +        self.layer.switchDbUser('launchpad')
>> +        new_db_branch.setTarget(
>> +            new_db_branch.owner,
>> +            source_package=self.factory.makeSourcePackage())
>> +        transaction.commit()
>> +        self.layer.switchDbUser('branch-distro')
>> +        ok = brancher.checkOneBranch(new_db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages(
>> +            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
>> +             'its sourcepackage$'])
>> +
>> +    # All these hosted/mirrored tests are very repetitive, perhaps some meta
>> +    # programming would reduce LOC count, but maybe make things harder to
>> +    # understand.
>> +
> 
> It's not just the LOC count, it's the duplicated logic. We never want hosted &
> mirrored to differ in behaviour here.
> 
> Did you try a metaprogramming approach? Was it really that much harder to
> follow?

No, I never really tried it.

I tried it, it's better -- thanks for the prod :-)

>> +    def test_checkOneBranch_new_hosted_branch_missing(self):
>> +        # checkOneBranch returns False when there is no bzr branch in the
>> +        # hosted area for the database branch in the new distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
>> +        get_transport(new_db_branch.getPullURL()).delete_tree('.bzr')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        # Deleting the new hosted branch will break the old branch, as that's
>> +        # stacked on the new one.
>> +        self.assertLogMessages([
>> +            '^WARNING No bzr branch at new location '
>> +            'lp-hosted:///.*/.*/.*/.*$',
>> +            '^WARNING No bzr branch at old location '
>> +            'lp-hosted:///.*/.*/.*/.*$',
>> +            ])
>> +
>> +    def test_checkOneBranch_new_mirrored_branch_missing(self):
>> +        # checkOneBranch returns False when there is no bzr branch in the
>> +        # mirrored area for the database branch in the new distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
>> +        get_transport(new_db_branch.warehouse_url).delete_tree('.bzr')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        # Deleting the new mirrored branch will break the old branch, as
>> +        # that's stacked on the new one.
>> +        self.assertLogMessages([
>> +            '^WARNING No bzr branch at new location '
>> +            'lp-mirrored:///.*/.*/.*/.*$',
>> +            '^WARNING No bzr branch at old location '
>> +            'lp-mirrored:///.*/.*/.*/.*$',
>> +            ])
>> +
>> +    def test_checkOneBranch_old_hosted_branch_missing(self):
>> +        # checkOneBranch returns False when there is no bzr branch in the
>> +        # hosted area for the database branch in old distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        get_transport(db_branch.getPullURL()).delete_tree('.bzr')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages([
>> +            '^WARNING No bzr branch at old location '
>> +            'lp-hosted:///.*/.*/.*/.*$',
>> +            ])
>> +
>> +    def test_checkOneBranch_old_mirrored_branch_missing(self):
>> +        # checkOneBranch returns False when there is no bzr branch in the
>> +        # mirrored area for the database branch in old distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        get_transport(db_branch.warehouse_url).delete_tree('.bzr')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages([
>> +            '^WARNING No bzr branch at old location '
>> +            'lp-mirrored:///.*/.*/.*/.*$',
>> +            ])
>> +
>> +    def test_checkOneBranch_new_hosted_stacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the hosted area
>> +        # for the database branch in new distroseries is stacked.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        b, _ = self.create_branch_and_tree(
>> +            self.factory.getUniqueString(), hosted=True)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
>> +        Branch.open(new_db_branch.getPullURL()).set_stacked_on_url(
>> +            '/' + b.unique_name)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages([
>> +            '^WARNING New branch at lp-hosted:///.*/.*/.*/.* is stacked on '
>> +            '/.*/.*/.*, should be unstacked.$',
>> +            ])
>> +
>> +    def test_checkOneBranch_new_mirrored_stacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the mirrored
>> +        # area for the database branch in new distroseries is stacked.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        b, _ = self.create_branch_and_tree(
>> +            self.factory.getUniqueString(), hosted=False)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        new_db_branch = brancher.makeOneNewBranch(db_branch)
>> +        Branch.open(new_db_branch.warehouse_url).set_stacked_on_url(
>> +            '/' + b.unique_name)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertFalse(ok)
>> +        self.assertLogMessages([
>> +            '^WARNING New branch at lp-mirrored:///.*/.*/.*/.* is stacked on '
>> +            '/.*/.*/.*, should be unstacked.$',
>> +            ])
>> +
>> +    def test_checkOneBranch_old_hosted_unstacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the hosted area
>> +        # for the database branch in old distroseries is not stacked.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
>> +        old_hosted_bzr_branch.set_stacked_on_url(None)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* is not '
>> +            'stacked, should be stacked on /.*/.*/.*.$',
>> +            '^.*has .* revisions.*$',
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_mirrored_unstacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the mirrored
>> +        # area for the database branch in old distroseries is not stacked.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
>> +        old_mirrored_bzr_branch.set_stacked_on_url(None)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* is not '
>> +            'stacked, should be stacked on /.*/.*/.*.$',
>> +            '^.*has .* revisions.*$',
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_hosted_misstacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the hosted area
>> +        # for the database branch in old distroseries stacked on some other
>> +        # branch than the branch in the new distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        b, _ = self.create_branch_and_tree(
>> +            self.factory.getUniqueString(), hosted=True)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        Branch.open(db_branch.getPullURL()).set_stacked_on_url(
>> +            '/' + b.unique_name)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* is stacked on '
>> +            '/.*/.*/.*, should be stacked on /.*/.*/.*.$',
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_mirrored_misstacked(self):
>> +        # checkOneBranch returns False when the bzr branch in the mirrored
>> +        # area for the database branch in old distroseries stacked on some
>> +        # other branch than the branch in the new distroseries.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        b, _ = self.create_branch_and_tree(
>> +            self.factory.getUniqueString(), hosted=False)
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        Branch.open(db_branch.warehouse_url).set_stacked_on_url(
>> +            '/' + b.unique_name)
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* is stacked on '
>> +            '/.*/.*/.*, should be stacked on /.*/.*/.*.$',
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_hosted_has_revisions(self):
>> +        # checkOneBranch returns False when the bzr branch in the hosted area
>> +        # for the database branch in old distroseries has a repository that
>> +        # contains revisions.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
>> +        old_hosted_bzr_branch.create_checkout(
>> +            self.factory.getUniqueString()).commit('')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Repository at lp-hosted:///.*/.*/.*/.* has 1 revisions.'
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_mirrored_has_revisions(self):
>> +        # checkOneBranch returns False when the bzr branch in the mirrored
>> +        # area for the database branch in old distroseries has a repository
>> +        # that contains revisions.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
>> +        old_mirrored_bzr_branch.create_checkout(
>> +            self.factory.getUniqueString()).commit('')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Repository at lp-mirrored:///.*/.*/.*/.* has 1 '
>> +            'revisions.'
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_hosted_has_null_tip(self):
>> +        # checkOneBranch returns False when the bzr branch in the hosted area
>> +        # for the database branch in old distroseries has tip revision of
>> +        # 'null:'.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_hosted_bzr_branch = Branch.open(db_branch.getPullURL())
>> +        old_hosted_bzr_branch.set_last_revision_info(0, 'null:')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-hosted:///.*/.*/.*/.* has null tip '
>> +            'revision.'
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def test_checkOneBranch_old_mirrored_has_null_tip(self):
>> +        # checkOneBranch returns False when the bzr branch in the mirrored
>> +        # area for the database branch in old distroseries has tip revision of
>> +        # 'null:'.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeOneNewBranch(db_branch)
>> +        old_mirrored_bzr_branch = Branch.open(db_branch.warehouse_url)
>> +        old_mirrored_bzr_branch.set_last_revision_info(0, 'null:')
>> +        ok = brancher.checkOneBranch(db_branch)
>> +        self.assertLogMessages([
>> +            '^WARNING Old branch at lp-mirrored:///.*/.*/.*/.* has null tip '
>> +            'revision.'
>> +            ])
>> +        self.assertFalse(ok)
>> +
>> +    def runBranchDistroScript(self, args):
>> +        """Run the branch-distro.py script with the given arguments.
>> +
>> +        ;param args: The arguments to pass to the branch-distro.py script.
>> +        :return: A tuple (returncode, output).  stderr and stdout are both
>> +            contained in the output.
>> +        """
>> +        script_path = os.path.join(config.root, 'scripts', 'branch-distro.py')
>> +        process = Popen([script_path] + args, stdout=PIPE, stderr=STDOUT)
>> +        output, error = process.communicate()
>> +        return process.returncode, output
>> +
>> +    def test_makeNewBranches_script(self):
>> +        # Running the script with the arguments 'distro old-series new-series'
>> +        # makes new branches in the new series.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        returncode, output = self.runBranchDistroScript(
>> +            ['-v', db_branch.distribution.name,
>> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
>> +        self.assertEqual(0, returncode)
>> +        self.assertEqual(
>> +            'DEBUG   Processing ' + db_branch.unique_name + '\n', output)
>> +        brancher.checkOneBranch(db_branch)
>> +
>> +    def test_checkNewBranches_script_success(self):
>> +        # Running the script with the arguments '--check distro old-series
>> +        # new-series' checks that the branches in the new series are as
>> +        # expected.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        brancher.makeNewBranches()
>> +        returncode, output = self.runBranchDistroScript(
>> +            ['-v', '--check', db_branch.distribution.name,
>> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
>> +        self.assertEqual(0, returncode)
>> +        self.assertEqual(
>> +            'DEBUG   Checking ' + db_branch.unique_name + '\n', output)
>> +        brancher.checkOneBranch(db_branch)
>> +
>> +    def test_checkNewBranches_script_failure(self):
>> +        # Running the script with the arguments '--check distro old-series
>> +        # new-series' checks that the branches in the new series are as
>> +        # expected and logs warnings and exits with code 1 is things are not
>> +        # as expected.
>> +        db_branch = self.makeOfficialPackageBranch()
>> +        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
>> +        returncode, output = self.runBranchDistroScript(
>> +            ['-v', '--check', db_branch.distribution.name,
>> +             brancher.old_distroseries.name, brancher.new_distroseries.name])
>> +        sp_path = brancher.new_distroseries.getSourcePackage(
>> +            db_branch.sourcepackagename).path
>> +        expected = '''\
>> +        DEBUG   Checking %(branch_name)s
>> +        WARNING No official branch found for %(sp_path)s
>> +        ERROR   Check failed
>> +        ''' % {'branch_name': db_branch.unique_name, 'sp_path': sp_path}
>> +        self.assertEqual(
>> +            textwrap.dedent(expected), output)
>> +        self.assertEqual(1, returncode)
>> +
>> +
>> +def test_suite():
>> +    return unittest.TestLoader().loadTestsFromName(__name__)
>> +
>>
> 
> That's a lot of test case. :)

Yes.

>> === added file 'scripts/branch-distro.py'
>> --- scripts/branch-distro.py	1970-01-01 00:00:00 +0000
>> +++ scripts/branch-distro.py	2009-10-13 09:11:15 +0000
>> @@ -0,0 +1,38 @@
>> +#!/usr/bin/python2.4
>> +#
>> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +import _pythonpath
>> +
>> +from lp.codehosting.branchdistro import DistroBrancher
>> +from lp.codehosting.vfs import get_multi_server
>> +from lp.services.scripts.base import LaunchpadScript, LaunchpadScriptFailure
>> +
>> +
>> +class BranchDistroScript(LaunchpadScript):
> 
> Blank line here please.

OK.

>> +    usage = "%prog distro old-series new-series"
>> +
>> +    def add_my_options(self):
>> +        self.parser.add_option(
>> +            '--check', dest="check", action="store_true", default=False,
>> +            help=("Check that the new distro series has its official "
>> +                  "branches set up correctly."))
>> +
>> +    def main(self):
>> +        if len(self.args) != 3:
>> +            self.parser.error("Wrong number of arguments.")
>> +        brancher = DistroBrancher.fromNames(self.logger, *self.args)
>> +        server = get_multi_server(write_mirrored=True, write_hosted=True)
>> +        server.setUp()
>> +        try:
>> +            if self.options.check:
>> +                if not brancher.checkNewBranches():
>> +                    raise LaunchpadScriptFailure("Check failed")
>> +            else:
>> +                brancher.makeNewBranches()
>> +        finally:
>> +            server.tearDown()
>> +
>> +if __name__ == '__main__':
>> +    BranchDistroScript("branch-distro", dbuser='branch-distro').run()
>>
>

Cheers,
mwh

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Barki Mustapha

Celso Providelo

Christian Reis

Christy Awad

Colin Watson

Harpianto,ANDI

James Troup

John A Meinel

Kevin bush

Launchpad code reviewers

Launchpad code reviewers from Canonical

Matthew Tanner

Maximiliano Bertacchini

Michael Hudson-Doyle

Oguz Ersoz

Simon Brakhane

Ubuntu-BR DevOps

William Grant

alhawiti

api.ng

pedro cavazos

todaioan

wenjingwen

to status/vote changes:

Tzaddi

Tzaddi Belding

 === modified file 'database/schema/security.cfg'
 --- database/schema/security.cfg	2009-10-05 15:07:10 +0000
 +++ database/schema/security.cfg	2009-10-14 00:31:13 +0000
@@ -641,6 +641,22 @@
  # Merge notifications
  public.codereviewvote                   = SELECT
++[branch-distro]
++type=user
++public.branch                    = SELECT, INSERT
++public.branchsubscription        = SELECT, INSERT
++public.distribution              = SELECT
++public.distroseries              = SELECT
++public.karma                     = SELECT, INSERT
++public.karmaaction               = SELECT
++public.person                    = SELECT
++public.product                   = SELECT
++public.seriessourcepackagebranch = SELECT, INSERT, DELETE
++public.sourcepackagename         = SELECT
++public.teamparticipation         = SELECT
++public.validpersoncache          = SELECT
++
++
  [targetnamecacheupdater]
  type=user
  groups=script
 === added file 'lib/lp/codehosting/branchdistro.py'
 --- lib/lp/codehosting/branchdistro.py	1970-01-01 00:00:00 +0000
 +++ lib/lp/codehosting/branchdistro.py	2009-10-14 00:31:13 +0000
@@ -0,0 +1,364 @@
++# Copyright 2009 Canonical Ltd.  This software is licensed under the
++# GNU Affero General Public License version 3 (see the file LICENSE).
++
++"""Opening a new DistroSeries for branch based development.
++
++Intended to be run just after a new distro series has been completed, this
++script will create an official package branch in the new series for every one
++in the old.  The old branch will become stacked on the new, to avoid a using
++too much disk space whilst retaining best performance for the new branch.
++"""
++
++__metaclass__ = type
++__all__ = [
++    'DistroBrancher',
++    'switch_branches',
++    ]
++
++import os
++
++from bzrlib.branch import Branch
++from bzrlib.bzrdir import BzrDir
++from bzrlib.errors import NotBranchError, NotStacked
++
++from lazr.uri import URI
++
++import transaction
++
++from zope.component import getUtility
++
++from canonical.launchpad.interfaces import ILaunchpadCelebrities
++from canonical.config import config
++
++from lp.code.interfaces.branchcollection import IAllBranches
++from lp.code.interfaces.branch import BranchExists
++from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
++from lp.code.interfaces.seriessourcepackagebranch import (
++    IFindOfficialBranchLinks)
++from lp.code.enums import BranchType
++from lp.codehosting.vfs import branch_id_to_path
++from lp.registry.interfaces.distribution import IDistributionSet
++from lp.registry.interfaces.pocket import PackagePublishingPocket
++
++
++def switch_branches(prefix, scheme, old_db_branch, new_db_branch):
++    """Move bzr data from an old to a new branch, leaving old stacked on new.
++
++    This function is intended to be used just after Ubuntu is released to
++    create (at the bzr level) a new trunk branch for a source package for the
++    next release of the distribution.  We move the bzr data to the location
++    for the new branch and replace the trunk branch for the just released
++    version with a stacked branch pointing at the new branch.
++
++    The procedure is to complicated to be carried out atomically, so if this
++    function is interrupted things may be a little inconsistent (e.g. there
++    might be a branch in the old location, but not stacked on the new location
++    yet).  There should be no data loss though.
++
++    :param prefix: The non-branch id dependent part of the physical path to
++        the branches on disk.
++    :param scheme: The branches should be open-able at a URL of the form
++        ``scheme + :/// + unique_name``.
++    :param old_db_branch: The branch that currently has the trunk bzr data.
++    :param old_db_branch: The new trunk branch.  This should not have any
++        presence on disk yet.
++    """
++    # Move .bzr directory from old to new location, crashing through the
++    # abstraction we usually hide our branch locations behind.
++    old_underlying_path = os.path.join(
++        prefix, branch_id_to_path(old_db_branch.id))
++    new_underlying_path = os.path.join(
++        prefix, branch_id_to_path(new_db_branch.id))
++    os.makedirs(new_underlying_path)
++    os.rename(
++        os.path.join(old_underlying_path, '.bzr'),
++        os.path.join(new_underlying_path, '.bzr'))
++
++    # Create branch at old location -- we use the "clone('null:')" trick to
++    # preserve the format.  We have to open at the logical, unique_name-based,
++    # location so that it works to set the stacked on url to '/' + a
++    # unique_name.
++    new_location_bzrdir = BzrDir.open(
++        str(URI(
++            scheme=scheme, host='', path='/' + new_db_branch.unique_name)))
++    old_location_bzrdir = new_location_bzrdir.clone(
++        str(URI(
++            scheme=scheme, host='', path='/' + old_db_branch.unique_name)),
++        revision_id='null:')
++
++    # Set the stacked on url for old location.
++    old_location_branch = old_location_bzrdir.open_branch()
++    old_location_branch.set_stacked_on_url('/' + new_db_branch.unique_name)
++
++    # Pull from new location to old -- this won't actually transfer any
++    # revisions, just update the last revision pointer.
++    old_location_branch.pull(new_location_bzrdir.open_branch())
++
++
++class DistroBrancher:
++    """Open a new distroseries for branch based development.
++
++    `makeNewBranches` will create an official package branch in the new series
++    for every one in the old.  `checkNewBranches` will check that a previous
++    run of this script completed successfully -- this is only likely to be
++    really useful if a script run died halfway through or had to be killed.
++    """
++
++    def __init__(self, logger, old_distroseries, new_distroseries):
++        """Construct a `DistroBrancher`.
++
++        The old and new distroseries must be from the same distribution, but
++        not the same distroseries.
++
++        :param logger: A Logger.  Problems will be logged to this object at
++            the WARNING level or higher; progress reports will be logged at
++            the DEBUG level.
++        :param old_distroseries: The distroseries that will be examined to
++            find existing source package branches.
++        :param new_distroseries: The distroseries that will have new official
++            source branches made for it.
++        """
++        self.logger = logger
++        if old_distroseries.distribution != new_distroseries.distribution:
++            raise AssertionError(
++                "%s and %s are from different distributions!" %
++                (old_distroseries, new_distroseries))
++        if old_distroseries == new_distroseries:
++            raise AssertionError(
++                "New and old distributions must be different!")
++        self.old_distroseries = old_distroseries
++        self.new_distroseries = new_distroseries
++
++    @classmethod
++    def fromNames(cls, logger, distribution_name, old_distroseries_name,
++                  new_distroseries_name):
++        """Make a `DistroBrancher` from the names of a distro and two series.
++        """
++        distribution = getUtility(IDistributionSet).getByName(
++            distribution_name)
++        new_distroseries = distribution.getSeries(new_distroseries_name)
++        old_distroseries = distribution.getSeries(old_distroseries_name)
++        return cls(logger, old_distroseries, new_distroseries)
++
++    def _existingOfficialBranches(self):
++        """Return the collection of official branches in the old distroseries.
++        """
++        branches = getUtility(IAllBranches)
++        distroseries_branches = branches.inDistroSeries(self.old_distroseries)
++        return distroseries_branches.officialBranches().getBranches()
++
++    def checkConsistentOfficialPackageBranch(self, db_branch):
++        """Check that `db_branch` is a consistent official package branch.
++
++        'Consistent official package branch' means:
++
++         * It's a package branch (rather than a personal or junk branch).
++         * It's official for its SourcePackage and no other.
++
++        This function simply returns True or False -- any problems will be
++        logged to ``self.logger``.
++
++        :param db_branch: The `IBranch` to check.
++        :return: ``True`` if the branch is a consistent official package
++            branch, ``False`` otherwise.
++        """
++        if db_branch.product:
++            self.logger.warning(
++                "Encountered unexpected product branch %r",
++                db_branch.unique_name)
++            return False
++        if not db_branch.distroseries:
++            self.logger.warning(
++                "Encountered unexpected personal branch %s",
++                db_branch.unique_name)
++            return False
++        find_branch_links = getUtility(IFindOfficialBranchLinks)
++        links = list(find_branch_links.findForBranch(db_branch))
++        if len(links) == 0:
++            self.logger.warning(
++                "%s is not an official branch", db_branch.unique_name)
++            return False
++        elif len(links) > 1:
++            series_text = ', '.join([
++                link.sourcepackage.path for link in links])
++            self.logger.warning(
++                "%s is official for multiple series: %s",
++                db_branch.unique_name, series_text)
++            return False
++        elif links[0].sourcepackage != db_branch.sourcepackage:
++            self.logger.warning(
++                "%s is the official branch for %s but not its "
++                "sourcepackage", db_branch.unique_name,
++                links[0].sourcepackage.path)
++            return False
++        return True
++
++    def makeNewBranches(self):
++        """Make official branches in the new distroseries."""
++        for db_branch in self._existingOfficialBranches():
++            self.logger.debug("Processing %s" % db_branch.unique_name)
++            try:
++                self.makeOneNewBranch(db_branch)
++            except BranchExists:
++                pass
++
++    def checkNewBranches(self):
++        """Check the branches in the new distroseries are present and correct.
++
++        This function checks that every official package branch in the old
++        distroseries has a matching branch in the new distroseries and that
++        stacking is set up as we expect in both the hosted and mirrored areas
++        on disk.
++
++        Every branch will be checked, even if some fail.
++
++        This function simply returns True or False -- any problems will be
++        logged to ``self.logger``.
++
++        :return: ``True`` if every branch passes the check, ``False``
++            otherwise.
++        """
++        ok = True
++        for db_branch in self._existingOfficialBranches():
++            self.logger.debug("Checking %s" % db_branch.unique_name)
++            try:
++                if not self.checkOneBranch(db_branch):
++                    ok = False
++            except:
++                ok = False
++                self.logger.exception(
++                    "Unexpected error checking %s!", db_branch)
++        return ok
++
++    def checkOneBranch(self, old_db_branch):
++        """Check a branch in the old distroseries has been copied to the new.
++
++        This function checks that `old_db_branch` has a matching branch in the
++        new distroseries and that stacking is set up as we expect in both the
++        hosted and mirrored areas on disk.
++
++        This function simply returns True or False -- any problems will be
++        logged to ``self.logger``.
++
++        :param old_db_branch: The branch to check.
++        :return: ``True`` if the branch passes the check, ``False`` otherwise.
++        """
++        ok = self.checkConsistentOfficialPackageBranch(old_db_branch)
++        if not ok:
++            return ok
++        new_sourcepackage = self.new_distroseries.getSourcePackage(
++            old_db_branch.sourcepackagename)
++        new_db_branch = new_sourcepackage.getBranch(
++            PackagePublishingPocket.RELEASE)
++        if new_db_branch is None:
++            self.logger.warning(
++                "No official branch found for %s",
++                new_sourcepackage.path)
++            return False
++        ok = self.checkConsistentOfficialPackageBranch(new_db_branch)
++        if not ok:
++            return ok
++        # for both mirrored and hosted areas:
++        for scheme in 'lp-mirrored', 'lp-hosted':
++            # the branch in the new distroseries is unstacked
++            new_location = str(URI(
++                scheme=scheme, host='', path='/' + new_db_branch.unique_name))
++            try:
++                new_bzr_branch = Branch.open(new_location)
++            except NotBranchError:
++                self.logger.warning(
++                    "No bzr branch at new location %s", new_location)
++                ok = False
++            else:
++                try:
++                    new_stacked_on_url = new_bzr_branch.get_stacked_on_url()
++                    ok = False
++                    self.logger.warning(
++                        "New branch at %s is stacked on %s, should be "
++                        "unstacked.", new_location, new_stacked_on_url)
++                except NotStacked:
++                    pass
++            # The branch in the old distroseries is stacked on that in the
++            # new.
++            old_location = str(URI(
++                scheme=scheme, host='', path='/' + old_db_branch.unique_name))
++            try:
++                old_bzr_branch = Branch.open(old_location)
++            except NotBranchError:
++                self.logger.warning(
++                    "No bzr branch at old location %s", old_location)
++                ok = False
++            else:
++                try:
++                    old_stacked_on_url = old_bzr_branch.get_stacked_on_url()
++                    if old_stacked_on_url != '/' + new_db_branch.unique_name:
++                        self.logger.warning(
++                            "Old branch at %s is stacked on %s, should be "
++                            "stacked on %s", old_location, old_stacked_on_url,
++                            '/' + new_db_branch.unique_name)
++                        ok = False
++                except NotStacked:
++                    self.logger.warning(
++                        "Old branch at %s is not stacked, should be stacked "
++                        "on %s", old_location,
++                        '/' + new_db_branch.unique_name)
++                    ok = False
++                # The branch in the old distroseries has no revisions in its
++                # repository.  We open the repository independently of the
++                # branch because the branch's repository has had its fallback
++                # location activated. Note that this check might fail if new
++                # revisions get pushed to the branch in the old distroseries,
++                # which shouldn't happen but isn't totally impossible.
++                old_repo = BzrDir.open(old_location).open_repository()
++                if len(old_repo.all_revision_ids()) > 0:
++                    self.logger.warning(
++                        "Repository at %s has %s revisions.",
++                        old_location, len(old_repo.all_revision_ids()))
++                    ok = False
++                # The branch in the old distroseries has at least some
++                # history.  (We can't check that the tips are the same because
++                # the branch in the new distroseries might have new revisons).
++                if old_bzr_branch.last_revision() == 'null:':
++                    self.logger.warning(
++                        "Old branch at %s has null tip revision.",
++                        old_location)
++                    ok = False
++        return ok
++
++    def makeOneNewBranch(self, old_db_branch):
++        """Copy a branch to the new distroseries.
++
++        This function makes a new database branch for the same source package
++        as old_db_branch but in the new distroseries and then uses
++        `switch_branches` to move the underlying bzr branch to the new series
++        and replace the old branch with a branch stacked on the new series'
++        branch.
++
++        :param old_db_branch: The branch to copy into the new distroseries.
++        :raises BranchExists: This will be raised if old_db_branch has already
++            been copied to the new distroseries (in the database, at least).
++        """
++        if not self.checkConsistentOfficialPackageBranch(old_db_branch):
++            self.logger.warning("Skipping branch")
++            return
++        new_namespace = getUtility(IBranchNamespaceSet).get(
++            person=old_db_branch.owner, product=None,
++            distroseries=self.new_distroseries,
++            sourcepackagename=old_db_branch.sourcepackagename)
++        new_db_branch = new_namespace.createBranch(
++            BranchType.HOSTED, old_db_branch.name, old_db_branch.registrant)
++        new_db_branch.sourcepackage.setBranch(
++            PackagePublishingPocket.RELEASE, new_db_branch,
++            getUtility(ILaunchpadCelebrities).ubuntu_branches.teamowner)
++        # switch_branches *moves* the data to locations dependent on the
++        # new_branch's id, so if the transaction was rolled back we wouldn't
++        # know the branch id and thus wouldn't be able to find the branch data
++        # again.  So commit before doing that.
++        transaction.commit()
++        switch_branches(
++            config.codehosting.hosted_branches_root,
++            'lp-hosted', old_db_branch, new_db_branch)
++        switch_branches(
++            config.codehosting.mirrored_branches_root,
++            'lp-mirrored', old_db_branch, new_db_branch)
++        return new_db_branch
 === added file 'lib/lp/codehosting/tests/test_branchdistro.py'
 --- lib/lp/codehosting/tests/test_branchdistro.py	1970-01-01 00:00:00 +0000
 +++ lib/lp/codehosting/tests/test_branchdistro.py	2009-10-14 00:31:13 +0000
@@ -0,0 +1,685 @@
++# Copyright 2009 Canonical Ltd.  This software is licensed under the
++# GNU Affero General Public License version 3 (see the file LICENSE).
++
++"""Tests for making new source package branches just after a distro release.
++"""
++
++__metaclass__ = type
++
++import os
++import re
++from StringIO import StringIO
++from subprocess import PIPE, Popen, STDOUT
++import textwrap
++import unittest
++
++from bzrlib.branch import Branch
++from bzrlib.bzrdir import BzrDir
++from bzrlib.errors import NotStacked
++from bzrlib.tests import TestCaseWithTransport
++from bzrlib.transport import get_transport
++from bzrlib.transport.chroot import ChrootServer
++
++from lazr.uri import URI
++
++import transaction
++
++from canonical.config import config
++from canonical.testing.layers import ZopelessAppServerLayer
++from canonical.launchpad.scripts.logger import FakeLogger, QuietFakeLogger
++
++from lp.codehosting.branchdistro import DistroBrancher, switch_branches
++from lp.codehosting.vfs import branch_id_to_path
++from lp.registry.interfaces.pocket import PackagePublishingPocket
++from lp.testing import TestCaseWithFactory
++
++
++# We say "RELEASE" often enough to not want to say "PackagePublishingPocket."
++# each time.
++RELEASE = PackagePublishingPocket.RELEASE
++
++
++class FakeBranch:
++    """Just enough of a Branch to pass `test_switch_branches`."""
++
++    def __init__(self, id):
++        self.id = id
++
++    @property
++    def unique_name(self):
++        return branch_id_to_path(self.id)
++
++
++class TestSwitchBranches(TestCaseWithTransport):
++    """Tests for `switch_branches`."""
++
++    def test_switch_branches(self):
++        # switch_branches moves a branch to the new location and places a
++        # branch (with no revisions) stacked on the new branch in the old
++        # location.
++
++        chroot_server = ChrootServer(self.get_transport())
++        chroot_server.setUp()
++        self.addCleanup(chroot_server.tearDown)
++        scheme = chroot_server.get_url().rstrip('/:')
++
++        old_branch = FakeBranch(1)
++        self.get_transport(old_branch.unique_name).create_prefix()
++        tree = self.make_branch_and_tree(old_branch.unique_name)
++        tree.commit(message='.')
++
++        new_branch = FakeBranch(2)
++
++        switch_branches('.', scheme, old_branch, new_branch)
++
++        # Post conditions:
++        # 1. unstacked branch in new_branch's location
++        # 2. stacked branch with no revisions in repo at old_branch
++        # 3. last_revision() the same for two branches
++
++        old_location_bzrdir = BzrDir.open(str(URI(
++            scheme=scheme, host='', path='/' + old_branch.unique_name)))
++        new_location_bzrdir = BzrDir.open(str(URI(
++            scheme=scheme, host='', path='/' + new_branch.unique_name)))
++
++        old_location_branch = old_location_bzrdir.open_branch()
++        new_location_branch = new_location_bzrdir.open_branch()
++
++        # 1. unstacked branch in new_branch's location
++        self.assertRaises(NotStacked, new_location_branch.get_stacked_on_url)
++
++        # 2. stacked branch with no revisions in repo at old_branch
++        self.assertEqual(
++            '/' + new_branch.unique_name,
++            old_location_branch.get_stacked_on_url())
++        self.assertEqual(
++            [], old_location_bzrdir.open_repository().all_revision_ids())
++
++        # 3. last_revision() the same for two branches
++        self.assertEqual(
++            old_location_branch.last_revision(),
++            new_location_branch.last_revision())
++
++
++class TestDistroBrancher(TestCaseWithFactory):
++    """Tests for `DistroBrancher`."""
++
++    layer = ZopelessAppServerLayer
++
++    def setUp(self):
++        TestCaseWithFactory.setUp(self)
++        self.useBzrBranches(real_server=True)
++
++    def makeOfficialPackageBranch(self, distroseries=None):
++        """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)
++
++        transaction.commit()
++
++        _, tree = self.create_branch_and_tree(
++            tree_location=self.factory.getUniqueString(), db_branch=db_branch,
++            hosted=True)
++        tree.commit('')
++        mirrored_branch = BzrDir.create_branch_convenience(
++            db_branch.warehouse_url)
++        mirrored_branch.pull(tree.branch)
++
++        return db_branch
++
++    def makeNewSeriesAndBrancher(self, distroseries=None):
++        """Make a DistroBrancher.
++
++        Any messages logged by this DistroBrancher can be checked by calling
++        `assertLogMessages` below.
++        """
++        if distroseries is None:
++            distroseries = self.factory.makeDistroRelease()
++        self._log_file = StringIO()
++        new_distroseries = self.factory.makeDistroRelease(
++            distribution=distroseries.distribution, name='new')
++        transaction.commit()
++        self.layer.switchDbUser('branch-distro')
++        return DistroBrancher(
++            FakeLogger(self._log_file), distroseries, new_distroseries)
++
++    def clearLogMessages(self):
++        """Forget about all logged messages seen so far."""
++        self._log_file.seek(0, 0)
++        self._log_file.truncate()
++
++    def assertLogMessages(self, patterns):
++        """Assert that the messages logged meet expectations.
++
++        :param patterns: A list of regular expressions.  The length must match
++            the number of messages logged, and then each pattern must match
++            the messages logged in order.
++        """
++        log_messages = self._log_file.getvalue().splitlines()
++        if len(log_messages) > len(patterns):
++            self.fail(
++                "More log messages (%s) than expected (%s)" %
++                (log_messages, patterns))
++        elif len(log_messages) < len(patterns):
++            self.fail(
++                "Fewer log messages (%s) than expected (%s)" %
++                (log_messages, patterns))
++        for pattern, message in zip(patterns, log_messages):
++            if not re.match(pattern, message):
++                self.fail("%r does not match %r" % (pattern, message))
++
++    def test_DistroBrancher_same_distro_check(self):
++        # DistroBrancher.__init__ raises AssertionError if the two
++        # distroseries passed are not from the same distribution.
++        self.assertRaises(
++            AssertionError, DistroBrancher, None,
++            self.factory.makeDistroRelease(),
++            self.factory.makeDistroRelease())
++
++    def test_DistroBrancher_same_distroseries_check(self):
++        # DistroBrancher.__init__ raises AssertionError if passed the same
++        # distroseries twice.
++        distroseries = self.factory.makeDistroRelease()
++        self.assertRaises(
++            AssertionError, DistroBrancher, None, distroseries, distroseries)
++
++    def test_fromNames(self):
++        # DistroBrancher.fromNames constructs a DistroBrancher from the names
++        # of a distribution and two distroseries within it.
++        distribution = self.factory.makeDistribution()
++        distroseries1 = self.factory.makeDistroRelease(
++            distribution=distribution)
++        distroseries2 = self.factory.makeDistroRelease(
++            distribution=distribution)
++        brancher = DistroBrancher.fromNames(
++            None, distribution.name, distroseries1.name, distroseries2.name)
++        self.assertEqual(
++            [distroseries1, distroseries2],
++            [brancher.old_distroseries, brancher.new_distroseries])
++
++    # A word on testing strategy: we don't directly test the post conditions
++    # of makeOneNewBranch, but we do test that it satisfies checkOneBranch and
++    # the tests for checkOneBranch verify that this function rejects various
++    # ways in which makeOneNewBranch could conceivably fail.
++
++    def test_makeOneNewBranch(self):
++        # makeOneNewBranch creates an official package branch in the new
++        # distroseries.
++        db_branch = self.makeOfficialPackageBranch()
++
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++
++        new_branch = brancher.new_distroseries.getSourcePackage(
++            db_branch.sourcepackage.name).getBranch(RELEASE)
++
++        self.assertIsNot(None, new_branch)
++        # The branch owner/name/target is the same, apart from the
++        # distroseries.
++        self.assertEqual(
++            [db_branch.owner, db_branch.distribution,
++             db_branch.sourcepackagename, db_branch.name],
++            [new_branch.owner, new_branch.distribution,
++             new_branch.sourcepackagename, new_branch.name])
++
++    def test_makeOneNewBranch_inconsistent_branch(self):
++        # makeOneNewBranch skips over an inconsistent official package branch
++        # (see `checkConsistentOfficialPackageBranch` for precisely what an
++        # "inconsistent official package branch" is).
++        unofficial_branch = self.factory.makePackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(
++            unofficial_branch.distroseries)
++        brancher.makeOneNewBranch(unofficial_branch)
++
++        new_branch = brancher.new_distroseries.getSourcePackage(
++            unofficial_branch.sourcepackage.name).getBranch(RELEASE)
++        self.assertIs(None, new_branch)
++        self.assertLogMessages(
++            ['^WARNING .* is not an official branch$',
++             '^WARNING Skipping branch$'])
++
++    def test_makeNewBranches(self):
++        # makeNewBranches calls makeOneNewBranch for each official branch in
++        # the old distroseries.
++        db_branch = self.makeOfficialPackageBranch()
++        db_branch2 = self.makeOfficialPackageBranch(
++            distroseries=db_branch.distroseries)
++
++        new_distroseries = self.factory.makeDistroRelease(
++            distribution=db_branch.distribution)
++
++        brancher = DistroBrancher(
++            QuietFakeLogger(), db_branch.distroseries, new_distroseries)
++
++        brancher.makeNewBranches()
++
++        new_sourcepackage = new_distroseries.getSourcePackage(
++            db_branch.sourcepackage.name)
++        new_branch = new_sourcepackage.getBranch(RELEASE)
++        new_sourcepackage2 = new_distroseries.getSourcePackage(
++            db_branch2.sourcepackage.name)
++        new_branch2 = new_sourcepackage2.getBranch(RELEASE)
++
++        self.assertIsNot(None, new_branch)
++        self.assertIsNot(None, new_branch2)
++
++    def test_makeNewBranches_idempotent(self):
++        # makeNewBranches is idempotent in the sense that if a branch in the
++        # old distroseries already has a counterpart in the new distroseries,
++        # it is silently ignored.
++        db_branch = self.makeOfficialPackageBranch()
++
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeNewBranches()
++        brancher.makeNewBranches()
++
++        new_branch = brancher.new_distroseries.getSourcePackage(
++            db_branch.sourcepackage.name).getBranch(RELEASE)
++
++        self.assertIsNot(new_branch, None)
++
++    def test_makeOneNewBranch_checks_ok(self):
++        # After calling makeOneNewBranch for a branch, calling checkOneBranch
++        # returns True for that branch.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        self.clearLogMessages()
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertLogMessages([])
++        self.assertTrue(ok)
++
++    def test_checkConsistentOfficialPackageBranch_product_branch(self):
++        # checkConsistentOfficialPackageBranch returns False when passed a
++        # product branch.
++        db_branch = self.factory.makeProductBranch()
++        brancher = self.makeNewSeriesAndBrancher()
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Encountered unexpected product branch .*/.*/.*$'])
++        self.assertFalse(ok)
++
++    def test_checkConsistentOfficialPackageBranch_personal_branch(self):
++        # checkConsistentOfficialPackageBranch returns False when passed a
++        # personal branch.
++        db_branch = self.factory.makePersonalBranch()
++        brancher = self.makeNewSeriesAndBrancher()
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Encountered unexpected personal branch .*/.*/.*$'])
++        self.assertFalse(ok)
++
++    def test_checkConsistentOfficialPackageBranch_no_official_branch(self):
++        # checkConsistentOfficialPackageBranch returns False when passed a
++        # branch which is not official for any package.
++        db_branch = self.factory.makePackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages(
++            ['^WARNING .*/.*/.* is not an official branch$'])
++        self.assertFalse(ok)
++
++    def test_checkConsistentOfficialPackageBranch_official_elsewhere(self):
++        # checkConsistentOfficialPackageBranch returns False when passed a
++        # branch which is official for a sourcepackage that it is not a branch
++        # for.
++        db_branch = self.factory.makePackageBranch()
++        self.factory.makeSourcePackage().setBranch(
++            RELEASE, db_branch, db_branch.owner)
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages(
++            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
++             'its sourcepackage$'])
++        self.assertFalse(ok)
++
++    def test_checkConsistentOfficialPackageBranch_official_twice(self):
++        # checkConsistentOfficialPackageBranch returns False when passed a
++        # branch that is official for two sourcepackages.
++        db_branch = self.factory.makePackageBranch()
++        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
++        self.factory.makeSourcePackage().setBranch(
++            RELEASE, db_branch, db_branch.owner)
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING .*/.*/.* is official for multiple series: .*/.*/.*, '
++            '.*/.*/.*$'])
++        self.assertFalse(ok)
++
++    def test_checkConsistentOfficialPackageBranch_ok(self):
++        # checkConsistentOfficialPackageBranch returns True when passed a
++        # branch that is official for its sourcepackage and no other.
++        db_branch = self.factory.makePackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        db_branch.sourcepackage.setBranch(RELEASE, db_branch, db_branch.owner)
++        ok = brancher.checkConsistentOfficialPackageBranch(db_branch)
++        self.assertLogMessages([])
++        self.assertTrue(ok)
++
++    def test_checkOneBranch_inconsistent_old_package_branch(self):
++        # checkOneBranch returns False when passed a branch that is not a
++        # consistent official package branch.
++        db_branch = self.factory.makePackageBranch()
++        brancher = self.makeNewSeriesAndBrancher()
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertFalse(ok)
++        self.assertLogMessages(
++            ['^WARNING .*/.*/.* is not an official branch$'])
++
++    def test_checkOneBranch_no_new_official_branch(self):
++        # checkOneBranch returns False when there is no corresponding official
++        # package branch in the new distroseries.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertFalse(ok)
++        self.assertLogMessages(
++            ['^WARNING No official branch found for .*/.*/.*$'])
++
++    def test_checkOneBranch_inconsistent_new_package_branch(self):
++        # checkOneBranch returns False when the corresponding official package
++        # branch in the new distroseries is not consistent.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        new_db_branch = brancher.makeOneNewBranch(db_branch)
++        self.layer.switchDbUser('launchpad')
++        new_db_branch.setTarget(
++            new_db_branch.owner,
++            source_package=self.factory.makeSourcePackage())
++        transaction.commit()
++        self.layer.switchDbUser('branch-distro')
++        ok = brancher.checkOneBranch(new_db_branch)
++        self.assertFalse(ok)
++        self.assertLogMessages(
++            ['^WARNING .*/.*/.* is the official branch for .*/.*/.* but not '
++             'its sourcepackage$'])
++
++    def checkOneBranch_new_branch_missing(self, branch_type):
++        # checkOneBranch returns False when there is no bzr branch for the
++        # database branch in the new distroseries.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        new_db_branch = brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = new_db_branch.getPullURL()
++        else:
++            url = new_db_branch.warehouse_url
++        get_transport(url).delete_tree('.bzr')
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertFalse(ok)
++        # Deleting the new branch will break the old branch, as that's stacked
++        # on the new one.
++        self.assertLogMessages([
++            '^WARNING No bzr branch at new location lp-%s:///.*/.*/.*/.*$'
++            % branch_type,
++            '^WARNING No bzr branch at old location lp-%s:///.*/.*/.*/.*$'
++            % branch_type,
++            ])
++
++    def test_checkOneBranch_new_hosted_branch_missing(self):
++        # checkOneBranch returns False when there is no bzr branch in the
++        # hosted area for the database branch in the new distroseries.
++        self.checkOneBranch_new_branch_missing('hosted')
++
++    def test_checkOneBranch_new_mirrored_branch_missing(self):
++        # checkOneBranch returns False when there is no bzr branch in the
++        # mirrored area for the database branch in the new distroseries.
++        self.checkOneBranch_new_branch_missing('mirrored')
++
++    def checkOneBranch_old_branch_missing(self, branch_type):
++        # checkOneBranch returns False when there is no bzr branchfor the
++        # database branch in old distroseries.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = db_branch.getPullURL()
++        else:
++            url = db_branch.warehouse_url
++        get_transport(url).delete_tree('.bzr')
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertFalse(ok)
++        self.assertLogMessages([
++            '^WARNING No bzr branch at old location lp-%s:///.*/.*/.*/.*$'
++            % branch_type,
++            ])
++
++    def test_checkOneBranch_old_hosted_branch_missing(self):
++        # checkOneBranch returns False when there is no bzr branch in the
++        # hosted area for the database branch in old distroseries.
++        self.checkOneBranch_old_branch_missing('hosted')
++
++    def test_checkOneBranch_old_mirrored_branch_missing(self):
++        # checkOneBranch returns False when there is no bzr branch in the
++        # mirrored area for the database branch in old distroseries.
++        self.checkOneBranch_old_branch_missing('mirrored')
++
++    def checkOneBranch_new_stacked(self, branch_type):
++        # checkOneBranch returns False when the bzr branch for the database
++        # branch in new distroseries is stacked.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        b, _ = self.create_branch_and_tree(
++            self.factory.getUniqueString(), hosted=(branch_type == 'hosted'))
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        new_db_branch = brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = new_db_branch.getPullURL()
++        else:
++            url = new_db_branch.warehouse_url
++        Branch.open(url).set_stacked_on_url('/' + b.unique_name)
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertFalse(ok)
++        self.assertLogMessages([
++            '^WARNING New branch at lp-%s:///.*/.*/.*/.* is stacked on '
++            '/.*/.*/.*, should be unstacked.$' % branch_type,
++            ])
++
++    def test_checkOneBranch_new_hosted_stacked(self):
++        # checkOneBranch returns False when the bzr branch in the hosted area
++        # for the database branch in new distroseries is stacked.
++        self.checkOneBranch_new_stacked('hosted')
++
++    def test_checkOneBranch_new_mirrored_stacked(self):
++        # checkOneBranch returns False when the bzr branch in the mirrored
++        # area for the database branch in new distroseries is stacked.
++        self.checkOneBranch_new_stacked('mirrored')
++
++    def checkOneBranch_old_unstacked(self, branch_type):
++        # checkOneBranch returns False when the bzr branch for the database
++        # branch in old distroseries is not stacked.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = db_branch.getPullURL()
++        else:
++            url = db_branch.warehouse_url
++        old_bzr_branch = Branch.open(url)
++        old_bzr_branch.set_stacked_on_url(None)
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Old branch at lp-%s:///.*/.*/.*/.* is not stacked, '
++            'should be stacked on /.*/.*/.*.$' % branch_type,
++            '^.*has .* revisions.*$',
++            ])
++        self.assertFalse(ok)
++
++    def test_checkOneBranch_old_hosted_unstacked(self):
++        # checkOneBranch returns False when the bzr branch in the hosted area
++        # for the database branch in old distroseries is not stacked.
++        self.checkOneBranch_old_unstacked('hosted')
++
++    def test_checkOneBranch_old_mirrored_unstacked(self):
++        # checkOneBranch returns False when the bzr branch in the mirrored
++        # area for the database branch in old distroseries is not stacked.
++        self.checkOneBranch_old_unstacked('mirrored')
++
++    def checkOneBranch_old_misstacked(self, branch_type):
++        # checkOneBranch returns False when the bzr branch for the database
++        # branch in old distroseries stacked on some other branch than the
++        # branch in the new distroseries.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        b, _ = self.create_branch_and_tree(
++            self.factory.getUniqueString(), hosted=(branch_type == 'hosted'))
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = db_branch.getPullURL()
++        else:
++            url = db_branch.warehouse_url
++        Branch.open(url).set_stacked_on_url('/' + b.unique_name)
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Old branch at lp-%s:///.*/.*/.*/.* is stacked on '
++            '/.*/.*/.*, should be stacked on /.*/.*/.*.$' % branch_type,
++            ])
++        self.assertFalse(ok)
++
++    def test_checkOneBranch_old_hosted_misstacked(self):
++        # checkOneBranch returns False when the bzr branch in the hosted area
++        # for the database branch in old distroseries stacked on some other
++        # branch than the branch in the new distroseries.
++        self.checkOneBranch_old_misstacked('hosted')
++
++    def test_checkOneBranch_old_mirrored_misstacked(self):
++        # checkOneBranch returns False when the bzr branch in the mirrored
++        # area for the database branch in old distroseries stacked on some
++        # other branch than the branch in the new distroseries.
++        self.checkOneBranch_old_misstacked('mirrored')
++
++    def checkOneBranch_old_has_revisions(self, branch_type):
++        # checkOneBranch returns False when the bzr branch for the database
++        # branch in old distroseries has a repository that contains revisions.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = db_branch.getPullURL()
++        else:
++            url = db_branch.warehouse_url
++        old_bzr_branch = Branch.open(url)
++        old_bzr_branch.create_checkout(
++            self.factory.getUniqueString()).commit('')
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Repository at lp-%s:///.*/.*/.*/.* has 1 revisions.'
++            % branch_type
++            ])
++        self.assertFalse(ok)
++
++    def test_checkOneBranch_old_hosted_has_revisions(self):
++        # checkOneBranch returns False when the bzr branch in the hosted area
++        # for the database branch in old distroseries has a repository that
++        # contains revisions.
++        self.checkOneBranch_old_has_revisions('hosted')
++
++    def test_checkOneBranch_old_mirrored_has_revisions(self):
++        # checkOneBranch returns False when the bzr branch in the mirrored
++        # area for the database branch in old distroseries has a repository
++        # that contains revisions.
++        self.checkOneBranch_old_has_revisions('mirrored')
++
++    def checkOneBranch_old_has_null_tip(self, branch_type):
++        # checkOneBranch returns False when the bzr branch for the database
++        # branch in old distroseries has tip revision of 'null:'.
++        assert branch_type in ('hosted', 'mirrored')
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeOneNewBranch(db_branch)
++        if branch_type == 'hosted':
++            url = db_branch.getPullURL()
++        else:
++            url = db_branch.warehouse_url
++        old_bzr_branch = Branch.open(url)
++        old_bzr_branch.set_last_revision_info(0, 'null:')
++        ok = brancher.checkOneBranch(db_branch)
++        self.assertLogMessages([
++            '^WARNING Old branch at lp-%s:///.*/.*/.*/.* has null tip '
++            'revision.' % branch_type
++            ])
++        self.assertFalse(ok)
++
++    def test_checkOneBranch_old_hosted_has_null_tip(self):
++        # checkOneBranch returns False when the bzr branch in the hosted area
++        # for the database branch in old distroseries has tip revision of
++        # 'null:'.
++        self.checkOneBranch_old_has_null_tip('hosted')
++
++    def test_checkOneBranch_old_mirrored_has_null_tip(self):
++        # checkOneBranch returns False when the bzr branch in the mirrored
++        # area for the database branch in old distroseries has tip revision of
++        # 'null:'.
++        self.checkOneBranch_old_has_null_tip('mirrored')
++
++    def runBranchDistroScript(self, args):
++        """Run the branch-distro.py script with the given arguments.
++
++        ;param args: The arguments to pass to the branch-distro.py script.
++        :return: A tuple (returncode, output).  stderr and stdout are both
++            contained in the output.
++        """
++        script_path = os.path.join(config.root, 'scripts', 'branch-distro.py')
++        process = Popen([script_path] + args, stdout=PIPE, stderr=STDOUT)
++        output, error = process.communicate()
++        return process.returncode, output
++
++    def test_makeNewBranches_script(self):
++        # Running the script with the arguments 'distro old-series new-series'
++        # makes new branches in the new series.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        returncode, output = self.runBranchDistroScript(
++            ['-v', db_branch.distribution.name,
++             brancher.old_distroseries.name, brancher.new_distroseries.name])
++        self.assertEqual(0, returncode)
++        self.assertEqual(
++            'DEBUG   Processing ' + db_branch.unique_name + '\n', output)
++        brancher.checkOneBranch(db_branch)
++
++    def test_checkNewBranches_script_success(self):
++        # Running the script with the arguments '--check distro old-series
++        # new-series' checks that the branches in the new series are as
++        # expected.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        brancher.makeNewBranches()
++        returncode, output = self.runBranchDistroScript(
++            ['-v', '--check', db_branch.distribution.name,
++             brancher.old_distroseries.name, brancher.new_distroseries.name])
++        self.assertEqual(0, returncode)
++        self.assertEqual(
++            'DEBUG   Checking ' + db_branch.unique_name + '\n', output)
++        brancher.checkOneBranch(db_branch)
++
++    def test_checkNewBranches_script_failure(self):
++        # Running the script with the arguments '--check distro old-series
++        # new-series' checks that the branches in the new series are as
++        # expected and logs warnings and exits with code 1 is things are not
++        # as expected.
++        db_branch = self.makeOfficialPackageBranch()
++        brancher = self.makeNewSeriesAndBrancher(db_branch.distroseries)
++        returncode, output = self.runBranchDistroScript(
++            ['-v', '--check', db_branch.distribution.name,
++             brancher.old_distroseries.name, brancher.new_distroseries.name])
++        sp_path = brancher.new_distroseries.getSourcePackage(
++            db_branch.sourcepackagename).path
++        expected = '''\
++        DEBUG   Checking %(branch_name)s
++        WARNING No official branch found for %(sp_path)s
++        ERROR   Check failed
++        ''' % {'branch_name': db_branch.unique_name, 'sp_path': sp_path}
++        self.assertEqual(
++            textwrap.dedent(expected), output)
++        self.assertEqual(1, returncode)
++
++
++def test_suite():
++    return unittest.TestLoader().loadTestsFromName(__name__)
++
 === added file 'scripts/branch-distro.py'
 --- scripts/branch-distro.py	1970-01-01 00:00:00 +0000
 +++ scripts/branch-distro.py	2009-10-14 00:31:13 +0000
@@ -0,0 +1,39 @@
++#!/usr/bin/python2.4
++#
++# Copyright 2009 Canonical Ltd.  This software is licensed under the
++# GNU Affero General Public License version 3 (see the file LICENSE).
++
++import _pythonpath
++
++from lp.codehosting.branchdistro import DistroBrancher
++from lp.codehosting.vfs import get_multi_server
++from lp.services.scripts.base import LaunchpadScript, LaunchpadScriptFailure
++
++
++class BranchDistroScript(LaunchpadScript):
++
++    usage = "%prog distro old-series new-series"
++
++    def add_my_options(self):
++        self.parser.add_option(
++            '--check', dest="check", action="store_true", default=False,
++            help=("Check that the new distro series has its official "
++                  "branches set up correctly."))
++
++    def main(self):
++        if len(self.args) != 3:
++            self.parser.error("Wrong number of arguments.")
++        brancher = DistroBrancher.fromNames(self.logger, *self.args)
++        server = get_multi_server(write_mirrored=True, write_hosted=True)
++        server.setUp()
++        try:
++            if self.options.check:
++                if not brancher.checkNewBranches():
++                    raise LaunchpadScriptFailure("Check failed")
++            else:
++                brancher.makeNewBranches()
++        finally:
++            server.tearDown()
++
++if __name__ == '__main__':
++    BranchDistroScript("branch-distro", dbuser='branch-distro').run()

Launchpad itself

Merge lp:~mwhudson/launchpad/branching-ubuntu into lp:launchpad

Commit message

Description of the change

Preview Diff

Subscribers