diff --git a/craft_parts/executor/organize.py b/craft_parts/executor/organize.py index 4b876d4bb..337e385b7 100644 --- a/craft_parts/executor/organize.py +++ b/craft_parts/executor/organize.py @@ -27,22 +27,26 @@ import shutil from glob import iglob from pathlib import Path -from typing import Dict +from typing import Dict, Mapping, Optional from craft_parts import errors from craft_parts.utils import file_utils, path_utils def organize_files( - *, part_name: str, mapping: Dict[str, str], base_dir: Path, overwrite: bool + *, + part_name: str, + file_map: Dict[str, str], + install_dir_map: Mapping[Optional[str], Path], + overwrite: bool, ) -> None: """Rearrange files for part staging. If partitions are enabled, source filepaths must be in the default partition. :param part_name: The name of the part to organize files for. - :param mapping: A mapping of source filepaths to destination filepaths. - :param base_dir: Directory containing files to organize. + :param file_map: A mapping of source filepaths to destination filepaths. + :param install_dir_map: A mapping of partition names to their install directories. :param overwrite: Whether existing files should be overwritten. This is only used in build updates, when a part may organize over files it previously organized. @@ -52,7 +56,7 @@ def organize_files( :raises FileOrganizeError: If partitions are enabled and the source file is not from the default partition. """ - for key in sorted(mapping, key=lambda x: ["*" in x, x]): + for key in sorted(file_map, key=lambda x: ["*" in x, x]): src_partition, src_inner_path = path_utils.get_partition_and_path(key) if src_partition and src_partition != "default": @@ -64,27 +68,21 @@ def organize_files( ), ) - src = os.path.join(base_dir, src_inner_path) + src = os.path.join(install_dir_map[src_partition], src_inner_path) # Remove the leading slash so the path actually joins # Also trailing slash is significant, be careful if using pathlib! dst_partition, dst_inner_path = path_utils.get_partition_and_path( - mapping[key].lstrip("/") + file_map[key].lstrip("/") ) + dst = os.path.join(install_dir_map[dst_partition], dst_inner_path) + + # prefix the partition to the log-friendly version of the destination if dst_partition and dst_partition != "default": - dst = os.path.join( - "partitions", - dst_partition, - "parts", - part_name, - "install", - dst_inner_path, - ) - partition_path = dst + dst_string = f"({dst_partition})/{dst_inner_path}" else: - dst = os.path.join(base_dir, dst_inner_path) - partition_path = str(dst_inner_path) + dst_string = str(dst_inner_path) sources = iglob(src, recursive=True) @@ -107,9 +105,9 @@ def organize_files( raise errors.FileOrganizeError( part_name=part_name, message=( - f"multiple files to be organized into " - f"{partition_path!r}. If this is " - f"supposed to be a directory, end it with a slash." + "multiple files to be organized into " + f"{dst_string!r}. If this is " + "supposed to be a directory, end it with a slash." ), ) else: @@ -117,8 +115,8 @@ def organize_files( part_name=part_name, message=( f"trying to organize file {key!r} to " - f"{mapping[key]!r}, but " - f"{partition_path!r} already exists" + f"{file_map[key]!r}, but " + f"{dst_string!r} already exists" ), ) diff --git a/craft_parts/executor/part_handler.py b/craft_parts/executor/part_handler.py index 939c461a1..5bb953d68 100644 --- a/craft_parts/executor/part_handler.py +++ b/craft_parts/executor/part_handler.py @@ -879,8 +879,8 @@ def _organize(self, *, overwrite: bool = False) -> None: mapping = self._part.spec.organize_files organize_files( part_name=self._part.name, - mapping=mapping, - base_dir=self._part.part_install_dir, + file_map=mapping, + install_dir_map=self._part.part_install_dirs, overwrite=overwrite, ) diff --git a/tests/unit/executor/test_organize.py b/tests/unit/executor/test_organize.py index 6006d146b..6fc2f8bd4 100644 --- a/tests/unit/executor/test_organize.py +++ b/tests/unit/executor/test_organize.py @@ -142,6 +142,7 @@ def test_organize(new_dir, data): expected_message=data.get("expected_message"), expected_overwrite=data.get("expected_overwrite"), overwrite=False, + install_dirs={None: Path(new_dir / "install")}, ) # Verify that it can be organized again by overwriting @@ -154,6 +155,7 @@ def test_organize(new_dir, data): expected_message=data.get("expected_message"), expected_overwrite=data.get("expected_overwrite"), overwrite=True, + install_dirs={None: Path(new_dir / "install")}, ) @@ -167,15 +169,16 @@ def organize_and_assert( expected_message, expected_overwrite, overwrite, + install_dirs, ): - base_dir = Path(tmp_path / "install") - base_dir.mkdir(parents=True, exist_ok=True) + install_dir = Path(tmp_path / "install") + install_dir.mkdir(parents=True, exist_ok=True) for directory in setup_dirs: - (base_dir / directory).mkdir(exist_ok=True) + (install_dir / directory).mkdir(exist_ok=True) for file_entry in setup_files: - (base_dir / file_entry).touch() + (install_dir / file_entry).touch() if overwrite and expected_overwrite is not None: expected = expected_overwrite @@ -185,8 +188,8 @@ def organize_and_assert( with pytest.raises(expected) as raised: organize_files( part_name="part-name", - mapping=organize_map, - base_dir=base_dir, + file_map=organize_map, + install_dir_map=install_dirs, overwrite=overwrite, ) assert re.match(expected_message, str(raised.value)) is not None @@ -194,13 +197,13 @@ def organize_and_assert( else: organize_files( part_name="part-name", - mapping=organize_map, - base_dir=base_dir, + file_map=organize_map, + install_dir_map=install_dirs, overwrite=overwrite, ) expected = cast(List[Tuple[List[str], str]], expected) for expect in expected: - dir_path = (base_dir / expect[1]).as_posix() + dir_path = (install_dir / expect[1]).as_posix() dir_contents = os.listdir(dir_path) dir_contents.sort() assert dir_contents == expect[0] diff --git a/tests/unit/features/partitions/executor/test_organize.py b/tests/unit/features/partitions/executor/test_organize.py index 903c69e56..e13457b18 100644 --- a/tests/unit/features/partitions/executor/test_organize.py +++ b/tests/unit/features/partitions/executor/test_organize.py @@ -121,7 +121,7 @@ "expected": errors.FileOrganizeError, "expected_message": ( r".*trying to organize file '\(default\)/foo' to '\(our/special-part\)/bar', " - r"but 'partitions/our/special-part/parts/part-name/install/bar' already exists." + r"but '\(our/special-part\)/bar' already exists." ), "expected_overwrite": [ (["bar"], "../partitions/our/special-part/parts/part-name/install") @@ -149,8 +149,7 @@ "organize_map": {"*.conf": "(our/special-part)/dir"}, "expected": errors.FileOrganizeError, "expected_message": ( - r".*multiple files to be organized into " - r"'partitions/our/special-part/parts/part-name/install/dir'.*" + r".*multiple files to be organized into '\(our/special-part\)/dir'.*" ), }, # *_for_directories @@ -202,6 +201,14 @@ ], ) def test_organize(new_dir, data): + install_dirs = { + "default": new_dir / "install", + "mypart": new_dir / "partitions/mypart/parts/part-name/install", + "yourpart": new_dir / "partitions/yourpart/parts/part-name/install", + "our/special-part": new_dir + / "partitions/our/special-part/parts/part-name/install", + } + organize_and_assert( tmp_path=new_dir, setup_dirs=data.get("setup_dirs", []), @@ -211,6 +218,7 @@ def test_organize(new_dir, data): expected_message=data.get("expected_message"), expected_overwrite=data.get("expected_overwrite"), overwrite=False, + install_dirs=install_dirs, ) # Verify that it can be organized again by overwriting @@ -223,4 +231,5 @@ def test_organize(new_dir, data): expected_message=data.get("expected_message"), expected_overwrite=data.get("expected_overwrite"), overwrite=True, + install_dirs=install_dirs, )