clean empty directories

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

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()

src/ahriman/core/archive/archive_tree.py

@@ -19,6 +19,7 @@
 #
 import datetime
 
+from collections.abc import Iterator
 from pathlib import Path
 
 from ahriman.core.alpm.repo import Repo
@@ -75,6 +76,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
 
@@ -83,6 +85,21 @@ class ArchiveTree(LazyLogging):
         """
         return Repo(self.repository_id.name, self.paths, self.sign_args, root)
 
+    def directories_fix(self, paths: set[Path]) -> None:
+        """
+        remove empty repository directories recursively
+
+        Args:
+            paths(set[Path]): repositories to check
+        """
+        root = self.paths.archive / "repos"
+        for repository in paths:
+            parents = [repository] + list(repository.parents[:-1])
+            for parent in parents:
+                path = root / parent
+                if not list(path.iterdir()):
+                    path.rmdir()
+
     def repository_for(self, date: datetime.date | None = None) -> Path:
         """
         get full path to repository at the specified date
@@ -126,9 +143,12 @@ class ArchiveTree(LazyLogging):
                 if self._package_symlinks_create(single, root, archive):
                     repo.add(root / single.filename)
 
-    def symlinks_fix(self) -> None:
+    def symlinks_fix(self) -> Iterator[Path]:
         """
         remove broken symlinks across repositories for all dates
+
+        Yields:
+            Path: path of the sub-repository with removed symlinks
         """
         for path in walk(self.paths.archive / "repos"):
             root = path.parent
@@ -141,7 +161,11 @@ 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)
+            yield path.parent.relative_to(self.paths.archive / "repos")
 
     def tree_create(self) -> None:
         """

src/ahriman/core/archive/archive_trigger.py

@@ -66,4 +66,5 @@ class ArchiveTrigger(Trigger):
         """
         trigger action which will be called before the stop of the application
         """
-        self.tree.symlinks_fix()
+        repositories = set(self.tree.symlinks_fix())
+        self.tree.directories_fix(repositories)

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)

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):

src/ahriman/models/repository_paths.py

@@ -292,6 +292,7 @@ class RepositoryPaths(LazyLogging):
         """
         for directory in (
                 self.cache_for(package_base),
+                self.archive_for(package_base),
         ):
             shutil.rmtree(directory, ignore_errors=True)
 

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

tests/ahriman/core/archive/test_archive_tree.py

@@ -29,6 +29,25 @@ def test_repository_for(archive_tree: ArchiveTree) -> None:
     assert set(map("{:02d}".format, utcnow().timetuple()[:3])).issubset(path.parts)
 
 
+def test_directories_fix(archive_tree: ArchiveTree, mocker: MockerFixture) -> None:
+    """
+    must remove empty directories recursively
+    """
+    root = archive_tree.paths.archive / "repos"
+    (root / "a" / "b").mkdir(parents=True, exist_ok=True)
+    (root / "a" / "b" / "file").touch()
+    (root / "a" / "b" / "c" / "d").mkdir(parents=True, exist_ok=True)
+
+    _original_rmdir = Path.rmdir
+    rmdir_mock = mocker.patch("pathlib.Path.rmdir", autospec=True, side_effect=_original_rmdir)
+
+    archive_tree.directories_fix({Path("a") / "b" / "c" / "d"})
+    rmdir_mock.assert_has_calls([
+        MockCall(root / "a" / "b" / "c" / "d"),
+        MockCall(root / "a" / "b" / "c"),
+    ])
+
+
 def test_symlinks_create(archive_tree: ArchiveTree, package_ahriman: Package, package_python_schedule: Package,
                          mocker: MockerFixture) -> None:
     """
@@ -80,7 +99,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 +107,20 @@ 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()
+    assert list(archive_tree.symlinks_fix()) == [
+        archive_tree.repository_for().relative_to(archive_tree.paths.archive / "repos"),
+    ]
     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 +130,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,11 +138,15 @@ 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")
 
-    archive_tree.symlinks_fix()
+    assert list(archive_tree.symlinks_fix()) == []
     remove_mock.assert_not_called()
 
 

tests/ahriman/core/archive/test_archive_trigger.py

@@ -1,3 +1,4 @@
+from pathlib import Path
 from pytest_mock import MockerFixture
 
 from ahriman.core.archive import ArchiveTrigger
@@ -27,6 +28,10 @@ def test_on_stop(archive_trigger: ArchiveTrigger, mocker: MockerFixture) -> None
     """
     must fix broken symlinks on stop
     """
-    symlinks_mock = mocker.patch("ahriman.core.archive.archive_tree.ArchiveTree.symlinks_fix")
+    local = Path("local")
+    symlinks_mock = mocker.patch("ahriman.core.archive.archive_tree.ArchiveTree.symlinks_fix", return_value=[local])
+    directories_mock = mocker.patch("ahriman.core.archive.archive_tree.ArchiveTree.directories_fix")
+
     archive_trigger.on_stop()
     symlinks_mock.assert_called_once_with()
+    directories_mock.assert_called_once_with({local})

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")
     ])

tests/ahriman/models/test_repository_paths.py

@@ -271,6 +271,7 @@ def test_tree_clear(repository_paths: RepositoryPaths, package_ahriman: Package,
     """
     paths = {
         repository_paths.cache_for(package_ahriman.base),
+        repository_paths.archive_for(package_ahriman.base),
     }
     rmtree_mock = mocker.patch("shutil.rmtree")