Standardise on move (files, directories) (#2884)

* Add new util function (and tests) called `move`.
* Change references to sh.mv to use move (as it is faster and cross-platform).
* Change conditional "mv -t" to a for loop.

Author: Julian-O

pythonforandroid/bootstrap.py

@@ -11,7 +11,7 @@
 from pythonforandroid.logger import (shprint, info, logger, debug)
 from pythonforandroid.util import (
     current_directory, ensure_dir, temp_directory, BuildInterruptingException,
-    rmdir)
+    rmdir, move)
 from pythonforandroid.recipe import Recipe
 
 
@@ -395,8 +395,8 @@ def fry_eggs(self, sitepackages):
             if isdir(rd) and d.endswith('.egg'):
                 info('  ' + d)
                 files = [join(rd, f) for f in listdir(rd) if f != 'EGG-INFO']
-                if files:
-                    shprint(sh.mv, '-t', sitepackages, *files)
+                for f in files:
+                    move(f, sitepackages)
                 rmdir(d)
 
 

pythonforandroid/recipe.py

@@ -18,7 +18,7 @@
     from urllib.parse import urlparse
 from pythonforandroid.logger import (logger, info, warning, debug, shprint, info_main)
 from pythonforandroid.util import (current_directory, ensure_dir,
-                                   BuildInterruptingException, rmdir)
+                                   BuildInterruptingException, rmdir, move)
 from pythonforandroid.util import load_source as import_recipe
 
 
@@ -457,14 +457,14 @@ def unpack(self, arch):
                         fileh = zipfile.ZipFile(extraction_filename, 'r')
                         root_directory = fileh.filelist[0].filename.split('/')[0]
                         if root_directory != basename(directory_name):
-                            shprint(sh.mv, root_directory, directory_name)
+                            move(root_directory, directory_name)
                     elif extraction_filename.endswith(
                             ('.tar.gz', '.tgz', '.tar.bz2', '.tbz2', '.tar.xz', '.txz')):
                         sh.tar('xf', extraction_filename)
                         root_directory = sh.tar('tf', extraction_filename).stdout.decode(
                                 'utf-8').split('\n')[0].split('/')[0]
                         if root_directory != basename(directory_name):
-                            shprint(sh.mv, root_directory, directory_name)
+                            move(root_directory, directory_name)
                     else:
                         raise Exception(
                             'Could not extract {} download, it must be .zip, '
@@ -1166,7 +1166,7 @@ def reduce_object_file_names(self, dirn):
             parts = file_basename.split('.')
             if len(parts) <= 2:
                 continue
-            shprint(sh.mv, filen, join(file_dirname, parts[0] + '.so'))
+            move(filen, join(file_dirname, parts[0] + '.so'))
 
 
 def algsum(alg, filen):

pythonforandroid/util.py

@@ -4,7 +4,7 @@
 from os.path import exists, join
 from os import getcwd, chdir, makedirs, walk
 from platform import uname
-from shutil import rmtree
+import shutil
 from tempfile import mkdtemp
 
 from pythonforandroid.logger import (logger, Err_Fore, error, info)
@@ -39,7 +39,7 @@ def temp_directory():
                               temp_dir, Err_Fore.RESET)))
         yield temp_dir
     finally:
-        rmtree(temp_dir)
+        shutil.rmtree(temp_dir)
         logger.debug(''.join((Err_Fore.CYAN, ' - temp directory deleted ',
                               temp_dir, Err_Fore.RESET)))
 
@@ -110,11 +110,16 @@ def rmdir(dn, ignore_errors=False):
     if not exists(dn):
         return
     LOGGER.debug("Remove directory and subdirectory {}".format(dn))
-    rmtree(dn, ignore_errors)
+    shutil.rmtree(dn, ignore_errors)
 
 
 def ensure_dir(dn):
     if exists(dn):
         return
     LOGGER.debug("Create directory {0}".format(dn))
     makedirs(dn)
+
+
+def move(source, destination):
+    LOGGER.debug("Moving {} to {}".format(source, destination))
+    shutil.move(source, destination)

