bldinstallercommon: improve dir strip functionality, add unit testsv6.5.0-beta3-packaging

- Switch os.path style paths -> pathlib.Path
- Switch move function: move_tree -> shutil.move
- Add missing unit tests
- Improve logging, error messages
- Add docstring

Change-Id: Ie8c0abe3af0163127dbfe2a016b0471bf9705b0a
Reviewed-by: Antti Kokko <antti.kokko@qt.io>

4 files changed, 83 insertions, 33 deletions

diff --git a/packaging-tools/bld_sdktool.py b/packaging-tools/bld_sdktool.py
index 55ca0f5ad..f3361418f 100644
--- a/packaging-tools/bld_sdktool.py
+++ b/packaging-tools/bld_sdktool.py
@@ -37,7 +37,7 @@ from pathlib import Path
 from typing import List, Optional
 
 from bld_utils import is_linux, is_windows
-from bldinstallercommon import extract_file, remove_one_tree_level, retrieve_url
+from bldinstallercommon import extract_file, retrieve_url, strip_dirs
 from runner import run_cmd
 
 BuildParams = namedtuple('BuildParams',
@@ -88,7 +88,7 @@ def get_and_extract_qt_src(url: str, temp: str, path: str) -> None:
     retrieve_url(url, file_path)
     Path(path).mkdir(parents=True, exist_ok=True)
     extract_file(file_path, path)
-    remove_one_tree_level(path)
+    strip_dirs(Path(path))
 
 
 def configure_qt(params: BuildParams, src: str, build: str) -> None:
diff --git a/packaging-tools/bldinstallercommon.py b/packaging-tools/bldinstallercommon.py
index f039d484a..a70d0cbb7 100644
--- a/packaging-tools/bldinstallercommon.py
+++ b/packaging-tools/bldinstallercommon.py
@@ -31,6 +31,7 @@
 
 import ctypes
 import errno
+import itertools
 import os
 import re
 import shutil
@@ -232,19 +233,41 @@ def copy_tree(source_dir: str, dest_dir: str) -> None:
             shutil.copy(full_file_name, dest_dir)
 
 
-def remove_one_tree_level(directory: str) -> None:
-    dircontents = os.listdir(directory)
-    items = len(dircontents)
-    if items == 1:
-        dir_name = dircontents[0]
-        full_dir_name = os.path.join(directory, dir_name)
-        # avoid directory name collision by first moving to temporary dir
-        with TemporaryDirectory() as tempdir_base:
-            tempdir = tempdir_base.path / 'a'  # dummy name
-            shutil.move(full_dir_name, str(tempdir))
-            move_tree(str(tempdir), directory)
-    else:
-        raise IOError(f'Cannot remove one level of directory structure of "{dir}", it has {items} subdirectories')
+def strip_dirs(directory: Path, iterations: int = 1) -> None:
+    """
+    Remove unnecessary tree structure from a given directory path
+
+    Args:
+        directory: A file system path to the folder to strip from
+        iterations: A number of middle directories to remove (0=do nothing)
+
+    Raises:
+        IOError: When the directory contains more than one subdirectory
+        IOError: When the directory contains a non-directory
+        IOError: When the directory contains no items
+    """
+    if not iterations:
+        # if no directories to strip, do nothing
+        return
+    log.info("Remove %s level(s) of tree structure: %s", iterations, directory)
+    dir_name = directory
+    while iterations:
+        sub_items = list(itertools.islice(dir_name.iterdir(), 2))
+        if len(sub_items) != 1:
+            raise IOError(f"Expected one item in directory: {dir_name}")
+        dir_name = sub_items[0]
+        if not dir_name.is_dir():
+            raise IOError(f"Subitem is not a directory: {dir_name}, expected one subdirectory")
+        iterations -= 1
+    with TemporaryDirectory() as temp_dir:
+        # first move to temp dir to avoid name collision
+        shutil.move(str(dir_name.resolve()), temp_dir.path)
+        # remove empty dirs
+        for item in directory.iterdir():
+            shutil.rmtree(item)
+        # move subitems to target dir
+        for item in temp_dir.path.joinpath(dir_name.name).iterdir():
+            shutil.move(str(item), str(directory))
 
 
 ###############################
diff --git a/packaging-tools/create_installer.py b/packaging-tools/create_installer.py
index 9a7985c35..ab7c020af 100644
--- a/packaging-tools/create_installer.py
+++ b/packaging-tools/create_installer.py
@@ -55,11 +55,11 @@ from bldinstallercommon import (
     locate_executable,
     locate_path,
     locate_paths,
-    remove_one_tree_level,
     remove_tree,
     replace_in_files,
     retrieve_url,
     safe_config_key_fetch,
+    strip_dirs,
     uri_exists,
 )
 from installer_utils import PackagingError
@@ -395,22 +395,6 @@ def get_component_sha1(sdk_comp: IfwSdkComponent, sha1_file_dest: Path) -> None:
     read_component_sha(sdk_comp=sdk_comp, sha1_file_path=sha1_file_dest)
 
 
-def strip_dirs(iterations: int, install_dir: Path) -> None:
-    """
-    Strip out unnecessary folder structure based on the configuration
-
-    Args:
-        iterations: Specifies how many tree levels to remove (0=do nothing)
-        install_dir: A file system path to the folder to strip from
-
-    Raises:
-        IOError: When there are too many tree levels to be removed based on the structure
-    """
-    while iterations:
-        remove_one_tree_level(str(install_dir))  # TODO: use shutil.move here
-        iterations -= 1
-
-
 def delete_docs(directory: Path) -> None:
     """
     Delete doc directory from directory if exists
@@ -555,7 +539,7 @@ def patch_component_data(
     """
     if archive.archive_action:
         exec_action_script(archive.archive_action, install_dir)
-    strip_dirs(archive.package_strip_dirs, install_dir)
+    strip_dirs(install_dir, archive.package_strip_dirs)
     finalize_items(task, archive.package_finalize_items, install_dir)
     process_debug_files_and_libs(
         archive.archive_name,
diff --git a/packaging-tools/tests/test_bldinstallercommon.py b/packaging-tools/tests/test_bldinstallercommon.py
index 88c068b81..08bf6085d 100644
--- a/packaging-tools/tests/test_bldinstallercommon.py
+++ b/packaging-tools/tests/test_bldinstallercommon.py
@@ -45,6 +45,7 @@ from bldinstallercommon import (
     locate_paths,
     replace_in_files,
     search_for_files,
+    strip_dirs,
 )
 from installer_utils import PackagingError
 
@@ -230,6 +231,48 @@ class TestCommon(unittest.TestCase):
         result = calculate_relpath(path1, path2)
         self.assertEqual(result, expected)
 
+    def test_strip_dirs(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            temp_dir.path.joinpath("remove_dir", "sub_dir").mkdir(parents=True, exist_ok=True)
+            strip_dirs(temp_dir.path)
+            self.assertTrue(temp_dir.path.joinpath("sub_dir").exists())
+            self.assertFalse(temp_dir.path.joinpath("remove_dir").exists())
+
+    def test_strip_dirs_multiple_iterations(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            temp_dir.path.joinpath("remove_dir", "remove_dir2", "sub_dir").mkdir(
+                parents=True, exist_ok=True
+            )
+            strip_dirs(temp_dir.path, iterations=2)
+            self.assertTrue(temp_dir.path.joinpath("sub_dir").exists())
+            self.assertFalse(temp_dir.path.joinpath("remove_dir").exists())
+
+    def test_strip_dirs_no_iterations(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            temp_dir.path.joinpath("no_remove_dir", "sub_dir").mkdir(parents=True, exist_ok=True)
+            strip_dirs(temp_dir.path, iterations=0)
+            self.assertTrue(temp_dir.path.joinpath("no_remove_dir", "sub_dir").exists())
+
+    def test_strip_dirs_identical_name(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            temp_dir.path.joinpath("dir_name", "dir_name").mkdir(parents=True, exist_ok=True)
+            strip_dirs(temp_dir.path)
+            self.assertTrue(temp_dir.path.joinpath("dir_name").exists())
+            self.assertFalse(temp_dir.path.joinpath("dir_name", "dir_name").exists())
+
+    def test_strip_dirs_invalid_subdir_count(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            with self.assertRaises(IOError):
+                temp_dir.path.joinpath("remove_dir").mkdir(parents=True, exist_ok=True)
+                temp_dir.path.joinpath("another_dir").mkdir(parents=True, exist_ok=True)
+                strip_dirs(temp_dir.path)
+
+    def test_strip_dirs_not_a_dir(self) -> None:
+        with TemporaryDirectory() as temp_dir:
+            with self.assertRaises(IOError):
+                temp_dir.path.joinpath("remove_dir").touch(exist_ok=True)
+                strip_dirs(temp_dir.path)
+
 
 if __name__ == "__main__":
     unittest.main()