diff --git a/src/ahriman/application/handlers/tree_migrate.py b/src/ahriman/application/handlers/tree_migrate.py index 7d9a0f38..5cce21b8 100644 --- a/src/ahriman/application/handlers/tree_migrate.py +++ b/src/ahriman/application/handlers/tree_migrate.py @@ -71,7 +71,7 @@ class TreeMigrate(Handler): @staticmethod def fix_symlinks(paths: RepositoryPaths) -> None: """ - fix packages archives symlinks + fix package archive symlinks Args: paths(RepositoryPaths): new repository paths diff --git a/src/ahriman/core/alpm/repo.py b/src/ahriman/core/alpm/repo.py index 5dcde02b..6659a1a3 100644 --- a/src/ahriman/core/alpm/repo.py +++ b/src/ahriman/core/alpm/repo.py @@ -88,16 +88,14 @@ class Repo(LazyLogging): check_output("repo-add", *self.sign_args, str(self.repo_path), cwd=self.root, logger=self.logger, user=self.uid) - def remove(self, package_name: str | None, filename: Path) -> None: + def remove(self, package_name: str, filename: Path) -> None: """ remove package from repository Args: - package_name(str | None): package name to remove. If none set, it will be guessed from filename + package_name(str): package name to remove filename(Path): package filename to remove """ - package_name = package_name or filename.name.rsplit("-", maxsplit=3)[0] - # remove package and signature (if any) from filesystem for full_path in self.root.glob(f"**/{filename.name}*"): full_path.unlink() diff --git a/src/ahriman/core/archive/archive_tree.py b/src/ahriman/core/archive/archive_tree.py index 23b1a121..976126f6 100644 --- a/src/ahriman/core/archive/archive_tree.py +++ b/src/ahriman/core/archive/archive_tree.py @@ -75,6 +75,7 @@ class ArchiveTree(LazyLogging): def _repo(self, root: Path) -> Repo: """ constructs :class:`ahriman.core.alpm.repo.Repo` object for given path + Args: root(Path): root of the repository @@ -141,7 +142,10 @@ class ArchiveTree(LazyLogging): if path.exists(): continue # filter out not broken symlinks - self._repo(root).remove(None, path) + # here we don't have access to original archive, so we have to guess name based on archive name + # normally it should be fine to do so + package_name = path.name.rsplit("-", maxsplit=3)[0] + self._repo(root).remove(package_name, path) def tree_create(self) -> None: """ diff --git a/src/ahriman/core/database/migrations/m016_archive.py b/src/ahriman/core/database/migrations/m016_archive.py index 056e972b..4a2e46a4 100644 --- a/src/ahriman/core/database/migrations/m016_archive.py +++ b/src/ahriman/core/database/migrations/m016_archive.py @@ -26,7 +26,7 @@ from ahriman.application.handlers.handler import Handler from ahriman.core.alpm.pacman import Pacman from ahriman.core.configuration import Configuration from ahriman.core.sign.gpg import GPG -from ahriman.core.utils import package_like, symlink_relative +from ahriman.core.utils import atomic_move, package_like, symlink_relative from ahriman.models.package import Package from ahriman.models.pacman_synchronization import PacmanSynchronization from ahriman.models.repository_paths import RepositoryPaths @@ -81,6 +81,6 @@ def move_packages(repository_paths: RepositoryPaths, pacman: Pacman) -> None: for source in artifacts: # move package to the archive directory target = repository_paths.archive_for(package.base) / source.name - source.rename(target) + atomic_move(source, target) # create symlink to the archive symlink_relative(source, target) diff --git a/src/ahriman/core/repository/executor.py b/src/ahriman/core/repository/executor.py index 4ce723e4..f62dc33e 100644 --- a/src/ahriman/core/repository/executor.py +++ b/src/ahriman/core/repository/executor.py @@ -143,7 +143,7 @@ class Executor(PackageInfo, Cleaner): remove package base from repository Args: - package_base(str): package base name: + package_base(str): package base name """ try: with self.in_event(package_base, EventType.PackageRemoved): diff --git a/tests/ahriman/core/alpm/test_repo.py b/tests/ahriman/core/alpm/test_repo.py index f0221262..8e8ee61e 100644 --- a/tests/ahriman/core/alpm/test_repo.py +++ b/tests/ahriman/core/alpm/test_repo.py @@ -71,20 +71,6 @@ def test_repo_remove(repo: Repo, package_ahriman: Package, mocker: MockerFixture assert package_ahriman.base in check_output_mock.call_args[0] -def test_repo_remove_guess_package(repo: Repo, package_ahriman: Package, mocker: MockerFixture) -> None: - """ - must call repo-remove on package removal if no package name set - """ - filepath = package_ahriman.packages[package_ahriman.base].filepath - mocker.patch("pathlib.Path.glob", return_value=[]) - check_output_mock = mocker.patch("ahriman.core.alpm.repo.check_output") - - repo.remove(None, filepath) - check_output_mock.assert_called_once() # it will be checked later - assert check_output_mock.call_args[0][0] == "repo-remove" - assert package_ahriman.base in check_output_mock.call_args[0] - - def test_repo_remove_fail_no_file(repo: Repo, mocker: MockerFixture) -> None: """ must fail removal on missing file diff --git a/tests/ahriman/core/archive/test_archive_tree.py b/tests/ahriman/core/archive/test_archive_tree.py index 399ab59d..9b689d0f 100644 --- a/tests/ahriman/core/archive/test_archive_tree.py +++ b/tests/ahriman/core/archive/test_archive_tree.py @@ -80,7 +80,7 @@ def test_symlinks_fix(archive_tree: ArchiveTree, mocker: MockerFixture) -> None: _original_exists = Path.exists def exists_mock(path: Path) -> bool: - if path.name == "symlink": + if path.name.startswith("symlink"): return True return _original_exists(path) @@ -88,13 +88,18 @@ def test_symlinks_fix(archive_tree: ArchiveTree, mocker: MockerFixture) -> None: mocker.patch("pathlib.Path.exists", autospec=True, side_effect=exists_mock) walk_mock = mocker.patch("ahriman.core.archive.archive_tree.walk", return_value=[ archive_tree.repository_for() / filename - for filename in ("symlink", "broken_symlink", "file") + for filename in ( + "symlink-1.0.0-1-x86_64.pkg.tar.zst", + "broken_symlink-1.0.0-1-x86_64.pkg.tar.zst", + "file-1.0.0-1-x86_64.pkg.tar.zst", + ) ]) remove_mock = mocker.patch("ahriman.core.alpm.repo.Repo.remove") archive_tree.symlinks_fix() walk_mock.assert_called_once_with(archive_tree.paths.archive / "repos") - remove_mock.assert_called_once_with(None, archive_tree.repository_for() / "broken_symlink") + remove_mock.assert_called_once_with( + "broken_symlink", archive_tree.repository_for() / "broken_symlink-1.0.0-1-x86_64.pkg.tar.zst") def test_symlinks_fix_foreign_repository(archive_tree: ArchiveTree, mocker: MockerFixture) -> None: @@ -104,7 +109,7 @@ def test_symlinks_fix_foreign_repository(archive_tree: ArchiveTree, mocker: Mock _original_exists = Path.exists def exists_mock(path: Path) -> bool: - if path.name == "symlink": + if path.name.startswith("symlink"): return True return _original_exists(path) @@ -112,7 +117,11 @@ def test_symlinks_fix_foreign_repository(archive_tree: ArchiveTree, mocker: Mock mocker.patch("pathlib.Path.exists", autospec=True, side_effect=exists_mock) mocker.patch("ahriman.core.archive.archive_tree.walk", return_value=[ archive_tree.repository_for().with_name("i686") / filename - for filename in ("symlink", "broken_symlink", "file") + for filename in ( + "symlink-1.0.0-1-x86_64.pkg.tar.zst", + "broken_symlink-1.0.0-1-x86_64.pkg.tar.zst", + "file-1.0.0-1-x86_64.pkg.tar.zst", + ) ]) remove_mock = mocker.patch("ahriman.core.alpm.repo.Repo.remove") diff --git a/tests/ahriman/core/database/migrations/test_m016_archive.py b/tests/ahriman/core/database/migrations/test_m016_archive.py index 8bb091b3..2c6745c5 100644 --- a/tests/ahriman/core/database/migrations/test_m016_archive.py +++ b/tests/ahriman/core/database/migrations/test_m016_archive.py @@ -54,18 +54,17 @@ def test_move_packages(repository_paths: RepositoryPaths, pacman: Pacman, packag mocker.patch("pathlib.Path.is_file", autospec=True, side_effect=is_file) mocker.patch("pathlib.Path.exists", return_value=True) archive_mock = mocker.patch("ahriman.models.package.Package.from_archive", return_value=package_ahriman) - rename_mock = mocker.patch("pathlib.Path.rename") + move_mock = mocker.patch("ahriman.core.database.migrations.m016_archive.atomic_move") symlink_mock = mocker.patch("pathlib.Path.symlink_to") move_packages(repository_paths, pacman) archive_mock.assert_has_calls([ - MockCall(repository_paths.repository / "file.pkg.tar.xz", pacman), - MockCall(repository_paths.repository / "file2.pkg.tar.xz", pacman), + MockCall(repository_paths.repository / filename, pacman) + for filename in ("file.pkg.tar.xz", "file2.pkg.tar.xz") ]) - rename_mock.assert_has_calls([ - MockCall(repository_paths.archive_for(package_ahriman.base) / "file.pkg.tar.xz"), - MockCall(repository_paths.archive_for(package_ahriman.base) / "file.pkg.tar.xz.sig"), - MockCall(repository_paths.archive_for(package_ahriman.base) / "file2.pkg.tar.xz"), + move_mock.assert_has_calls([ + MockCall(repository_paths.repository / filename, repository_paths.archive_for(package_ahriman.base) / filename) + for filename in ("file.pkg.tar.xz", "file.pkg.tar.xz.sig", "file2.pkg.tar.xz") ]) symlink_mock.assert_has_calls([ MockCall( @@ -73,20 +72,7 @@ def test_move_packages(repository_paths: RepositoryPaths, pacman: Pacman, packag ".." / ".." / repository_paths.archive_for(package_ahriman.base).relative_to(repository_paths.root) / - "file.pkg.tar.xz" - ), - MockCall( - Path("..") / - ".." / - ".." / - repository_paths.archive_for(package_ahriman.base).relative_to(repository_paths.root) / - "file.pkg.tar.xz.sig" - ), - MockCall( - Path("..") / - ".." / - ".." / - repository_paths.archive_for(package_ahriman.base).relative_to(repository_paths.root) / - "file2.pkg.tar.xz" - ), + filename + ) + for filename in ("file.pkg.tar.xz", "file.pkg.tar.xz.sig", "file2.pkg.tar.xz") ])