launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/foreigntree-use-subvertpy into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jelmer/launchpad/foreigntree-use-subvertpy/+merge/72733

Use subvertpy everywhere in the launchpad codebase, rather than the exotic of subvertpy, SVN_OO and pysvn. 

This also allows us to drop a subprocess call that was present to work around a pysvn bug.

We can't drop SVN_OO and pysvn just yet as they are dependencies of cscvs. 
-- 
https://code.launchpad.net/~jelmer/launchpad/foreigntree-use-subvertpy/+merge/72733
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/foreigntree-use-subvertpy into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/foreigntree.py'
--- lib/lp/codehosting/codeimport/foreigntree.py	2009-06-25 04:06:00 +0000
+++ lib/lp/codehosting/codeimport/foreigntree.py	2011-08-24 14:55:24 +0000
@@ -7,10 +7,11 @@
 __all__ = ['CVSWorkingTree', 'SubversionWorkingTree']
 
 import os
-import subprocess
 
 import CVS
-import pysvn
+import subvertpy
+import subvertpy.client
+import subvertpy.ra
 
 
 class CVSWorkingTree:
@@ -52,25 +53,23 @@
         self.remote_url = url
         self.local_path = path
 
+    def _get_client(self):
+        username_provider = subvertpy.ra.get_username_provider()
+        auth = subvertpy.ra.Auth([username_provider])
+        auth.set_parameter(subvertpy.AUTH_PARAM_DEFAULT_USERNAME, "lptest2")
+        return subvertpy.client.Client(auth=auth)
+
     def checkout(self):
-        # XXX: JonathanLange 2008-02-19: Can't do this with cscvs yet because
-        # its Repository class assumes that the repository lives on the local
-        # filesystem.
-        svn_client = pysvn.Client()
-        svn_client.checkout(
-            self.remote_url, self.local_path, ignore_externals=True)
+        client = self._get_client()
+        client.checkout(
+            self.remote_url, self.local_path, rev="HEAD",
+            ignore_externals=True)
 
     def commit(self):
-        client = pysvn.Client()
-        client.checkin(self.local_path, 'Log message', recurse=True)
+        client = self._get_client()
+        client.log_msg_func = lambda c: 'Log message'
+        client.commit([self.local_path], recurse=True)
 
     def update(self):
-        # XXX: David Allouche 2006-01-31 bug=82483: A bug in
-        # pysvn prevents us from ignoring svn:externals. We work
-        # around it by shelling out to svn. When cscvs no longer
-        # uses pysvn, we will use the cscvs API again.
-        arguments = ['svn', 'update', '--ignore-externals']
-        retcode = subprocess.call(
-            arguments, cwd=self.local_path, stdout=open('/dev/null', 'w'))
-        if retcode != 0:
-            raise RuntimeError("svn update failed with code %s" % retcode)
+        client = self._get_client()
+        client.update(self.local_path, "HEAD", True, True)

=== modified file 'lib/lp/codehosting/codeimport/tests/test_foreigntree.py'
--- lib/lp/codehosting/codeimport/tests/test_foreigntree.py	2011-08-12 11:37:08 +0000
+++ lib/lp/codehosting/codeimport/tests/test_foreigntree.py	2011-08-24 14:55:24 +0000
@@ -10,7 +10,10 @@
 
 from bzrlib.tests import TestCaseWithTransport
 import CVS
-import pysvn
+
+import subvertpy.client
+import subvertpy.ra
+import subvertpy.wc
 
 from canonical.testing.layers import BaseLayer
 from lp.codehosting.codeimport.foreigntree import (
@@ -33,11 +36,12 @@
         :param original_url: The URL of the Subversion branch.
         :param new_path: The path of the checkout.
         """
-        client = pysvn.Client()
-        [(path, local_info)] = client.info2(new_path, recurse=False)
-        [(path, remote_info)] = client.info2(original_url, recurse=False)
-        self.assertEqual(original_url, local_info['URL'])
-        self.assertEqual(remote_info['rev'].number, local_info['rev'].number)
+        working_copy = subvertpy.wc.WorkingCopy(None, new_path)
+        entry = working_copy.entry(new_path)
+
+        remote = subvertpy.ra.RemoteAccess(original_url)
+        self.assertEqual(original_url, entry.url)
+        self.assertEqual(entry.revision, remote.get_latest_revnum())
 
     def setUp(self):
         TestCaseWithTransport.setUp(self)
@@ -96,7 +100,7 @@
         tree2 = SubversionWorkingTree(self.svn_branch_url, 'tree2')
         tree2.checkout()
 
-        client = pysvn.Client()
+        client = subvertpy.client.Client()
         client.propset(
             'svn:externals', 'external http://foo.invalid/svn/something',
             tree.local_path)

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2011-08-19 03:58:35 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2011-08-24 14:55:24 +0000
@@ -36,7 +36,9 @@
     tree as CVSTree,
     )
 from dulwich.repo import Repo as GitRepo
-import pysvn
+import subvertpy
+import subvertpy.client
+import subvertpy.ra
 
 from canonical.config import config
 from canonical.testing.layers import BaseLayer
@@ -945,13 +947,16 @@
 
     def makeForeignCommit(self, source_details):
         """Change the foreign tree."""
-        client = pysvn.Client()
-        client.checkout(source_details.url, 'working_tree')
+        auth = subvertpy.ra.Auth([subvertpy.ra.get_username_provider()])
+        auth.set_parameter(subvertpy.AUTH_PARAM_DEFAULT_USERNAME, "lptest2")
+        client = subvertpy.client.Client(auth=auth)
+        client.checkout(source_details.url, 'working_tree', "HEAD")
         file = open('working_tree/newfile', 'w')
         file.write('No real content\n')
         file.close()
         client.add('working_tree/newfile')
-        client.checkin('working_tree', 'Add a file', recurse=True)
+        client.log_msg_func = lambda c: 'Add a file'
+        client.commit(['working_tree'], recurse=True)
         self.foreign_commit_count += 1
         shutil.rmtree('working_tree')