launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05940
[Merge] lp:~stevenk/launchpad/revert-r14491 into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/revert-r14491 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/revert-r14491/+merge/85789
Rollback r14491.
--
https://code.launchpad.net/~stevenk/launchpad/revert-r14491/+merge/85789
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/revert-r14491 into lp:launchpad.
=== added directory 'bzrplugins/optional'
=== added file 'bzrplugins/optional/README'
--- bzrplugins/optional/README 1970-01-01 00:00:00 +0000
+++ bzrplugins/optional/README 2011-12-15 04:01:27 +0000
@@ -0,0 +1,4 @@
+These are for plugins that we do *not* want loaded all of the time.
+
+Specifically, the code import system loads these plugins, but we do not want
+the branch puller or scanner loading them.
=== renamed symlink 'bzrplugins/git' => 'bzrplugins/optional/git'
=== target changed u'../sourcecode/bzr-git' => u'../../sourcecode/bzr-git/'
=== renamed symlink 'bzrplugins/hg' => 'bzrplugins/optional/hg'
=== target changed u'../sourcecode/bzr-hg' => u'../../sourcecode/bzr-hg'
=== renamed symlink 'bzrplugins/svn' => 'bzrplugins/optional/svn'
=== target changed u'../sourcecode/bzr-svn' => u'../../sourcecode/bzr-svn'
=== modified file 'lib/lp/codehosting/__init__.py'
--- lib/lp/codehosting/__init__.py 2011-12-11 21:02:54 +0000
+++ lib/lp/codehosting/__init__.py 2011-12-15 04:01:27 +0000
@@ -11,12 +11,14 @@
__all__ = [
'get_bzr_path',
'get_BZR_PLUGIN_PATH_for_subprocess',
+ 'load_optional_plugin',
]
import os
import bzrlib
+from bzrlib import hooks
from bzrlib.plugin import load_plugins
from canonical.config import config
@@ -59,6 +61,15 @@
load_plugins([_get_bzr_plugins_path()])
+def load_optional_plugin(plugin_name):
+ """Load the plugin named `plugin_name` from optionalbzrplugins/."""
+ from bzrlib import plugins
+ optional_plugin_dir = os.path.join(config.root, 'bzrplugins/optional')
+ if optional_plugin_dir not in plugins.__path__:
+ plugins.__path__.append(optional_plugin_dir)
+ __import__("bzrlib.plugins.%s" % plugin_name)
+
+
def load_bundled_plugin(plugin_name):
"""Load a plugin bundled with Bazaar."""
from bzrlib.plugin import get_core_plugin_path
=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py 2011-12-13 12:12:28 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py 2011-12-15 04:01:27 +0000
@@ -57,7 +57,7 @@
branch_id_alias,
compose_public_url,
)
-import lp.codehosting
+from lp.codehosting import load_optional_plugin
from lp.codehosting.codeimport.tarball import (
create_tarball,
extract_tarball,
@@ -1143,6 +1143,7 @@
def setUp(self):
super(TestGitImport, self).setUp()
+ load_optional_plugin('git')
self.setUpImport()
def tearDown(self):
@@ -1219,6 +1220,7 @@
def setUp(self):
super(TestMercurialImport, self).setUp()
+ load_optional_plugin('hg')
self.setUpImport()
def tearDown(self):
@@ -1300,6 +1302,7 @@
def setUp(self):
super(TestBzrSvnImport, self).setUp()
+ load_optional_plugin('svn')
self.setUpImport()
def makeImportWorker(self, source_details, opener_policy):
=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2011-12-13 12:12:28 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2011-12-15 04:01:27 +0000
@@ -49,6 +49,7 @@
from lp.code.interfaces.codeimportjob import ICodeImportJobSet
from lp.code.model.codeimport import CodeImport
from lp.code.model.codeimportjob import CodeImportJob
+from lp.codehosting import load_optional_plugin
from lp.codehosting.codeimport.tests.servers import (
BzrServer,
CVSServer,
@@ -741,6 +742,7 @@
def makeGitCodeImport(self):
"""Make a `CodeImport` that points to a real Git repository."""
+ load_optional_plugin('git')
self.git_server = GitServer(self.repo_path, use_server=False)
self.git_server.start_server()
self.addCleanup(self.git_server.stop_server)
@@ -753,6 +755,7 @@
def makeHgCodeImport(self):
"""Make a `CodeImport` that points to a real Mercurial repository."""
+ load_optional_plugin('hg')
self.hg_server = MercurialServer(self.repo_path, use_server=False)
self.hg_server.start_server()
self.addCleanup(self.hg_server.stop_server)
=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py 2011-12-11 21:02:54 +0000
+++ lib/lp/codehosting/codeimport/worker.py 2011-12-15 04:01:27 +0000
@@ -40,6 +40,7 @@
TooManyRedirections,
)
from bzrlib.transport import (
+ do_catching_redirections,
get_transport_from_path,
get_transport_from_url,
)
@@ -65,7 +66,6 @@
branch_id_alias,
compose_public_url,
)
-import lp.codehosting # for bzr plugins
from lp.codehosting.codeimport.foreigntree import (
CVSWorkingTree,
SubversionWorkingTree,
@@ -711,17 +711,49 @@
"""
return None
+ def _open_dir(self, url):
+ """Simple BzrDir.open clone that only uses self.probers.
+
+ :param url: URL to open
+ :return: ControlDir instance
+ """
+ def redirected(transport, e, redirection_notice):
+ self._opener_policy.checkOneURL(e.target)
+ redirected_transport = transport._redirected_to(
+ e.source, e.target)
+ if redirected_transport is None:
+ raise NotBranchError(e.source)
+ self._logger.info('%s is%s redirected to %s',
+ transport.base, e.permanently, redirected_transport.base)
+ return redirected_transport
+
+ def find_format(transport):
+ last_error = None
+ for prober_kls in self.probers:
+ prober = prober_kls()
+ try:
+ return transport, prober.probe_transport(transport)
+ except NotBranchError, e:
+ last_error = e
+ else:
+ raise last_error
+ transport = get_transport_from_url(url)
+ transport, format = do_catching_redirections(find_format, transport,
+ redirected)
+ return format.open(transport)
+
def _doImport(self):
self._logger.info("Starting job.")
saved_factory = bzrlib.ui.ui_factory
- opener = SafeBranchOpener(self._opener_policy, self.probers)
+ opener = SafeBranchOpener(self._opener_policy)
bzrlib.ui.ui_factory = LoggingUIFactory(logger=self._logger)
try:
self._logger.info(
"Getting exising bzr branch from central store.")
bazaar_branch = self.getBazaarBranch()
try:
- remote_branch = opener.open(self.source_details.url)
+ remote_branch = opener.open(
+ self.source_details.url, self._open_dir)
except TooManyRedirections:
self._logger.info("Too many redirections.")
return CodeImportWorkerExitCode.FAILURE_INVALID
=== modified file 'lib/lp/codehosting/puller/worker.py'
--- lib/lp/codehosting/puller/worker.py 2011-12-11 21:52:16 +0000
+++ lib/lp/codehosting/puller/worker.py 2011-12-15 04:01:27 +0000
@@ -214,7 +214,7 @@
:return: The destination branch.
"""
return self.opener.runWithTransformFallbackLocationHookInstalled(
- self.policy.createDestinationBranch, source_branch,
+ BzrDir.open, self.policy.createDestinationBranch, source_branch,
destination_url)
def openDestinationBranch(self, source_branch, destination_url):
=== modified file 'lib/lp/codehosting/safe_open.py'
--- lib/lp/codehosting/safe_open.py 2011-12-12 13:59:58 +0000
+++ lib/lp/codehosting/safe_open.py 2011-12-15 04:01:27 +0000
@@ -7,21 +7,10 @@
import threading
-from bzrlib import (
- trace,
- errors,
- urlutils,
- )
+from bzrlib import urlutils
from bzrlib.branch import Branch
-from bzrlib.bzrdir import (
- BzrProber,
- RemoteBzrProber,
- )
+from bzrlib.bzrdir import BzrDir
from lazr.uri import URI
-from bzrlib.transport import (
- do_catching_redirections,
- get_transport,
- )
__all__ = [
@@ -40,10 +29,6 @@
# TODO JelmerVernooij 2011-08-06: This module is generic enough to be
# in bzrlib, and may be of use to others. bug=850843
-# These are the default probers that SafeBranchOpener will try to use,
-# unless a different set was specified.
-
-DEFAULT_PROBERS = [BzrProber, RemoteBzrProber]
class BadUrl(Exception):
"""Tried to access a branch from a bad URL."""
@@ -199,19 +184,9 @@
_threading_data = threading.local()
- def __init__(self, policy, probers=None):
- """Create a new SafeBranchOpener.
-
- :param policy: The opener policy to use.
- :param probers: Optional list of probers to allow.
- Defaults to local and remote bzr probers.
- """
+ def __init__(self, policy):
self.policy = policy
self._seen_urls = set()
- if probers is None:
- self.probers = list(DEFAULT_PROBERS)
- else:
- self.probers = probers
@classmethod
def install_hook(cls):
@@ -230,7 +205,7 @@
cls.transformFallbackLocationHook,
'SafeBranchOpener.transformFallbackLocationHook')
- def checkAndFollowBranchReference(self, url):
+ def checkAndFollowBranchReference(self, url, open_dir=None):
"""Check URL (and possibly the referenced URL) for safety.
This method checks that `url` passes the policy's `checkOneURL`
@@ -239,6 +214,8 @@
also -- recursively, until a real branch is found.
:param url: URL to check
+ :param open_dir: Optional function to use for opening control
+ directories (defaults to BzrDir.open)
:raise BranchLoopError: If the branch references form a loop.
:raise BranchReferenceForbidden: If this opener forbids branch
references.
@@ -248,7 +225,7 @@
raise BranchLoopError()
self._seen_urls.add(url)
self.policy.checkOneURL(url)
- next_url = self.followReference(url)
+ next_url = self.followReference(url, open_dir=open_dir)
if next_url is None:
return url
url = next_url
@@ -269,75 +246,54 @@
return url
new_url, check = opener.policy.transformFallbackLocation(branch, url)
if check:
- return opener.checkAndFollowBranchReference(new_url)
+ return opener.checkAndFollowBranchReference(new_url,
+ getattr(cls._threading_data, "open_dir"))
else:
return new_url
def runWithTransformFallbackLocationHookInstalled(
- self, callable, *args, **kw):
+ self, open_dir, callable, *args, **kw):
assert (self.transformFallbackLocationHook in
Branch.hooks['transform_fallback_location'])
self._threading_data.opener = self
+ self._threading_data.open_dir = open_dir
try:
return callable(*args, **kw)
finally:
+ del self._threading_data.open_dir
del self._threading_data.opener
# We reset _seen_urls here to avoid multiple calls to open giving
# spurious loop exceptions.
self._seen_urls = set()
- def followReference(self, url):
+ def followReference(self, url, open_dir=None):
"""Get the branch-reference value at the specified url.
This exists as a separate method only to be overriden in unit tests.
"""
- bzrdir = self._open_dir(url)
+ if open_dir is None:
+ open_dir = BzrDir.open
+ bzrdir = open_dir(url)
return bzrdir.get_branch_reference()
- def _open_dir(self, url):
- """Simple BzrDir.open clone that only uses specific probers.
-
- :param url: URL to open
- :return: ControlDir instance
- """
- def redirected(transport, e, redirection_notice):
- self.policy.checkOneURL(e.target)
- redirected_transport = transport._redirected_to(
- e.source, e.target)
- if redirected_transport is None:
- raise errors.NotBranchError(e.source)
- trace.note('%s is%s redirected to %s',
- transport.base, e.permanently, redirected_transport.base)
- return redirected_transport
-
- def find_format(transport):
- last_error = errors.NotBranchError(transport.base)
- for prober_kls in self.probers:
- prober = prober_kls()
- try:
- return transport, prober.probe_transport(transport)
- except errors.NotBranchError, e:
- last_error = e
- else:
- raise last_error
- transport = get_transport(url)
- transport, format = do_catching_redirections(find_format, transport,
- redirected)
- return format.open(transport)
-
- def open(self, url):
+ def open(self, url, open_dir=None):
"""Open the Bazaar branch at url, first checking for safety.
What safety means is defined by a subclasses `followReference` and
`checkOneURL` methods.
+
+ :param open_dir: Optional function to use for opening control
+ directories (defaults to BzrDir.open)
"""
- url = self.checkAndFollowBranchReference(url)
+ url = self.checkAndFollowBranchReference(url, open_dir=open_dir)
+ if open_dir is None:
+ open_dir = BzrDir.open
def open_branch(url):
- dir = self._open_dir(url)
+ dir = open_dir(url)
return dir.open_branch()
return self.runWithTransformFallbackLocationHookInstalled(
- open_branch, url)
+ open_dir, open_branch, url)
def safe_open(allowed_scheme, url):
=== modified file 'lib/lp/codehosting/tests/test_acceptance.py'
--- lib/lp/codehosting/tests/test_acceptance.py 2011-12-11 21:16:42 +0000
+++ lib/lp/codehosting/tests/test_acceptance.py 2011-12-15 04:01:27 +0000
@@ -756,9 +756,6 @@
'RepositoryFormat5',
'RepositoryFormat6',
'RepositoryFormat7',
- 'HgRepositoryFormat',
- 'GitRepositoryFormat',
- 'SvnRepositoryFormat',
]
scenarios = all_repository_format_scenarios()
scenarios = [
=== modified file 'lib/lp/codehosting/tests/test_bzrutils.py'
--- lib/lp/codehosting/tests/test_bzrutils.py 2011-12-11 21:16:42 +0000
+++ lib/lp/codehosting/tests/test_bzrutils.py 2011-12-15 04:01:27 +0000
@@ -224,9 +224,7 @@
get_branch_stacked_on_url_tests = loader.loadTestsFromTestCase(
TestGetBranchStackedOnURL)
scenarios = [scenario for scenario in branch_scenarios()
- if scenario[0] not in (
- 'BranchReferenceFormat', 'GitBranchFormat',
- 'HgBranchFormat', 'SvnBranchFormat')]
+ if scenario[0] != 'BranchReferenceFormat']
multiply_tests(get_branch_stacked_on_url_tests, scenarios, result)
result.addTests(loader.loadTestsFromTestCase(TestIsBranchStackable))
=== modified file 'lib/lp/codehosting/tests/test_safe_open.py'
--- lib/lp/codehosting/tests/test_safe_open.py 2011-12-12 14:04:22 +0000
+++ lib/lp/codehosting/tests/test_safe_open.py 2011-12-15 04:01:27 +0000
@@ -14,10 +14,7 @@
from bzrlib.bzrdir import (
BzrDir,
BzrDirMetaFormat1,
- BzrProber,
)
-from bzrlib.controldir import ControlDirFormat
-from bzrlib.errors import NotBranchError
from bzrlib.repofmt.knitpack_repo import RepositoryFormatKnitPack1
from bzrlib.tests import TestCaseWithTransport
from bzrlib.transport import chroot
@@ -58,7 +55,7 @@
self._reference_values[references[i]] = references[i + 1]
self.follow_reference_calls = []
- def followReference(self, url):
+ def followReference(self, url, open_dir=None):
self.follow_reference_calls.append(url)
return self._reference_values[url]
@@ -71,15 +68,13 @@
def testCheckInitialURL(self):
# checkSource rejects all URLs that are not allowed.
opener = self.makeBranchOpener(None, [], set(['a']))
- self.assertRaises(
- BadUrl, opener.checkAndFollowBranchReference, 'a')
+ self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
def testNotReference(self):
# When branch references are forbidden, checkAndFollowBranchReference
# does not raise on non-references.
opener = self.makeBranchOpener(False, ['a', None])
- self.assertEquals(
- 'a', opener.checkAndFollowBranchReference('a'))
+ self.assertEquals('a', opener.checkAndFollowBranchReference('a'))
self.assertEquals(['a'], opener.follow_reference_calls)
def testBranchReferenceForbidden(self):
@@ -97,8 +92,7 @@
# is allowed and the source URL points to a branch reference to a
# permitted location.
opener = self.makeBranchOpener(True, ['a', 'b', None])
- self.assertEquals(
- 'b', opener.checkAndFollowBranchReference('a'))
+ self.assertEquals('b', opener.checkAndFollowBranchReference('a'))
self.assertEquals(['a', 'b'], opener.follow_reference_calls)
def testCheckReferencedURLs(self):
@@ -106,8 +100,7 @@
# to is safe.
opener = self.makeBranchOpener(
True, ['a', 'b', None], unsafe_urls=set('b'))
- self.assertRaises(
- BadUrl, opener.checkAndFollowBranchReference, 'a')
+ self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
self.assertEquals(['a'], opener.follow_reference_calls)
def testSelfReferencingBranch(self):
@@ -130,26 +123,15 @@
self.assertEquals(['a', 'b'], opener.follow_reference_calls)
-class TrackingProber(BzrProber):
- """Subclass of BzrProber which tracks URLs it has been asked to open."""
-
- seen_urls = []
-
- @classmethod
- def probe_transport(klass, transport):
- klass.seen_urls.append(transport.base)
- return BzrProber.probe_transport(transport)
-
-
class TestSafeBranchOpenerStacking(TestCaseWithTransport):
def setUp(self):
super(TestSafeBranchOpenerStacking, self).setUp()
SafeBranchOpener.install_hook()
- def makeBranchOpener(self, allowed_urls, probers=None):
+ def makeBranchOpener(self, allowed_urls):
policy = WhitelistPolicy(True, allowed_urls, True)
- return SafeBranchOpener(policy, probers)
+ return SafeBranchOpener(policy)
def makeBranch(self, path, branch_format, repository_format):
"""Make a Bazaar branch at 'path' with the given formats."""
@@ -159,39 +141,6 @@
repository_format.initialize(bzrdir)
return bzrdir.create_branch()
- def testProbers(self):
- # Only the specified probers should be used
- b = self.make_branch('branch')
- opener = self.makeBranchOpener([b.base], probers=[])
- self.assertRaises(NotBranchError, opener.open, b.base)
- opener = self.makeBranchOpener([b.base], probers=[BzrProber])
- self.assertEquals(b.base, opener.open(b.base).base)
-
- def testDefaultProbers(self):
- # If no probers are specified to the constructor
- # of SafeBranchOpener, then a safe set will be used,
- # rather than all probers registered in bzr.
- self.addCleanup(ControlDirFormat.unregister_prober, TrackingProber)
- ControlDirFormat.register_prober(TrackingProber)
- # Open a location without any branches, so that all probers are
- # tried.
- # First, check that the TrackingProber tracks correctly.
- TrackingProber.seen_urls = []
- opener = self.makeBranchOpener(["."], probers=[TrackingProber])
- self.assertRaises(NotBranchError, opener.open, ".")
- self.assertEquals(1, len(TrackingProber.seen_urls))
- TrackingProber.seen_urls = []
- # And make sure it's registered in such a way that BzrDir.open would
- # use it.
- self.assertRaises(NotBranchError, BzrDir.open, ".")
- self.assertEquals(1, len(TrackingProber.seen_urls))
- TrackingProber.seen_urls = []
- # Make sure that SafeBranchOpener doesn't use it if no
- # probers were specified
- opener = self.makeBranchOpener(["."])
- self.assertRaises(NotBranchError, opener.open, ".")
- self.assertEquals(0, len(TrackingProber.seen_urls))
-
def testAllowedURL(self):
# checkSource does not raise an exception for branches stacked on
# branches with allowed URLs.
@@ -286,25 +235,30 @@
a = self.make_branch('a', format='2a')
b = self.make_branch('b', format='2a')
b.set_stacked_on_url(a.base)
-
- TrackingProber.seen_urls = []
- opener = self.makeBranchOpener(
- [a.base, b.base], probers=[TrackingProber])
- opener.open(b.base)
- self.assertEquals(
- set(TrackingProber.seen_urls), set([b.base, a.base]))
+ seen_urls = set()
+
+ def open_dir(url):
+ seen_urls.add(url)
+ return BzrDir.open(url)
+
+ opener = self.makeBranchOpener([a.base, b.base])
+ opener.open(b.base, open_dir=open_dir)
+ self.assertEquals(seen_urls, set([b.base, a.base]))
def testCustomOpenerWithBranchReference(self):
# A custom function for opening a control dir can be specified.
a = self.make_branch('a', format='2a')
b_dir = self.make_bzrdir('b')
b = BranchReferenceFormat().initialize(b_dir, target_branch=a)
- TrackingProber.seen_urls = []
- opener = self.makeBranchOpener(
- [a.base, b.base], probers=[TrackingProber])
- opener.open(b.base)
- self.assertEquals(
- set(TrackingProber.seen_urls), set([b.base, a.base]))
+ seen_urls = set()
+
+ def open_dir(url):
+ seen_urls.add(url)
+ return BzrDir.open(url)
+
+ opener = self.makeBranchOpener([a.base, b.base])
+ opener.open(b.base, open_dir=open_dir)
+ self.assertEquals(seen_urls, set([b.base, a.base]))
class TestSafeOpen(TestCaseWithTransport):
=== modified file 'scripts/code-import-worker.py'
--- scripts/code-import-worker.py 2011-12-10 23:19:26 +0000
+++ scripts/code-import-worker.py 2011-12-15 04:01:27 +0000
@@ -24,6 +24,7 @@
from bzrlib.transport import get_transport
from canonical.config import config
+from lp.codehosting import load_optional_plugin
from lp.codehosting.codeimport.worker import (
BzrImportWorker, BzrSvnImportWorker, CSCVSImportWorker,
CodeImportBranchOpenPolicy, CodeImportSourceDetails, GitImportWorker,
@@ -68,12 +69,16 @@
force_bzr_to_use_urllib()
source_details = CodeImportSourceDetails.fromArguments(self.args)
if source_details.rcstype == 'git':
+ load_optional_plugin('git')
import_worker_cls = GitImportWorker
elif source_details.rcstype == 'bzr-svn':
+ load_optional_plugin('svn')
import_worker_cls = BzrSvnImportWorker
elif source_details.rcstype == 'hg':
+ load_optional_plugin('hg')
import_worker_cls = HgImportWorker
elif source_details.rcstype == 'bzr':
+ load_optional_plugin('loom')
import_worker_cls = BzrImportWorker
elif source_details.rcstype in ['cvs', 'svn']:
import_worker_cls = CSCVSImportWorker
