launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/proper-codeimport-servers into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jelmer/launchpad/proper-codeimport-servers/+merge/70617

Use actual Git and Mercurial servers during the code import tests, like we already do for Subversion.

This allows tightening the security on the URL field for code imports in the database a bit; at the moment we allow file:// there for the benefit of the test suite.

This change is also necessary for some upcoming changes to the code imports, to make it use the safe branch opener code currently used for code mirrors. Because the code imports can be tested with actual code import URLs, it is no longer necessary to have a custom branch policy that allows local paths.
-- 
https://code.launchpad.net/~jelmer/launchpad/proper-codeimport-servers/+merge/70617
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/proper-codeimport-servers into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/servers.py'
--- lib/lp/codehosting/codeimport/tests/servers.py	2011-06-02 10:48:54 +0000
+++ lib/lp/codehosting/codeimport/tests/servers.py	2011-08-05 19:11:30 +0000
@@ -20,6 +20,7 @@
 import subprocess
 import tempfile
 import time
+import threading
 
 from bzrlib.tests.treeshape import build_tree_contents
 from bzrlib.transport import Server
@@ -31,6 +32,17 @@
 import dulwich.index
 from dulwich.objects import Blob
 from dulwich.repo import Repo as GitRepo
+from dulwich.server import (
+    DictBackend,
+    TCPGitServer,
+    )
+from mercurial.ui import (
+    ui as hg_ui,
+    )
+from mercurial.hgweb import (
+    hgweb,
+    server as hgweb_server,
+    )
 import subvertpy.ra
 import svn_oo
 
@@ -204,42 +216,127 @@
         self._repository = self.createRepository(self._repository_path)
 
 
+class TCPGitServerThread(threading.Thread):
+    """TCP Git server that runs in a separate thread."""
+
+    def __init__(self, backend, address, port=None):
+        super(TCPGitServerThread, self).__init__()
+        self.setName("TCP Git server on %s:%s" % (address, port))
+        self.server = TCPGitServer(backend, address, port)
+
+    def run(self):
+        self.server.serve_forever()
+
+    def get_address(self):
+        return self.server.server_address
+
+    def stop(self):
+        self.server.shutdown()
+
+
 class GitServer(Server):
 
-    def __init__(self, repo_url):
+    def __init__(self, repository_path, use_server=False):
         super(GitServer, self).__init__()
-        self.repo_url = repo_url
+        self.repository_path = repository_path
+        self._use_server = use_server
+
+    def get_url(self):
+        """Return a URL to the Git repository."""
+        if self._use_server:
+            return 'git://%s:%d/' % self._server.get_address()
+        else:
+            return local_path_to_url(self.repository_path)
+
+    def createRepository(self, path):
+        GitRepo.init(path)
+
+    def start_server(self):
+        super(GitServer, self).start_server()
+        self.createRepository(self.repository_path)
+        if self._use_server:
+            repo = GitRepo(self.repository_path)
+            self._server = TCPGitServerThread(
+                DictBackend({"/": repo}), "localhost", 0)
+            self._server.start()
+
+    def stop_server(self):
+        super(GitServer, self).stop_server()
+        if self._use_server:
+            self._server.stop()
 
     def makeRepo(self, tree_contents):
-        wd = os.getcwd()
-        try:
-            os.chdir(self.repo_url)
-            repo = GitRepo.init(".")
-            blobs = [
-                (Blob.from_string(contents), filename) for (filename, contents)
-                in tree_contents]
-            repo.object_store.add_objects(blobs)
-            root_id = dulwich.index.commit_tree(repo.object_store, [
-                (filename, b.id, stat.S_IFREG | 0644)
-                for (b, filename) in blobs])
-            repo.do_commit(committer='Joe Foo <joe@xxxxxxx>',
-                message=u'<The commit message>', tree=root_id)
-        finally:
-            os.chdir(wd)
+        repo = GitRepo(self.repository_path)
+        blobs = [
+            (Blob.from_string(contents), filename) for (filename, contents)
+            in tree_contents]
+        repo.object_store.add_objects(blobs)
+        root_id = dulwich.index.commit_tree(repo.object_store, [
+            (filename, b.id, stat.S_IFREG | 0644)
+            for (b, filename) in blobs])
+        repo.do_commit(committer='Joe Foo <joe@xxxxxxx>',
+            message=u'<The commit message>', tree=root_id)
+
+
+class MercurialServerThread(threading.Thread):
+
+    def __init__(self, path, address, port=0):
+        super(MercurialServerThread, self).__init__()
+        self.ui = hg_ui()
+        self.ui.setconfig("web", "address", address)
+        self.ui.setconfig("web", "port", 0)
+        self.app = hgweb(path, baseui=self.ui)
+        self.httpd = hgweb_server.create_server(self.ui, self.app)
+        # By default the Mercurial server output goes to stdout
+        self.httpd.errorlog = StringIO()
+        self.httpd.accesslog = StringIO()
+
+    def get_address(self):
+        return (self.httpd.addr, self.httpd.port)
+
+    def run(self):
+        self.httpd.serve_forever()
+
+    def stop(self):
+        self.httpd.shutdown()
 
 
 class MercurialServer(Server):
 
