implement atomic_move method, move files only with lock

src/ahriman/core/database/sqlite.py

@@ -25,8 +25,16 @@ from typing import Self
 
 from ahriman.core.configuration import Configuration
 from ahriman.core.database.migrations import Migrations
-from ahriman.core.database.operations import AuthOperations, BuildOperations, ChangesOperations, \
-    DependenciesOperations, EventOperations, LogsOperations, PackageOperations, PatchOperations
+from ahriman.core.database.operations import (
+    AuthOperations,
+    BuildOperations,
+    ChangesOperations,
+    DependenciesOperations,
+    EventOperations,
+    LogsOperations,
+    PackageOperations,
+    PatchOperations,
+)
 from ahriman.models.repository_id import RepositoryId
 
 

src/ahriman/core/repository/executor.py

@@ -17,8 +17,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-import shutil  # shutil.move is used here to ensure cross fs file movement
-
 from collections.abc import Iterable
 from pathlib import Path
 from tempfile import TemporaryDirectory
@@ -27,7 +25,7 @@ from ahriman.core.build_tools.package_archive import PackageArchive
 from ahriman.core.build_tools.task import Task
 from ahriman.core.repository.cleaner import Cleaner
 from ahriman.core.repository.package_info import PackageInfo
-from ahriman.core.utils import safe_filename
+from ahriman.core.utils import atomic_move, safe_filename
 from ahriman.models.changes import Changes
 from ahriman.models.event import EventType
 from ahriman.models.package import Package
@@ -54,7 +52,7 @@ class Executor(PackageInfo, Cleaner):
             return  # suppress type checking, it never can be none actually
 
         if (safe := safe_filename(description.filename)) != description.filename:
-            (self.paths.packages / description.filename).rename(self.paths.packages / safe)
+            atomic_move(self.paths.packages / description.filename, self.paths.packages / safe)
             description.filename = safe
 
     def _package_build(self, package: Package, path: Path, packager: str | None,
@@ -81,7 +79,7 @@ class Executor(PackageInfo, Cleaner):
         package.with_packages(built, self.pacman)
         for src in built:
             dst = self.paths.packages / src.name
-            shutil.move(src, dst)
+            atomic_move(src, dst)
 
         return commit_sha
 
@@ -130,7 +128,7 @@ class Executor(PackageInfo, Cleaner):
 
         for src in files:
             dst = self.paths.archive_for(package_base) / src.name
-            src.rename(dst)  # move package to archive directory
+            atomic_move(src, dst)  # move package to archive directory
             if not (symlink := self.paths.repository / dst.name).exists():
                 symlink.symlink_to(dst.relative_to(symlink.parent, walk_up=True))  # create link to archive
 

src/ahriman/core/utils.py

@@ -19,12 +19,14 @@
 #
 # pylint: disable=too-many-lines
 import datetime
+import fcntl
 import io
 import itertools
 import logging
 import os
 import re
 import selectors
+import shutil
 import subprocess
 
 from collections.abc import Callable, Iterable, Iterator, Mapping
@@ -38,6 +40,7 @@ from ahriman.core.exceptions import CalledProcessError, OptionError, UnsafeRunEr
 
 
 __all__ = [
+    "atomic_move",
     "check_output",
     "check_user",
     "dataclass_view",
@@ -65,6 +68,34 @@ __all__ = [
 T = TypeVar("T")
 
 
+def atomic_move(src: Path, dst: Path) -> None:
+    """
+    move file from ``source`` location to ``destination``. This method uses lock and :func:`shutil.move` to ensure that
+    file will be copied (if not rename) atomically. This method blocks execution until lock is available
+
+    Args:
+        src(Path): path to the source file
+        dst(Path): path to the destination
+
+    Examples:
+        This method is a drop-in replacement for :func:`shutil.move` (except it doesn't allow to override copy method)
+        which first locking destination file. To use it simply call method with arguments::
+
+            >>> atomic_move(src, dst)
+    """
+    lock_path = dst.with_name(f".{dst.name}")
+    try:
+        with lock_path.open("ab") as lock_file:
+            fd = lock_file.fileno()
+            try:
+                fcntl.flock(fd, fcntl.LOCK_EX)  # lock file and wait lock is until available
+                shutil.move(src, dst)
+            finally:
+                fcntl.flock(fd, fcntl.LOCK_UN)  # unlock file first
+    finally:
+        lock_path.unlink(missing_ok=True)  # remove lock file at the end
+
+
 # pylint: disable=too-many-locals
 def check_output(*args: str, exception: Exception | Callable[[int, list[str], str, str], Exception] | None = None,
                  cwd: Path | None = None, input_data: str | None = None,

src/ahriman/web/middlewares/exception_handler.py

@@ -21,8 +21,18 @@ import aiohttp_jinja2
 import logging
 
 from aiohttp.typedefs import Middleware
-from aiohttp.web import HTTPClientError, HTTPException, HTTPMethodNotAllowed, HTTPNoContent, HTTPServerError, \
-    HTTPUnauthorized, Request, StreamResponse, json_response, middleware
+from aiohttp.web import (
+    HTTPClientError,
+    HTTPException,
+    HTTPMethodNotAllowed,
+    HTTPNoContent,
+    HTTPServerError,
+    HTTPUnauthorized,
+    Request,
+    StreamResponse,
+    json_response,
+    middleware,
+)
 
 from ahriman.web.middlewares import HandlerType
 

src/ahriman/web/views/v1/packages/package.py

@@ -25,8 +25,12 @@ from ahriman.models.build_status import BuildStatusEnum
 from ahriman.models.package import Package
 from ahriman.models.user_access import UserAccess
 from ahriman.web.apispec.decorators import apidocs
-from ahriman.web.schemas import PackageNameSchema, PackageStatusSchema, PackageStatusSimplifiedSchema, \
-    RepositoryIdSchema
+from ahriman.web.schemas import (
+    PackageNameSchema,
+    PackageStatusSchema,
+    PackageStatusSimplifiedSchema,
+    RepositoryIdSchema,
+)
 from ahriman.web.views.base import BaseView
 from ahriman.web.views.status_view_guard import StatusViewGuard
 

src/ahriman/web/views/v1/service/upload.py

@@ -26,6 +26,7 @@ from tempfile import NamedTemporaryFile
 from typing import ClassVar
 
 from ahriman.core.configuration import Configuration
+from ahriman.core.utils import atomic_move
 from ahriman.models.repository_paths import RepositoryPaths
 from ahriman.models.user_access import UserAccess
 from ahriman.web.apispec.decorators import apidocs
@@ -152,10 +153,8 @@ class UploadView(BaseView):
 
             files.append(await self.save_file(part, target, max_body_size=max_body_size))
 
-        # and now we can rename files, which is relatively fast operation
-        # it is probably good way to call lock here, however
         for filename, current_location in files:
             target_location = current_location.parent / filename
-            current_location.rename(target_location)
+            atomic_move(current_location, target_location)
 
         raise HTTPCreated