launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:remove-py2-rmtree-workarounds into turnip:master.

Commit message:
Remove workarounds for Python 2's shutil.rmtree

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/405050
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:remove-py2-rmtree-workarounds into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 06d3e1c..f56962d 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -466,10 +466,6 @@ def set_default_branch(repo_path, target):
 
 def delete_repo(repo_path):
     """Permanently delete a git repository from repo store."""
-    # XXX cjwatson 2018-11-20: Python 2's shutil.rmtree crashes if given a
-    # Unicode path to a directory that contains non-UTF-8 files.
-    if not isinstance(repo_path, bytes):
-        repo_path = repo_path.encode('UTF-8')
     shutil.rmtree(repo_path)
 
 
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index cbc77f8..dbeb2dc 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -103,20 +103,16 @@ class FunctionalTestMixin(WithScenarios):
 
     def startHookRPC(self):
         self.hookrpc_handler = HookRPCHandler(self.virtinfo_url, 15)
-        # XXX cjwatson 2018-11-20: Use bytes so that shutil.rmtree doesn't
-        # get confused on Python 2.
-        dir = tempfile.mkdtemp(prefix=b'turnip-test-hook-')
+        dir = tempfile.mkdtemp(prefix='turnip-test-hook-')
         self.addCleanup(shutil.rmtree, dir, ignore_errors=True)
 
-        self.hookrpc_sock_path = os.path.join(dir, b'hookrpc_sock')
+        self.hookrpc_sock_path = os.path.join(dir, 'hookrpc_sock')
         self.hookrpc_listener = reactor.listenUNIX(
             self.hookrpc_sock_path, HookRPCServerFactory(self.hookrpc_handler))
         self.addCleanup(self.hookrpc_listener.stopListening)
 
     def startPackBackend(self):
-        # XXX cjwatson 2018-11-20: Use bytes so that shutil.rmtree doesn't
-        # get confused on Python 2.
-        self.root = tempfile.mkdtemp(prefix=b'turnip-test-root-')
+        self.root = tempfile.mkdtemp(prefix='turnip-test-root-')
         self.addCleanup(shutil.rmtree, self.root, ignore_errors=True)
         self.statsd_client = MockStatsd()
         self.backend_listener = reactor.listenTCP(
@@ -670,10 +666,10 @@ class FrontendFunctionalTestMixin(FunctionalTestMixin):
         self.startVirtInfo()
         self.startHookRPC()
         self.startPackBackend()
-        self.internal_name = six.ensure_binary(hashlib.sha256(
-            b'/test').hexdigest())
+        self.internal_name = hashlib.sha256(b'/test').hexdigest()
         yield self.assertCommandSuccess(
-            (b'git', b'init', b'--bare', self.internal_name), path=self.root)
+            (b'git', b'init', b'--bare', self.internal_name.encode()),
+            path=self.root)
 
         self.virt_listener = reactor.listenTCP(
             0,
@@ -724,8 +720,7 @@ class FrontendFunctionalTestMixin(FunctionalTestMixin):
         yield self.assertCommandSuccess(
             (b'git', b'push', b'origin', b'master'), path=clone1)
         self.assertEqual(
-            six.ensure_text(self.internal_name),
-            self.virtinfo.push_notifications[0][0])
+            self.internal_name, self.virtinfo.push_notifications[0][0])
 
     @defer.inlineCallbacks
     def test_unicode_fault(self):
@@ -842,8 +837,7 @@ class TestSmartHTTPFrontendFunctional(FrontendFunctionalTestMixin, TestCase):
         head_target = yield self.get_symbolic_ref(repo, b'HEAD')
         self.assertEqual(b'refs/heads/new-head', head_target)
         self.assertEqual(
-            six.ensure_text(self.internal_name),
-            self.virtinfo.push_notifications[0][0])
+            self.internal_name, self.virtinfo.push_notifications[0][0])
         self.assertIsNotNone(
             self.virtinfo.push_notifications[0][1].get('pack_count'))
         self.assertIsNotNone(
@@ -935,7 +929,7 @@ class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional):
             (b'git', b'push', b'origin', b'master'), path=clone)
         self.assertThat(self.virtinfo.ref_permissions_checks, MatchesListwise([
             MatchesListwise([
-                Equals(six.ensure_text(self.internal_name)),
+                Equals(self.internal_name),
                 Equals([b'refs/heads/master']),
                 MatchesDict({
                     'can-authenticate': Is(True),