From 1a57b806c77c2a97328039402d90357dbc4f0c7c Mon Sep 17 00:00:00 2001 From: Patrik Teivonen Date: Wed, 11 Jan 2023 12:05:20 +0200 Subject: bldinstallercommon: improve dir strip functionality, add unit tests - 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 --- packaging-tools/bld_sdktool.py | 4 +- packaging-tools/bldinstallercommon.py | 49 +++++++++++++++++------- packaging-tools/create_installer.py | 20 +--------- packaging-tools/tests/test_bldinstallercommon.py | 43 +++++++++++++++++++++ 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() -- cgit v1.2.3