[gentoo-catalyst] [PATCH 1/2] catalyst: Remove clear_dir's never used clear_nondir parameter

[gentoo-catalyst] [PATCH 1/2] catalyst: Remove clear_dir's never used clear_nondir parameter

[Matt Turner]

Signed-off-by: Matt Turner <mattst88@gentoo.org>
---
 catalyst/fileops.py | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/catalyst/fileops.py b/catalyst/fileops.py
index 59525420..4252285e 100644
--- a/catalyst/fileops.py
+++ b/catalyst/fileops.py
@@ -46,8 +46,7 @@ def ensure_dirs(path, gid=-1, uid=-1, mode=0o755, minimal=True,
     return succeeded
 
 
-def clear_dir(target, mode=0o755, remove=False,
-              clear_nondir=True):
+def clear_dir(target, mode=0o755, remove=False):
     '''Universal directory clearing function
 
     @target: string, path to be cleared or removed
@@ -74,12 +73,8 @@ def clear_dir(target, mode=0o755, remove=False,
             log.error('clear_dir failed', exc_info=True)
             return False
     elif os.path.exists(target):
-        if clear_nondir:
-            log.debug("Clearing (unlinking) non-directory: %s", target)
-            os.unlink(target)
-        else:
-            log.info('clear_dir failed: %s: is not a directory', target)
-            return False
+        log.debug("Clearing (unlinking) non-directory: %s", target)
+        os.unlink(target)
     else:
         log.debug("Conditions not met to clear: %s", target)
         log.debug("                      isdir: %s", os.path.isdir(target))
-- 
2.31.1

[gentoo-catalyst] [PATCH 2/2] catalyst: Replace snakeoil's locks with fasteners

[Matt Turner]

To no great surprise, the existing locking was broken. For example,
clear_chroot() releases the lock. It is called by unpack(), which is
part of prepare_sequence. The result is that the whole build could be
done without holding the lock.

Just lock around run(). It's not apparent that finer-grained locking
does anything for us.

Signed-off-by: Matt Turner <mattst88@gentoo.org>
---
 catalyst/base/clearbase.py   |  2 --
 catalyst/base/stagebase.py   | 10 +++----
 catalyst/lock.py             | 58 ------------------------------------
 catalyst/targets/snapshot.py |  6 ++--
 4 files changed, 7 insertions(+), 69 deletions(-)
 delete mode 100644 catalyst/lock.py

diff --git a/catalyst/base/clearbase.py b/catalyst/base/clearbase.py
index dcf6c523..6218330e 100644
--- a/catalyst/base/clearbase.py
+++ b/catalyst/base/clearbase.py
@@ -27,12 +27,10 @@ class ClearBase():
             self.resume.clear_all(remove=True)
 
     def clear_chroot(self):
-        self.chroot_lock.unlock()
         log.notice('Clearing the chroot path ...')
         clear_dir(self.settings["chroot_path"], mode=0o755)
 
     def remove_chroot(self):
-        self.chroot_lock.unlock()
         log.notice('Removing the chroot path ...')
         clear_dir(self.settings["chroot_path"], mode=0o755, remove=True)
 
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 448d6265..10cffd4c 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -8,6 +8,7 @@ import sys
 
 from pathlib import Path
 
+import fasteners
 import libmount
 import toml
 
@@ -25,7 +26,6 @@ from catalyst.support import (CatalystError, file_locate, normpath,
 from catalyst.base.targetbase import TargetBase
 from catalyst.base.clearbase import ClearBase
 from catalyst.base.genbase import GenBase
-from catalyst.lock import LockDir, LockInUse
 from catalyst.fileops import ensure_dirs, clear_dir, clear_path
 from catalyst.base.resume import AutoResume
 
@@ -36,9 +36,6 @@ def run_sequence(sequence):
         sys.stdout.flush()
         try:
             func()
-        except LockInUse:
-            log.error('Unable to aquire the lock...')
-            return False
         except Exception:
             log.error('Exception running action sequence %s',
                       func.__name__, exc_info=True)
@@ -478,7 +475,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
         """
         self.settings["chroot_path"] = normpath(self.settings["storedir"] +
                                                 "/tmp/" + self.settings["target_subpath"].rstrip('/'))
-        self.chroot_lock = LockDir(self.settings["chroot_path"])
 
     def set_autoresume_path(self):
         self.settings["autoresume_path"] = normpath(pjoin(
@@ -1366,8 +1362,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
             pass
 
     def run(self):
-        self.chroot_lock.write_lock()
+        with fasteners.InterProcessLock(self.settings["chroot_path"] + '.lock'):
+            return self._run()
 
+    def _run(self):
         if "clear-autoresume" in self.settings["options"]:
             self.clear_autoresume()
 
diff --git a/catalyst/lock.py b/catalyst/lock.py
deleted file mode 100644
index e31745b2..00000000
--- a/catalyst/lock.py
+++ /dev/null
@@ -1,58 +0,0 @@
-
-import os
-
-from contextlib import contextmanager
-
-from snakeoil import fileutils
-from snakeoil import osutils
-from catalyst.fileops import ensure_dirs
-
-
-LockInUse = osutils.LockException
-
-class Lock:
-    """
-    A fnctl-based filesystem lock
-    """
-    def __init__(self, lockfile):
-        fileutils.touch(lockfile, mode=0o664)
-        os.chown(lockfile, uid=-1, gid=250)
-        self.lock = osutils.FsLock(lockfile)
-
-    def read_lock(self):
-        self.lock.acquire_read_lock()
-
-    def write_lock(self):
-        self.lock.acquire_write_lock()
-
-    def unlock(self):
-        # Releasing a write lock is the same as a read lock.
-        self.lock.release_write_lock()
-
-class LockDir(Lock):
-    """
-    A fnctl-based filesystem lock in a directory
-    """
-    def __init__(self, lockdir):
-        ensure_dirs(lockdir)
-        lockfile = os.path.join(lockdir, '.catalyst_lock')
-
-        Lock.__init__(self, lockfile)
-
-@contextmanager
-def read_lock(filename):
-    lock = Lock(filename)
-    lock.read_lock()
-    try:
-        yield
-    finally:
-        lock.unlock()
-
-@contextmanager
-def write_lock(filename):
-    lock = Lock(filename)
-    lock.write_lock()
-    try:
-        yield
-    finally:
-        lock.unlock()
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
index b494575a..ef68765d 100644
--- a/catalyst/targets/snapshot.py
+++ b/catalyst/targets/snapshot.py
@@ -5,11 +5,12 @@ Snapshot target
 import subprocess
 import sys
 
+import fasteners
+
 from pathlib import Path
 
 from catalyst import log
 from catalyst.base.targetbase import TargetBase
-from catalyst.lock import write_lock
 from catalyst.support import CatalystError, command
 
 class snapshot(TargetBase):
@@ -93,8 +94,7 @@ class snapshot(TargetBase):
         log.notice('>>> ' + ' '.join([*git_cmd, '|']))
         log.notice('    ' + ' '.join(tar2sqfs_cmd))
 
-        lockfile = self.snapshot.with_suffix('.lock')
-        with write_lock(lockfile):
+        with fasteners.InterProcessLock(self.snapshot.with_suffix('.lock')):
             git = subprocess.Popen(git_cmd,
                                    stdout=subprocess.PIPE,
                                    stderr=sys.stderr,
-- 
2.31.1