Gentoo Archives: gentoo-commits

From: Matt Turner <mattst88@g.o>
To: gentoo-commits@l.g.o
Subject: [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/targets/, catalyst/
Date: Thu, 10 Jun 2021 00:48:49
Message-Id: 1623285614.f535f7c03185442359ab1c5381205de736f547ea.mattst88@gentoo
1 commit: f535f7c03185442359ab1c5381205de736f547ea
2 Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
3 AuthorDate: Wed Jun 9 06:17:31 2021 +0000
4 Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
5 CommitDate: Thu Jun 10 00:40:14 2021 +0000
6 URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=f535f7c0
7
8 catalyst: Replace snakeoil's locks with fasteners
9
10 To no great surprise, the existing locking was broken. For example,
11 clear_chroot() releases the lock. It is called by unpack(), which is
12 part of prepare_sequence. The result is that the whole build could be
13 done without holding the lock.
14
15 Just lock around run(). It's not apparent that finer-grained locking
16 does anything for us.
17
18 Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org>
19
20 catalyst/base/clearbase.py | 2 --
21 catalyst/base/stagebase.py | 8 +-----
22 catalyst/lock.py | 58 --------------------------------------------
23 catalyst/targets/snapshot.py | 6 ++---
24 4 files changed, 4 insertions(+), 70 deletions(-)
25
26 diff --git a/catalyst/base/clearbase.py b/catalyst/base/clearbase.py
27 index dcf6c523..6218330e 100644
28 --- a/catalyst/base/clearbase.py
29 +++ b/catalyst/base/clearbase.py
30 @@ -27,12 +27,10 @@ class ClearBase():
31 self.resume.clear_all(remove=True)
32
33 def clear_chroot(self):
34 - self.chroot_lock.unlock()
35 log.notice('Clearing the chroot path ...')
36 clear_dir(self.settings["chroot_path"], mode=0o755)
37
38 def remove_chroot(self):
39 - self.chroot_lock.unlock()
40 log.notice('Removing the chroot path ...')
41 clear_dir(self.settings["chroot_path"], mode=0o755, remove=True)
42
43
44 diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
45 index 448d6265..fac5d790 100644
46 --- a/catalyst/base/stagebase.py
47 +++ b/catalyst/base/stagebase.py
48 @@ -25,7 +25,6 @@ from catalyst.support import (CatalystError, file_locate, normpath,
49 from catalyst.base.targetbase import TargetBase
50 from catalyst.base.clearbase import ClearBase
51 from catalyst.base.genbase import GenBase
52 -from catalyst.lock import LockDir, LockInUse
53 from catalyst.fileops import ensure_dirs, clear_dir, clear_path
54 from catalyst.base.resume import AutoResume
55
56 @@ -36,9 +35,6 @@ def run_sequence(sequence):
57 sys.stdout.flush()
58 try:
59 func()
60 - except LockInUse:
61 - log.error('Unable to aquire the lock...')
62 - return False
63 except Exception:
64 log.error('Exception running action sequence %s',
65 func.__name__, exc_info=True)
66 @@ -478,7 +474,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
67 """
68 self.settings["chroot_path"] = normpath(self.settings["storedir"] +
69 "/tmp/" + self.settings["target_subpath"].rstrip('/'))
70 - self.chroot_lock = LockDir(self.settings["chroot_path"])
71
72 def set_autoresume_path(self):
73 self.settings["autoresume_path"] = normpath(pjoin(
74 @@ -1365,9 +1360,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
75 except CatalystError:
76 pass
77
78 + @fasteners.interprocess_locked(self.chroot_path + '.lock')
79 def run(self):
80 - self.chroot_lock.write_lock()
81 -
82 if "clear-autoresume" in self.settings["options"]:
83 self.clear_autoresume()
84
85
86 diff --git a/catalyst/lock.py b/catalyst/lock.py
87 deleted file mode 100644
88 index e31745b2..00000000
89 --- a/catalyst/lock.py
90 +++ /dev/null
91 @@ -1,58 +0,0 @@
92 -
93 -import os
94 -
95 -from contextlib import contextmanager
96 -
97 -from snakeoil import fileutils
98 -from snakeoil import osutils
99 -from catalyst.fileops import ensure_dirs
100 -
101 -
102 -LockInUse = osutils.LockException
103 -
104 -class Lock:
105 - """
106 - A fnctl-based filesystem lock
107 - """
108 - def __init__(self, lockfile):
109 - fileutils.touch(lockfile, mode=0o664)
110 - os.chown(lockfile, uid=-1, gid=250)
111 - self.lock = osutils.FsLock(lockfile)
112 -
113 - def read_lock(self):
114 - self.lock.acquire_read_lock()
115 -
116 - def write_lock(self):
117 - self.lock.acquire_write_lock()
118 -
119 - def unlock(self):
120 - # Releasing a write lock is the same as a read lock.
121 - self.lock.release_write_lock()
122 -
123 -class LockDir(Lock):
124 - """
125 - A fnctl-based filesystem lock in a directory
126 - """
127 - def __init__(self, lockdir):
128 - ensure_dirs(lockdir)
129 - lockfile = os.path.join(lockdir, '.catalyst_lock')
130 -
131 - Lock.__init__(self, lockfile)
132 -
133 -@contextmanager
134 -def read_lock(filename):
135 - lock = Lock(filename)
136 - lock.read_lock()
137 - try:
138 - yield
139 - finally:
140 - lock.unlock()
141 -
142 -@contextmanager
143 -def write_lock(filename):
144 - lock = Lock(filename)
145 - lock.write_lock()
146 - try:
147 - yield
148 - finally:
149 - lock.unlock()
150
151 diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
152 index b494575a..ef68765d 100644
153 --- a/catalyst/targets/snapshot.py
154 +++ b/catalyst/targets/snapshot.py
155 @@ -5,11 +5,12 @@ Snapshot target
156 import subprocess
157 import sys
158
159 +import fasteners
160 +
161 from pathlib import Path
162
163 from catalyst import log
164 from catalyst.base.targetbase import TargetBase
165 -from catalyst.lock import write_lock
166 from catalyst.support import CatalystError, command
167
168 class snapshot(TargetBase):
169 @@ -93,8 +94,7 @@ class snapshot(TargetBase):
170 log.notice('>>> ' + ' '.join([*git_cmd, '|']))
171 log.notice(' ' + ' '.join(tar2sqfs_cmd))
172
173 - lockfile = self.snapshot.with_suffix('.lock')
174 - with write_lock(lockfile):
175 + with fasteners.InterProcessLock(self.snapshot.with_suffix('.lock')):
176 git = subprocess.Popen(git_cmd,
177 stdout=subprocess.PIPE,
178 stderr=sys.stderr,