launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05088
[Merge] lp:~jelmer/launchpad/stacked-code-imports into lp:launchpad
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/stacked-code-imports into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #485932 in Launchpad itself: "should stack foreign branch imports"
https://bugs.launchpad.net/launchpad/+bug/485932
For more details, see:
https://code.launchpad.net/~jelmer/launchpad/stacked-code-imports/+merge/76998
Stack code imports on the default development focus branch.
This should make imports of branches that are related to the development focus a lot faster, and use less disk space.
This will help significantly with e.g. imports of new gcc branches.
For now, this only stacks onto public branches.
--
https://code.launchpad.net/~jelmer/launchpad/stacked-code-imports/+merge/76998
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/stacked-code-imports into lp:launchpad.
=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py 2011-09-19 06:57:55 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py 2011-09-26 15:09:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the code import worker."""
@@ -49,7 +49,14 @@
import subvertpy.ra
from canonical.config import config
-from canonical.testing.layers import BaseLayer
+from canonical.testing.layers import (
+ BaseLayer,
+ DatabaseFunctionalLayer,
+ )
+from lp.code.interfaces.codehosting import (
+ branch_id_alias,
+ compose_public_url,
+ )
from lp.codehosting import load_optional_plugin
from lp.codehosting.codeimport.tarball import (
create_tarball,
@@ -67,6 +74,7 @@
BzrImportWorker,
BzrSvnImportWorker,
CodeImportBranchOpenPolicy,
+ CodeImportSourceDetails,
CodeImportWorkerExitCode,
CSCVSImportWorker,
ForeignTreeStore,
@@ -85,7 +93,11 @@
)
from lp.codehosting.tests.helpers import create_branch_with_one_revision
from lp.services.log.logger import BufferLogger
-from lp.testing import TestCase
+from lp.testing import (
+ TestCase,
+ TestCaseWithFactory,
+ )
+from zope.security.proxy import removeSecurityProxy
class ForeignBranchPluginLayer(BaseLayer):
@@ -807,7 +819,7 @@
raise NotImplementedError(
"Override this with a VCS-specific implementation.")
- def makeSourceDetails(self, module_name, files):
+ def makeSourceDetails(self, module_name, files, stacked_on_url=None):
"""Make a `CodeImportSourceDetails` that points to a real repository.
This should set `self.foreign_commit_count` to an appropriate value.
@@ -954,7 +966,7 @@
self.foreign_commit_count += 1
shutil.rmtree('working_dir')
- def makeSourceDetails(self, module_name, files):
+ def makeSourceDetails(self, module_name, files, stacked_on_url=None):
"""Make a CVS `CodeImportSourceDetails` pointing at a real CVS repo.
"""
cvs_server = CVSServer(self.makeTemporaryDirectory())
@@ -966,7 +978,8 @@
self.foreign_commit_count = 2
return self.factory.makeCodeImportSourceDetails(
- rcstype='cvs', cvs_root=cvs_server.getRoot(), cvs_module='trunk')
+ rcstype='cvs', cvs_root=cvs_server.getRoot(), cvs_module='trunk',
+ stacked_on_url=stacked_on_url)
class SubversionImportHelpers:
@@ -991,7 +1004,7 @@
self.foreign_commit_count += 1
shutil.rmtree('working_tree')
- def makeSourceDetails(self, branch_name, files):
+ def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
"""Make a SVN `CodeImportSourceDetails` pointing at a real SVN repo.
"""
svn_server = SubversionServer(self.makeTemporaryDirectory())
@@ -1002,7 +1015,8 @@
svn_branch_url = svn_branch_url.replace('://localhost/', ':///')
self.foreign_commit_count = 2
return self.factory.makeCodeImportSourceDetails(
- rcstype=self.rcstype, url=svn_branch_url)
+ rcstype=self.rcstype, url=svn_branch_url,
+ stacked_on_url=stacked_on_url)
class TestSubversionImport(WorkerTest, SubversionImportHelpers,
@@ -1092,6 +1106,26 @@
self.assertEqual(
CodeImportWorkerExitCode.SUCCESS, worker.run())
+ def test_stacked(self):
+ stacked_on = self.make_branch('stacked-on')
+ source_details = self.makeSourceDetails(
+ 'trunk', [('README', 'Original contents')],
+ stacked_on_url=stacked_on.base)
+ stacked_on.fetch(Branch.open(source_details.url))
+ self.assertTrue(len(stacked_on.repository.all_revision_ids()) > 0)
+ worker = self.makeImportWorker(
+ source_details,
+ opener_policy=AcceptAnythingPolicy())
+ self.makeForeignCommit(source_details)
+ self.assertEqual(
+ CodeImportWorkerExitCode.SUCCESS, worker.run())
+ branch = self.getStoredBazaarBranch(worker)
+ # There should only be one revision there, the other
+ # one is in the stacked-on repository.
+ self.addCleanup(stacked_on.lock_read().unlock)
+ self.assertEquals(1, len(stacked_on.repository.revisions.keys()))
+ self.assertEquals(stacked_on.base, branch.get_stacked_on_url())
+
class TestGitImport(WorkerTest, TestActualImportMixin,
PullingImportWorkerTests):
@@ -1131,7 +1165,7 @@
committer="Joe Random Hacker <joe@xxxxxxxxxxx>", ref=ref)
self.foreign_commit_count += 1
- def makeSourceDetails(self, branch_name, files):
+ def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
"""Make a Git `CodeImportSourceDetails` pointing at a real Git repo.
"""
repository_path = self.makeTemporaryDirectory()
@@ -1143,7 +1177,8 @@
self.foreign_commit_count = 1
return self.factory.makeCodeImportSourceDetails(
- rcstype='git', url=git_server.get_url())
+ rcstype='git', url=git_server.get_url(),
+ stacked_on_url=stacked_on_url)
def test_non_master(self):
# non-master branches can be specified in the import URL.
@@ -1212,7 +1247,7 @@
text=message, user="Jane Random Hacker", force=1, extra=extra)
self.foreign_commit_count += 1
- def makeSourceDetails(self, branch_name, files):
+ def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
"""Make a Mercurial `CodeImportSourceDetails` pointing at a real repo.
"""
repository_path = self.makeTemporaryDirectory()
@@ -1224,7 +1259,8 @@
self.foreign_commit_count = 1
return self.factory.makeCodeImportSourceDetails(
- rcstype='hg', url=hg_server.get_url())
+ rcstype='hg', url=hg_server.get_url(),
+ stacked_on_url=stacked_on_url)
def test_non_default(self):
# non-default branches can be specified in the import URL.
@@ -1291,7 +1327,7 @@
committer="Joe Random Hacker <joe@xxxxxxxxxxx>")
self.foreign_commit_count += 1
- def makeSourceDetails(self, branch_name, files):
+ def makeSourceDetails(self, branch_name, files, stacked_on_url=None):
"""Make Bzr `CodeImportSourceDetails` pointing at a real Bzr repo.
"""
repository_path = self.makeTemporaryDirectory()
@@ -1303,7 +1339,8 @@
self.foreign_commit_count = 1
return self.factory.makeCodeImportSourceDetails(
- rcstype='bzr', url=bzr_server.get_url())
+ rcstype='bzr', url=bzr_server.get_url(),
+ stacked_on_url=stacked_on_url)
def test_partial(self):
self.skip(
@@ -1441,3 +1478,88 @@
self.old_server.stop_server()
self.assertEqual(
CodeImportWorkerExitCode.FAILURE_INVALID, worker.run())
+
+
+class CodeImportSourceDetailsTests(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ # Use an admin user as we aren't checking edit permissions here.
+ TestCaseWithFactory.setUp(self, 'admin@xxxxxxxxxxxxx')
+
+ def test_bzr_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ bzr_branch_url="http://example.com/foo";)
+ arguments = CodeImportSourceDetails.fromCodeImport(
+ code_import).asArguments()
+ self.assertEquals([
+ str(code_import.branch.id), 'bzr', 'http://example.com/foo'],
+ arguments)
+
+ def test_hg_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ hg_repo_url="http://example.com/foo";)
+ arguments = CodeImportSourceDetails.fromCodeImport(
+ code_import).asArguments()
+ self.assertEquals([
+ str(code_import.branch.id), 'hg', 'http://example.com/foo'],
+ arguments)
+
+ def test_git_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ git_repo_url="git://git.example.com/project.git")
+ arguments = CodeImportSourceDetails.fromCodeImport(
+ code_import).asArguments()
+ self.assertEquals([
+ str(code_import.branch.id), 'git',
+ 'git://git.example.com/project.git'],
+ arguments)
+
+ def test_cvs_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ cvs_root=':pserver:foo@xxxxxxxxxxx/bar', cvs_module='bar')
+ arguments = CodeImportSourceDetails.fromCodeImport(
+ code_import).asArguments()
+ self.assertEquals([
+ str(code_import.branch.id), 'cvs',
+ ':pserver:foo@xxxxxxxxxxx/bar', 'bar'],
+ arguments)
+
+ def test_svn_arguments(self):
+ code_import = self.factory.makeCodeImport(
+ svn_branch_url='svn://svn.example.com/trunk')
+ arguments = CodeImportSourceDetails.fromCodeImport(
+ code_import).asArguments()
+ self.assertEquals([
+ str(code_import.branch.id), 'svn',
+ 'svn://svn.example.com/trunk'],
+ arguments)
+
+ def test_bzr_stacked(self):
+ devfocus = self.factory.makeAnyBranch(private=False)
+ devfocus.default_stacked_on_branch = devfocus
+ code_import = self.factory.makeCodeImport(
+ bzr_branch_url='bzr://bzr.example.com/foo',
+ target=devfocus.target)
+ details = CodeImportSourceDetails.fromCodeImport(
+ code_import)
+ self.assertEquals([
+ str(code_import.branch.id), 'bzr',
+ 'bzr://bzr.example.com/foo',
+ compose_public_url('http', branch_id_alias(devfocus))],
+ details.asArguments())
+
+ def test_bzr_stacked_private(self):
+ # Code imports can't be stacked on private branches.
+ devfocus = self.factory.makeAnyBranch(private=True)
+ devfocus.default_stacked_on_branch = devfocus
+ code_import = self.factory.makeCodeImport(
+ target=devfocus.target,
+ bzr_branch_url='bzr://bzr.example.com/foo')
+ details = CodeImportSourceDetails.fromCodeImport(
+ code_import)
+ self.assertEquals([
+ str(code_import.branch.id), 'bzr',
+ 'bzr://bzr.example.com/foo'],
+ details.asArguments())
=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py 2011-09-19 06:57:55 +0000
+++ lib/lp/codehosting/codeimport/worker.py 2011-09-26 15:09:29 +0000
@@ -41,7 +41,8 @@
)
from bzrlib.transport import (
do_catching_redirections,
- get_transport,
+ get_transport_from_path,
+ get_transport_from_url,
)
import bzrlib.ui
from bzrlib.upgrade import upgrade
@@ -61,6 +62,10 @@
from canonical.config import config
from lp.code.enums import RevisionControlSystems
from lp.code.interfaces.branch import get_blacklisted_hostnames
+from lp.code.interfaces.codehosting import (
+ branch_id_alias,
+ compose_public_url,
+ )
from lp.codehosting.codeimport.foreigntree import (
CVSWorkingTree,
SubversionWorkingTree,
@@ -150,7 +155,7 @@
return urljoin(self.transport.base, '%08x' % db_branch_id)
def pull(self, db_branch_id, target_path, required_format,
- needs_tree=False):
+ needs_tree=False, stacked_on_url=None):
"""Pull down the Bazaar branch of an import to `target_path`.
:return: A Bazaar branch for the code import corresponding to the
@@ -164,6 +169,8 @@
target_path, format=required_format)
if needs_tree:
local_branch.bzrdir.create_workingtree()
+ if stacked_on_url:
+ local_branch.set_stacked_on_url(stacked_on_url)
return local_branch
# The proper thing to do here would be to call
# "remote_bzr_dir.sprout()". But 2a fetch slowly checks which
@@ -172,7 +179,7 @@
# at the vfs level.
control_dir = remote_bzr_dir.root_transport.relpath(
remote_bzr_dir.transport.abspath('.'))
- target = get_transport(target_path)
+ target = get_transport_from_path(target_path)
target_control = target.clone(control_dir)
target_control.create_prefix()
remote_bzr_dir.transport.copy_tree_to_transport(target_control)
@@ -187,7 +194,8 @@
local_bzr_dir.create_workingtree()
return local_bzr_dir.open_branch()
- def push(self, db_branch_id, bzr_branch, required_format):
+ def push(self, db_branch_id, bzr_branch, required_format,
+ stacked_on_url=None):
"""Push up `bzr_branch` as the Bazaar branch for `code_import`.
:return: A boolean that is true if the push was non-trivial
@@ -219,6 +227,10 @@
upgrade_url, format=required_format)
else:
old_branch = None
+ # This can be done safely, since only modern formats are used to import
+ # to.
+ if stacked_on_url is not None:
+ remote_branch.set_stacked_on_url(stacked_on_url)
pull_result = remote_branch.pull(bzr_branch, overwrite=True)
# Because of the way we do incremental imports, there may be revisions
# in the branch's repo that are not in the ancestry of the branch tip.
@@ -237,7 +249,7 @@
def get_default_bazaar_branch_store():
"""Return the default `BazaarBranchStore`."""
return BazaarBranchStore(
- get_transport(config.codeimport.bazaar_branch_store))
+ get_transport_from_url(config.codeimport.bazaar_branch_store))
class CodeImportSourceDetails:
@@ -260,12 +272,13 @@
"""
def __init__(self, branch_id, rcstype, url=None, cvs_root=None,
- cvs_module=None):
+ cvs_module=None, stacked_on_url=None):
self.branch_id = branch_id
self.rcstype = rcstype
self.url = url
self.cvs_root = cvs_root
self.cvs_module = cvs_module
+ self.stacked_on_url = stacked_on_url
@classmethod
def fromArguments(cls, arguments):
@@ -273,34 +286,55 @@
branch_id = int(arguments.pop(0))
rcstype = arguments.pop(0)
if rcstype in ['svn', 'bzr-svn', 'git', 'hg', 'bzr']:
- [url] = arguments
+ url = arguments.pop(0)
+ try:
+ stacked_on_url = arguments.pop(0)
+ except IndexError:
+ stacked_on_url = None
cvs_root = cvs_module = None
elif rcstype == 'cvs':
url = None
+ stacked_on_url = None
[cvs_root, cvs_module] = arguments
else:
raise AssertionError("Unknown rcstype %r." % rcstype)
- return cls(branch_id, rcstype, url, cvs_root, cvs_module)
+ return cls(
+ branch_id, rcstype, url, cvs_root, cvs_module, stacked_on_url)
@classmethod
def fromCodeImport(cls, code_import):
"""Convert a `CodeImport` to an instance."""
- branch_id = code_import.branch.id
+ branch = code_import.branch
+ if branch.stacked_on is not None and not branch.stacked_on.private:
+ stacked_path = branch_id_alias(branch.stacked_on)
+ stacked_on_url = compose_public_url('http', stacked_path)
+ else:
+ stacked_on_url = None
if code_import.rcs_type == RevisionControlSystems.SVN:
- return cls(branch_id, 'svn', str(code_import.url))
+ return cls(
+ branch.id, 'svn', str(code_import.url),
+ stacked_on_url=stacked_on_url)
elif code_import.rcs_type == RevisionControlSystems.BZR_SVN:
- return cls(branch_id, 'bzr-svn', str(code_import.url))
+ return cls(
+ branch.id, 'bzr-svn', str(code_import.url),
+ stacked_on_url=stacked_on_url)
elif code_import.rcs_type == RevisionControlSystems.CVS:
return cls(
- branch_id, 'cvs',
+ branch.id, 'cvs',
cvs_root=str(code_import.cvs_root),
cvs_module=str(code_import.cvs_module))
elif code_import.rcs_type == RevisionControlSystems.GIT:
- return cls(branch_id, 'git', str(code_import.url))
+ return cls(
+ branch.id, 'git', str(code_import.url),
+ stacked_on_url=stacked_on_url)
elif code_import.rcs_type == RevisionControlSystems.HG:
- return cls(branch_id, 'hg', str(code_import.url))
+ return cls(
+ branch.id, 'hg', str(code_import.url),
+ stacked_on_url=stacked_on_url)
elif code_import.rcs_type == RevisionControlSystems.BZR:
- return cls(branch_id, 'bzr', str(code_import.url))
+ return cls(
+ branch.id, 'bzr', str(code_import.url),
+ stacked_on_url=stacked_on_url)
else:
raise AssertionError("Unknown rcstype %r." % code_import.rcs_type)
@@ -310,6 +344,8 @@
result = [str(self.branch_id), self.rcstype]
if self.rcstype in ['svn', 'bzr-svn', 'git', 'hg', 'bzr']:
result.append(self.url)
+ if self.stacked_on_url is not None:
+ result.append(self.stacked_on_url)
elif self.rcstype == 'cvs':
result.append(self.cvs_root)
result.append(self.cvs_module)
@@ -367,12 +403,12 @@
:param filename: The name of the file to retrieve (must be a filename,
not a path).
:param dest_transport: The transport to retrieve the file to,
- defaulting to ``get_transport('.')``.
+ defaulting to ``get_transport_from_path('.')``.
:return: A boolean, true if the file was found and retrieved, false
otherwise.
"""
if dest_transport is None:
- dest_transport = get_transport('.')
+ dest_transport = get_transport_from_path('.')
remote_name = self._getRemoteName(filename)
if self._transport.has(remote_name):
dest_transport.put_file(
@@ -390,7 +426,7 @@
defaulting to ``get_transport('.')``.
"""
if source_transport is None:
- source_transport = get_transport('.')
+ source_transport = get_transport_from_path('.')
remote_name = self._getRemoteName(filename)
local_file = source_transport.get(filename)
self._transport.create_prefix()
@@ -506,7 +542,8 @@
shutil.rmtree(self.BZR_BRANCH_PATH)
return self.bazaar_branch_store.pull(
self.source_details.branch_id, self.BZR_BRANCH_PATH,
- self.required_format, self.needs_bzr_tree)
+ self.required_format, self.needs_bzr_tree,
+ stacked_on_url=self.source_details.stacked_on_url)
def pushBazaarBranch(self, bazaar_branch):
"""Push the updated Bazaar branch to the server.
@@ -515,7 +552,8 @@
"""
return self.bazaar_branch_store.push(
self.source_details.branch_id, bazaar_branch,
- self.required_format)
+ self.required_format,
+ stacked_on_url=self.source_details.stacked_on_url)
def getWorkingDirectory(self):
"""The directory we should change to and store all scratch files in.
@@ -699,7 +737,7 @@
last_error = e
else:
raise last_error
- transport = get_transport(url)
+ transport = get_transport_from_url(url)
transport, format = do_catching_redirections(find_format, transport,
redirected)
return format.open(transport)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-09-23 13:14:36 +0000
+++ lib/lp/testing/factory.py 2011-09-26 15:09:29 +0000
@@ -472,7 +472,8 @@
return epoch + timedelta(minutes=self.getUniqueInteger())
def makeCodeImportSourceDetails(self, branch_id=None, rcstype=None,
- url=None, cvs_root=None, cvs_module=None):
+ url=None, cvs_root=None, cvs_module=None,
+ stacked_on_url=None):
if branch_id is None:
branch_id = self.getUniqueInteger()
if rcstype is None:
@@ -494,7 +495,8 @@
else:
raise AssertionError("Unknown rcstype %r." % rcstype)
return CodeImportSourceDetails(
- branch_id, rcstype, url, cvs_root, cvs_module)
+ branch_id, rcstype, url, cvs_root, cvs_module,
+ stacked_on_url=stacked_on_url)
class BareLaunchpadObjectFactory(ObjectFactory):