-    def __init__(self, repo_url):
+    def __init__(self, repository_path, use_server=False):
         super(MercurialServer, self).__init__()
-        self.repo_url = repo_url
+        self.repository_path = repository_path
+        self._use_server = use_server
+
+    def get_url(self):
+        if self._use_server:
+            return "http://%s:%d/"; % self._hgserver.get_address()
+        else:
+            return local_path_to_url(self.repository_path)
+
+    def start_server(self):
+        super(MercurialServer, self).start_server()
+        self.createRepository(self.repository_path)
+        if self._use_server:
+            self._hgserver = MercurialServerThread(self.repository_path, "localhost")
+            self._hgserver.start()
+
+    def stop_server(self):
+        super(MercurialServer, self).stop_server()
+        if self._use_server:
+            self._hgserver.stop()
+
+    def createRepository(self, path):
+        from mercurial.ui import ui
+        from mercurial.localrepo import localrepository
+        localrepository(ui(), self.repository_path, create=1)
 
     def makeRepo(self, tree_contents):
         from mercurial.ui import ui
         from mercurial.localrepo import localrepository
-        repo = localrepository(ui(), self.repo_url, create=1)
+        repo = localrepository(ui(), self.repository_path)
         for filename, contents in tree_contents:
-            f = open(os.path.join(self.repo_url, filename), 'w')
+            f = open(os.path.join(self.repository_path, filename), 'w')
             try:
                 f.write(contents)
             finally:

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2011-07-20 17:20:03 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2011-08-05 19:11:30 +0000
@@ -1071,7 +1071,7 @@
 
     def makeForeignCommit(self, source_details):
         """Change the foreign tree, generating exactly one commit."""
-        repo = GitRepo(source_details.url)
+        repo = GitRepo(local_path_from_url(source_details.url))
         repo.do_commit(message=self.factory.getUniqueString(),
             committer="Joe Random Hacker <joe@xxxxxxxxxxx>")
         self.foreign_commit_count += 1
@@ -1088,8 +1088,7 @@
         self.foreign_commit_count = 1
 
         return self.factory.makeCodeImportSourceDetails(
-            rcstype='git', url=repository_path)
-
+            rcstype='git', url=git_server.get_url())
 
 
 class TestMercurialImport(WorkerTest, TestActualImportMixin,
@@ -1124,7 +1123,7 @@
         """Change the foreign tree, generating exactly one commit."""
         from mercurial.ui import ui
         from mercurial.localrepo import localrepository
-        repo = localrepository(ui(), source_details.url)
+        repo = localrepository(ui(), local_path_from_url(source_details.url))
         repo.commit(text="hello world!", user="Jane Random Hacker", force=1)
         self.foreign_commit_count += 1
 
@@ -1140,7 +1139,7 @@
         self.foreign_commit_count = 1
 
         return self.factory.makeCodeImportSourceDetails(
-            rcstype='hg', url=repository_path)
+            rcstype='hg', url=hg_server.get_url())
 
 
 class TestBzrSvnImport(WorkerTest, SubversionImportHelpers,

=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-06-16 23:43:04 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-08-05 19:11:30 +0000
@@ -661,26 +661,28 @@
     def makeGitCodeImport(self):
         """Make a `CodeImport` that points to a real Git repository."""
         load_optional_plugin('git')
-        self.git_server = GitServer(self.repo_path)
+        self.git_server = GitServer(self.repo_path, use_server=True)
         self.git_server.start_server()
         self.addCleanup(self.git_server.stop_server)
 
         self.git_server.makeRepo([('README', 'contents')])
         self.foreign_commit_count = 1
 
-        return self.factory.makeCodeImport(git_repo_url=self.repo_path)
+        return self.factory.makeCodeImport(
+            git_repo_url=self.git_server.get_url())
 
     def makeHgCodeImport(self):
         """Make a `CodeImport` that points to a real Mercurial repository."""
         load_optional_plugin('hg')
-        self.hg_server = MercurialServer(self.repo_path)
+        self.hg_server = MercurialServer(self.repo_path, use_server=True)
         self.hg_server.start_server()
         self.addCleanup(self.hg_server.stop_server)
 
         self.hg_server.makeRepo([('README', 'contents')])
         self.foreign_commit_count = 1
 
-        return self.factory.makeCodeImport(hg_repo_url=self.repo_path)
+        return self.factory.makeCodeImport(
+            hg_repo_url=self.hg_server.get_url())
 
     def getStartedJobForImport(self, code_import):
         """Get a started `CodeImportJob` for `code_import`.