tests/test_bootstrap.py

@@ -562,10 +562,10 @@ def test_bootstrap_strip(
 
     @mock.patch("pythonforandroid.bootstrap.listdir")
     @mock.patch("pythonforandroid.bootstrap.rmdir")
-    @mock.patch("pythonforandroid.bootstrap.sh.mv")
+    @mock.patch("pythonforandroid.bootstrap.move")
     @mock.patch("pythonforandroid.bootstrap.isdir")
     def test_bootstrap_fry_eggs(
-        self, mock_isdir, mock_sh_mv, mock_rmdir, mock_listdir
+        self, mock_isdir, mock_move, mock_rmdir, mock_listdir
     ):
         mock_listdir.return_value = [
             "jnius",
@@ -597,7 +597,7 @@ def test_bootstrap_fry_eggs(
         )
         # check that the other mocks we made are actually called
         mock_isdir.assert_called()
-        mock_sh_mv.assert_called()
+        mock_move.assert_called()
 
 
 class TestBootstrapSdl2(GenericBootstrapTest, unittest.TestCase):

tests/test_util.py

@@ -1,4 +1,6 @@
 import os
+from pathlib import Path
+from tempfile import TemporaryDirectory
 import types
 import unittest
 from unittest import mock
@@ -21,20 +23,21 @@ def test_ensure_dir(self, mock_makedirs):
         util.ensure_dir("fake_directory")
         mock_makedirs.assert_called_once_with("fake_directory")
 
-    @mock.patch("pythonforandroid.util.rmtree")
+    @mock.patch("shutil.rmtree")
     @mock.patch("pythonforandroid.util.mkdtemp")
-    def test_temp_directory(self, mock_mkdtemp, mock_rmtree):
+    def test_temp_directory(self, mock_mkdtemp, mock_shutil_rmtree):
+
         """
         Basic test for method :meth:`~pythonforandroid.util.temp_directory`. We
         perform this test by `mocking` the command `mkdtemp` and
-        `rmdir` and we make sure that those functions are called in the
+        `shutil.rmtree` and we make sure that those functions are called in the
         proper place.
         """
         mock_mkdtemp.return_value = "/temp/any_directory"
         with util.temp_directory():
             mock_mkdtemp.assert_called_once()
-            mock_rmtree.assert_not_called()
-        mock_rmtree.assert_called_once_with("/temp/any_directory")
+            mock_shutil_rmtree.assert_not_called()
+        mock_shutil_rmtree.assert_called_once_with("/temp/any_directory")
 
     @mock.patch("pythonforandroid.util.chdir")
     def test_current_directory(self, moch_chdir):
@@ -136,3 +139,42 @@ def test_util_exceptions(self):
         )
         with self.assertRaises(SystemExit):
             util.handle_build_exception(exc)
+
+    def test_move(self):
+        with mock.patch(
+                "pythonforandroid.util.LOGGER"
+        ) as m_logger, TemporaryDirectory() as base_dir:
+            new_path = Path(base_dir) / "new"
+
+            # Set up source
+            old_path = Path(base_dir) / "old"
+            with open(old_path, "w") as outfile:
+                outfile.write("Temporary content")
+
+            # Non existent source
+            with self.assertRaises(FileNotFoundError):
+                util.move(new_path, new_path)
+            m_logger.debug.assert_called()
+            m_logger.error.assert_not_called()
+            m_logger.reset_mock()
+            assert old_path.exists()
+            assert not new_path.exists()
+
+            # Successful move
+            util.move(old_path, new_path)
+            assert not old_path.exists()
+            assert new_path.exists()
+            m_logger.debug.assert_called()
+            m_logger.error.assert_not_called()
+            m_logger.reset_mock()
+
+            # Move over existing:
+            existing_path = Path(base_dir) / "existing"
+            existing_path.touch()
+
+            util.move(new_path, existing_path)
+            with open(existing_path, "r") as infile:
+                assert infile.read() == "Temporary content"
+            m_logger.debug.assert_called()
+            m_logger.error.assert_not_called()
+            m_logger.reset_mock()