* [gentoo-commits] proj/catalyst:pending/mattst88 commit in: catalyst/targets/, catalyst/, catalyst/base/
@ 2021-01-28 2:09 Matt Turner
2021-01-28 2:41 ` [gentoo-commits] proj/catalyst:master commit in: catalyst/, catalyst/targets/, catalyst/base/ Matt Turner
2021-01-29 23:50 ` [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/ Matt Turner
0 siblings, 2 replies; 5+ messages in thread
From: Matt Turner @ 2021-01-28 2:09 UTC (permalink / raw
To: gentoo-commits
commit: 50ba8aadf5f7096e9079eafec9182d526956cd49
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Sun Jan 24 02:43:47 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jan 28 02:06:46 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=50ba8aad
catalyst: Clean up exception handling
Use 'raise from' where sensible, but a lot of the CatalystErrors are
just trading one CatalystError for another, so remove them.
Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org>
catalyst/base/stagebase.py | 110 ++++++++++++++------------------------
catalyst/main.py | 4 +-
catalyst/support.py | 4 +-
catalyst/targets/livecd_stage2.py | 4 +-
catalyst/targets/netboot.py | 33 ++++--------
catalyst/targets/snapshot.py | 2 +-
6 files changed, 57 insertions(+), 100 deletions(-)
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 28ff8fd2..46cb1fda 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -906,7 +906,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
fstype=fstype, options=options)
cxt.mount()
except Exception as e:
- raise CatalystError(f"Couldn't mount: {source}, {e}")
+ raise CatalystError(f"Couldn't mount: {source}, {e}") from e
def chroot_setup(self):
self.makeconf = read_makeconf(normpath(self.settings["chroot_path"] +
@@ -978,7 +978,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
except OSError as e:
raise CatalystError('Could not write %s: %s' % (
normpath(self.settings["chroot_path"] +
- self.settings["make_conf"]), e))
+ self.settings["make_conf"]), e)) from e
self.resume.enable("chroot_setup")
def write_make_conf(self, setup=True):
@@ -1206,13 +1206,11 @@ class StageBase(TargetBase, ClearBase, GenBase):
# operations, so we get easy glob handling.
log.notice('%s: removing %s', self.settings["spec_prefix"], x)
clear_path(self.settings["stage_path"] + x)
- try:
- if os.path.exists(self.settings["controller_file"]):
- cmd([self.settings['controller_file'], 'clean'],
- env=self.env)
- self.resume.enable("remove")
- except:
- raise
+
+ if os.path.exists(self.settings["controller_file"]):
+ cmd([self.settings['controller_file'], 'clean'],
+ env=self.env)
+ self.resume.enable("remove")
def preclean(self):
if "autoresume" in self.settings["options"] \
@@ -1278,19 +1276,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
log.notice('Resume point detected, skipping run_local operation...')
return
- try:
- if os.path.exists(self.settings["controller_file"]):
- log.info('run_local() starting controller script...')
- cmd([self.settings['controller_file'], 'run'],
- env=self.env)
- self.resume.enable("run_local")
- else:
- log.info('run_local() no controller_file found... %s',
- self.settings['controller_file'])
-
- except CatalystError:
- raise CatalystError("Stage build aborting due to error.",
- print_traceback=False)
+ if os.path.exists(self.settings["controller_file"]):
+ log.info('run_local() starting controller script...')
+ cmd([self.settings['controller_file'], 'run'],
+ env=self.env)
+ self.resume.enable("run_local")
+ else:
+ log.info('run_local() no controller_file found... %s',
+ self.settings['controller_file'])
def setup_environment(self):
log.debug('setup_environment(); settings = %r', self.settings)
@@ -1382,13 +1375,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
[self.settings[self.settings["spec_prefix"] + "/unmerge"]]
# Before cleaning, unmerge stuff
- try:
- cmd([self.settings['controller_file'], 'unmerge'] +
- self.settings[self.settings['spec_prefix'] + '/unmerge'],
- env=self.env)
- log.info('unmerge shell script')
- except CatalystError:
- raise
+ cmd([self.settings['controller_file'], 'unmerge'] +
+ self.settings[self.settings['spec_prefix'] + '/unmerge'],
+ env=self.env)
+ log.info('unmerge shell script')
self.resume.enable("unmerge")
def target_setup(self):
@@ -1457,14 +1447,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
command.append(self.settings[target_pkgs])
else:
command.extend(self.settings[target_pkgs])
- try:
- cmd(command, env=self.env)
- fileutils.touch(build_packages_resume)
- self.resume.enable("build_packages")
- except CatalystError:
- raise CatalystError(
- self.settings["spec_prefix"] +
- "build aborting due to error.")
+ cmd(command, env=self.env)
+ fileutils.touch(build_packages_resume)
+ self.resume.enable("build_packages")
def build_kernel(self):
'''Build all configured kernels'''
@@ -1475,19 +1460,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
return
if "boot/kernel" in self.settings:
- try:
- mynames = self.settings["boot/kernel"]
- if isinstance(mynames, str):
- mynames = [mynames]
- # Execute the script that sets up the kernel build environment
- cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
- for kname in [sanitize_name(name) for name in mynames]:
- self._build_kernel(kname=kname)
- self.resume.enable("build_kernel")
- except CatalystError:
- raise CatalystError(
- "build aborting due to kernel build error.",
- print_traceback=True)
+ mynames = self.settings["boot/kernel"]
+ if isinstance(mynames, str):
+ mynames = [mynames]
+ # Execute the script that sets up the kernel build environment
+ cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
+ for kname in [sanitize_name(name) for name in mynames]:
+ self._build_kernel(kname=kname)
+ self.resume.enable("build_kernel")
def _build_kernel(self, kname):
"Build a single configured kernel by name"
@@ -1531,12 +1511,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
raise CatalystError("Can't find kernel config: %s" %
self.settings[key])
- try:
- shutil.copy(self.settings[key],
- self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
-
- except IOError:
- raise
+ shutil.copy(self.settings[key],
+ self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
def _copy_initramfs_overlay(self, kname):
key = 'boot/kernel/' + kname + '/initramfs_overlay'
@@ -1560,13 +1536,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping bootloader operation...')
return
- try:
- cmd([self.settings['controller_file'], 'bootloader',
- self.settings['target_path'].rstrip('/')],
- env=self.env)
- self.resume.enable("bootloader")
- except CatalystError:
- raise CatalystError("Script aborting due to error.")
+ cmd([self.settings['controller_file'], 'bootloader',
+ self.settings['target_path'].rstrip('/')],
+ env=self.env)
+ self.resume.enable("bootloader")
def livecd_update(self):
if "autoresume" in self.settings["options"] \
@@ -1575,14 +1548,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping build_packages operation...')
return
- try:
- cmd([self.settings['controller_file'], 'livecd-update'],
- env=self.env)
- self.resume.enable("livecd_update")
-
- except CatalystError:
- raise CatalystError(
- "build aborting due to livecd_update error.")
+ cmd([self.settings['controller_file'], 'livecd-update'],
+ env=self.env)
+ self.resume.enable("livecd_update")
@staticmethod
def _debug_pause_():
diff --git a/catalyst/main.py b/catalyst/main.py
index b0d9015f..0de1040f 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -77,10 +77,10 @@ def build_target(addlargs):
target = addlargs["target"].replace('-', '_')
module = import_module(target)
target = getattr(module, target)(conf_values, addlargs)
- except AttributeError:
+ except AttributeError as e:
raise CatalystError(
"Target \"%s\" not available." % target,
- print_traceback=True)
+ print_traceback=True) from e
except CatalystError:
return False
return target.run()
diff --git a/catalyst/support.py b/catalyst/support.py
index fa652987..fc50fa34 100644
--- a/catalyst/support.py
+++ b/catalyst/support.py
@@ -140,9 +140,9 @@ def read_makeconf(mymakeconffile):
if os.path.exists(mymakeconffile):
try:
return read_bash_dict(mymakeconffile, sourcing_command="source")
- except Exception:
+ except Exception as e:
raise CatalystError("Could not parse make.conf file " +
- mymakeconffile, print_traceback=True)
+ mymakeconffile, print_traceback=True) from e
else:
makeconf = {}
return makeconf
diff --git a/catalyst/targets/livecd_stage2.py b/catalyst/targets/livecd_stage2.py
index e90e9f53..ff4ea62a 100644
--- a/catalyst/targets/livecd_stage2.py
+++ b/catalyst/targets/livecd_stage2.py
@@ -79,11 +79,11 @@ class livecd_stage2(StageBase):
"livecd/modblacklist"].split()
for x in self.settings["livecd/modblacklist"]:
myf.write("\nblacklist "+x)
- except:
+ except Exception as e:
raise CatalystError("Couldn't open " +
self.settings["chroot_path"] +
"/etc/modprobe.d/blacklist.conf.",
- print_traceback=True)
+ print_traceback=True) from e
def set_action_sequence(self):
self.build_sequence.extend([
diff --git a/catalyst/targets/netboot.py b/catalyst/targets/netboot.py
index a2a9fcb3..38d0cb45 100644
--- a/catalyst/targets/netboot.py
+++ b/catalyst/targets/netboot.py
@@ -30,17 +30,14 @@ class netboot(StageBase):
])
def __init__(self, spec, addlargs):
- try:
- if "netboot/packages" in addlargs:
- if isinstance(addlargs['netboot/packages'], str):
- loopy = [addlargs["netboot/packages"]]
- else:
- loopy = addlargs["netboot/packages"]
+ if "netboot/packages" in addlargs:
+ if isinstance(addlargs['netboot/packages'], str):
+ loopy = [addlargs["netboot/packages"]]
+ else:
+ loopy = addlargs["netboot/packages"]
- for x in loopy:
- self.valid_values |= {"netboot/packages/"+x+"/files"}
- except:
- raise CatalystError("configuration error in netboot/packages.")
+ for x in loopy:
+ self.valid_values |= {"netboot/packages/"+x+"/files"}
StageBase.__init__(self, spec, addlargs)
self.settings["merge_path"] = normpath("/tmp/image/")
@@ -89,12 +86,8 @@ class netboot(StageBase):
else:
myfiles.append(self.settings["netboot/extra_files"])
- try:
- cmd([self.settings['controller_file'], 'image'] +
- myfiles, env=self.env)
- except CatalystError:
- raise CatalystError("Failed to copy files to image!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'image'] +
+ myfiles, env=self.env)
self.resume.enable("copy_files_to_image")
@@ -116,12 +109,8 @@ class netboot(StageBase):
# we're done, move the kernels to builds/*
# no auto resume here as we always want the
# freshest images moved
- try:
- cmd([self.settings['controller_file'], 'final'], env=self.env)
- log.notice('Netboot Build Finished!')
- except CatalystError:
- raise CatalystError("Failed to move kernel images!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'final'], env=self.env)
+ log.notice('Netboot Build Finished!')
def remove(self):
if "autoresume" in self.settings["options"] \
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
index 7732312c..6b727600 100644
--- a/catalyst/targets/snapshot.py
+++ b/catalyst/targets/snapshot.py
@@ -73,7 +73,7 @@ class snapshot(TargetBase):
except subprocess.CalledProcessError as e:
raise CatalystError(f'{e.cmd} failed with return code'
f'{e.returncode}\n'
- f'{e.output}\n')
+ f'{e.output}\n') from e
def run(self):
if self.settings['snapshot_treeish'] == 'stable':
^ permalink raw reply related [flat|nested] 5+ messages in thread* [gentoo-commits] proj/catalyst:master commit in: catalyst/, catalyst/targets/, catalyst/base/
2021-01-28 2:09 [gentoo-commits] proj/catalyst:pending/mattst88 commit in: catalyst/targets/, catalyst/, catalyst/base/ Matt Turner
@ 2021-01-28 2:41 ` Matt Turner
2021-01-29 23:50 ` [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/ Matt Turner
1 sibling, 0 replies; 5+ messages in thread
From: Matt Turner @ 2021-01-28 2:41 UTC (permalink / raw
To: gentoo-commits
commit: 50ba8aadf5f7096e9079eafec9182d526956cd49
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Sun Jan 24 02:43:47 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jan 28 02:06:46 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=50ba8aad
catalyst: Clean up exception handling
Use 'raise from' where sensible, but a lot of the CatalystErrors are
just trading one CatalystError for another, so remove them.
Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org>
catalyst/base/stagebase.py | 110 ++++++++++++++------------------------
catalyst/main.py | 4 +-
catalyst/support.py | 4 +-
catalyst/targets/livecd_stage2.py | 4 +-
catalyst/targets/netboot.py | 33 ++++--------
catalyst/targets/snapshot.py | 2 +-
6 files changed, 57 insertions(+), 100 deletions(-)
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 28ff8fd2..46cb1fda 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -906,7 +906,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
fstype=fstype, options=options)
cxt.mount()
except Exception as e:
- raise CatalystError(f"Couldn't mount: {source}, {e}")
+ raise CatalystError(f"Couldn't mount: {source}, {e}") from e
def chroot_setup(self):
self.makeconf = read_makeconf(normpath(self.settings["chroot_path"] +
@@ -978,7 +978,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
except OSError as e:
raise CatalystError('Could not write %s: %s' % (
normpath(self.settings["chroot_path"] +
- self.settings["make_conf"]), e))
+ self.settings["make_conf"]), e)) from e
self.resume.enable("chroot_setup")
def write_make_conf(self, setup=True):
@@ -1206,13 +1206,11 @@ class StageBase(TargetBase, ClearBase, GenBase):
# operations, so we get easy glob handling.
log.notice('%s: removing %s', self.settings["spec_prefix"], x)
clear_path(self.settings["stage_path"] + x)
- try:
- if os.path.exists(self.settings["controller_file"]):
- cmd([self.settings['controller_file'], 'clean'],
- env=self.env)
- self.resume.enable("remove")
- except:
- raise
+
+ if os.path.exists(self.settings["controller_file"]):
+ cmd([self.settings['controller_file'], 'clean'],
+ env=self.env)
+ self.resume.enable("remove")
def preclean(self):
if "autoresume" in self.settings["options"] \
@@ -1278,19 +1276,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
log.notice('Resume point detected, skipping run_local operation...')
return
- try:
- if os.path.exists(self.settings["controller_file"]):
- log.info('run_local() starting controller script...')
- cmd([self.settings['controller_file'], 'run'],
- env=self.env)
- self.resume.enable("run_local")
- else:
- log.info('run_local() no controller_file found... %s',
- self.settings['controller_file'])
-
- except CatalystError:
- raise CatalystError("Stage build aborting due to error.",
- print_traceback=False)
+ if os.path.exists(self.settings["controller_file"]):
+ log.info('run_local() starting controller script...')
+ cmd([self.settings['controller_file'], 'run'],
+ env=self.env)
+ self.resume.enable("run_local")
+ else:
+ log.info('run_local() no controller_file found... %s',
+ self.settings['controller_file'])
def setup_environment(self):
log.debug('setup_environment(); settings = %r', self.settings)
@@ -1382,13 +1375,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
[self.settings[self.settings["spec_prefix"] + "/unmerge"]]
# Before cleaning, unmerge stuff
- try:
- cmd([self.settings['controller_file'], 'unmerge'] +
- self.settings[self.settings['spec_prefix'] + '/unmerge'],
- env=self.env)
- log.info('unmerge shell script')
- except CatalystError:
- raise
+ cmd([self.settings['controller_file'], 'unmerge'] +
+ self.settings[self.settings['spec_prefix'] + '/unmerge'],
+ env=self.env)
+ log.info('unmerge shell script')
self.resume.enable("unmerge")
def target_setup(self):
@@ -1457,14 +1447,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
command.append(self.settings[target_pkgs])
else:
command.extend(self.settings[target_pkgs])
- try:
- cmd(command, env=self.env)
- fileutils.touch(build_packages_resume)
- self.resume.enable("build_packages")
- except CatalystError:
- raise CatalystError(
- self.settings["spec_prefix"] +
- "build aborting due to error.")
+ cmd(command, env=self.env)
+ fileutils.touch(build_packages_resume)
+ self.resume.enable("build_packages")
def build_kernel(self):
'''Build all configured kernels'''
@@ -1475,19 +1460,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
return
if "boot/kernel" in self.settings:
- try:
- mynames = self.settings["boot/kernel"]
- if isinstance(mynames, str):
- mynames = [mynames]
- # Execute the script that sets up the kernel build environment
- cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
- for kname in [sanitize_name(name) for name in mynames]:
- self._build_kernel(kname=kname)
- self.resume.enable("build_kernel")
- except CatalystError:
- raise CatalystError(
- "build aborting due to kernel build error.",
- print_traceback=True)
+ mynames = self.settings["boot/kernel"]
+ if isinstance(mynames, str):
+ mynames = [mynames]
+ # Execute the script that sets up the kernel build environment
+ cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
+ for kname in [sanitize_name(name) for name in mynames]:
+ self._build_kernel(kname=kname)
+ self.resume.enable("build_kernel")
def _build_kernel(self, kname):
"Build a single configured kernel by name"
@@ -1531,12 +1511,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
raise CatalystError("Can't find kernel config: %s" %
self.settings[key])
- try:
- shutil.copy(self.settings[key],
- self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
-
- except IOError:
- raise
+ shutil.copy(self.settings[key],
+ self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
def _copy_initramfs_overlay(self, kname):
key = 'boot/kernel/' + kname + '/initramfs_overlay'
@@ -1560,13 +1536,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping bootloader operation...')
return
- try:
- cmd([self.settings['controller_file'], 'bootloader',
- self.settings['target_path'].rstrip('/')],
- env=self.env)
- self.resume.enable("bootloader")
- except CatalystError:
- raise CatalystError("Script aborting due to error.")
+ cmd([self.settings['controller_file'], 'bootloader',
+ self.settings['target_path'].rstrip('/')],
+ env=self.env)
+ self.resume.enable("bootloader")
def livecd_update(self):
if "autoresume" in self.settings["options"] \
@@ -1575,14 +1548,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping build_packages operation...')
return
- try:
- cmd([self.settings['controller_file'], 'livecd-update'],
- env=self.env)
- self.resume.enable("livecd_update")
-
- except CatalystError:
- raise CatalystError(
- "build aborting due to livecd_update error.")
+ cmd([self.settings['controller_file'], 'livecd-update'],
+ env=self.env)
+ self.resume.enable("livecd_update")
@staticmethod
def _debug_pause_():
diff --git a/catalyst/main.py b/catalyst/main.py
index b0d9015f..0de1040f 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -77,10 +77,10 @@ def build_target(addlargs):
target = addlargs["target"].replace('-', '_')
module = import_module(target)
target = getattr(module, target)(conf_values, addlargs)
- except AttributeError:
+ except AttributeError as e:
raise CatalystError(
"Target \"%s\" not available." % target,
- print_traceback=True)
+ print_traceback=True) from e
except CatalystError:
return False
return target.run()
diff --git a/catalyst/support.py b/catalyst/support.py
index fa652987..fc50fa34 100644
--- a/catalyst/support.py
+++ b/catalyst/support.py
@@ -140,9 +140,9 @@ def read_makeconf(mymakeconffile):
if os.path.exists(mymakeconffile):
try:
return read_bash_dict(mymakeconffile, sourcing_command="source")
- except Exception:
+ except Exception as e:
raise CatalystError("Could not parse make.conf file " +
- mymakeconffile, print_traceback=True)
+ mymakeconffile, print_traceback=True) from e
else:
makeconf = {}
return makeconf
diff --git a/catalyst/targets/livecd_stage2.py b/catalyst/targets/livecd_stage2.py
index e90e9f53..ff4ea62a 100644
--- a/catalyst/targets/livecd_stage2.py
+++ b/catalyst/targets/livecd_stage2.py
@@ -79,11 +79,11 @@ class livecd_stage2(StageBase):
"livecd/modblacklist"].split()
for x in self.settings["livecd/modblacklist"]:
myf.write("\nblacklist "+x)
- except:
+ except Exception as e:
raise CatalystError("Couldn't open " +
self.settings["chroot_path"] +
"/etc/modprobe.d/blacklist.conf.",
- print_traceback=True)
+ print_traceback=True) from e
def set_action_sequence(self):
self.build_sequence.extend([
diff --git a/catalyst/targets/netboot.py b/catalyst/targets/netboot.py
index a2a9fcb3..38d0cb45 100644
--- a/catalyst/targets/netboot.py
+++ b/catalyst/targets/netboot.py
@@ -30,17 +30,14 @@ class netboot(StageBase):
])
def __init__(self, spec, addlargs):
- try:
- if "netboot/packages" in addlargs:
- if isinstance(addlargs['netboot/packages'], str):
- loopy = [addlargs["netboot/packages"]]
- else:
- loopy = addlargs["netboot/packages"]
+ if "netboot/packages" in addlargs:
+ if isinstance(addlargs['netboot/packages'], str):
+ loopy = [addlargs["netboot/packages"]]
+ else:
+ loopy = addlargs["netboot/packages"]
- for x in loopy:
- self.valid_values |= {"netboot/packages/"+x+"/files"}
- except:
- raise CatalystError("configuration error in netboot/packages.")
+ for x in loopy:
+ self.valid_values |= {"netboot/packages/"+x+"/files"}
StageBase.__init__(self, spec, addlargs)
self.settings["merge_path"] = normpath("/tmp/image/")
@@ -89,12 +86,8 @@ class netboot(StageBase):
else:
myfiles.append(self.settings["netboot/extra_files"])
- try:
- cmd([self.settings['controller_file'], 'image'] +
- myfiles, env=self.env)
- except CatalystError:
- raise CatalystError("Failed to copy files to image!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'image'] +
+ myfiles, env=self.env)
self.resume.enable("copy_files_to_image")
@@ -116,12 +109,8 @@ class netboot(StageBase):
# we're done, move the kernels to builds/*
# no auto resume here as we always want the
# freshest images moved
- try:
- cmd([self.settings['controller_file'], 'final'], env=self.env)
- log.notice('Netboot Build Finished!')
- except CatalystError:
- raise CatalystError("Failed to move kernel images!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'final'], env=self.env)
+ log.notice('Netboot Build Finished!')
def remove(self):
if "autoresume" in self.settings["options"] \
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
index 7732312c..6b727600 100644
--- a/catalyst/targets/snapshot.py
+++ b/catalyst/targets/snapshot.py
@@ -73,7 +73,7 @@ class snapshot(TargetBase):
except subprocess.CalledProcessError as e:
raise CatalystError(f'{e.cmd} failed with return code'
f'{e.returncode}\n'
- f'{e.output}\n')
+ f'{e.output}\n') from e
def run(self):
if self.settings['snapshot_treeish'] == 'stable':
^ permalink raw reply related [flat|nested] 5+ messages in thread* [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/
2021-01-28 2:09 [gentoo-commits] proj/catalyst:pending/mattst88 commit in: catalyst/targets/, catalyst/, catalyst/base/ Matt Turner
2021-01-28 2:41 ` [gentoo-commits] proj/catalyst:master commit in: catalyst/, catalyst/targets/, catalyst/base/ Matt Turner
@ 2021-01-29 23:50 ` Matt Turner
1 sibling, 0 replies; 5+ messages in thread
From: Matt Turner @ 2021-01-29 23:50 UTC (permalink / raw
To: gentoo-commits
commit: 50ba8aadf5f7096e9079eafec9182d526956cd49
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Sun Jan 24 02:43:47 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jan 28 02:06:46 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=50ba8aad
catalyst: Clean up exception handling
Use 'raise from' where sensible, but a lot of the CatalystErrors are
just trading one CatalystError for another, so remove them.
Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org>
catalyst/base/stagebase.py | 110 ++++++++++++++------------------------
catalyst/main.py | 4 +-
catalyst/support.py | 4 +-
catalyst/targets/livecd_stage2.py | 4 +-
catalyst/targets/netboot.py | 33 ++++--------
catalyst/targets/snapshot.py | 2 +-
6 files changed, 57 insertions(+), 100 deletions(-)
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 28ff8fd2..46cb1fda 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -906,7 +906,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
fstype=fstype, options=options)
cxt.mount()
except Exception as e:
- raise CatalystError(f"Couldn't mount: {source}, {e}")
+ raise CatalystError(f"Couldn't mount: {source}, {e}") from e
def chroot_setup(self):
self.makeconf = read_makeconf(normpath(self.settings["chroot_path"] +
@@ -978,7 +978,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
except OSError as e:
raise CatalystError('Could not write %s: %s' % (
normpath(self.settings["chroot_path"] +
- self.settings["make_conf"]), e))
+ self.settings["make_conf"]), e)) from e
self.resume.enable("chroot_setup")
def write_make_conf(self, setup=True):
@@ -1206,13 +1206,11 @@ class StageBase(TargetBase, ClearBase, GenBase):
# operations, so we get easy glob handling.
log.notice('%s: removing %s', self.settings["spec_prefix"], x)
clear_path(self.settings["stage_path"] + x)
- try:
- if os.path.exists(self.settings["controller_file"]):
- cmd([self.settings['controller_file'], 'clean'],
- env=self.env)
- self.resume.enable("remove")
- except:
- raise
+
+ if os.path.exists(self.settings["controller_file"]):
+ cmd([self.settings['controller_file'], 'clean'],
+ env=self.env)
+ self.resume.enable("remove")
def preclean(self):
if "autoresume" in self.settings["options"] \
@@ -1278,19 +1276,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
log.notice('Resume point detected, skipping run_local operation...')
return
- try:
- if os.path.exists(self.settings["controller_file"]):
- log.info('run_local() starting controller script...')
- cmd([self.settings['controller_file'], 'run'],
- env=self.env)
- self.resume.enable("run_local")
- else:
- log.info('run_local() no controller_file found... %s',
- self.settings['controller_file'])
-
- except CatalystError:
- raise CatalystError("Stage build aborting due to error.",
- print_traceback=False)
+ if os.path.exists(self.settings["controller_file"]):
+ log.info('run_local() starting controller script...')
+ cmd([self.settings['controller_file'], 'run'],
+ env=self.env)
+ self.resume.enable("run_local")
+ else:
+ log.info('run_local() no controller_file found... %s',
+ self.settings['controller_file'])
def setup_environment(self):
log.debug('setup_environment(); settings = %r', self.settings)
@@ -1382,13 +1375,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
[self.settings[self.settings["spec_prefix"] + "/unmerge"]]
# Before cleaning, unmerge stuff
- try:
- cmd([self.settings['controller_file'], 'unmerge'] +
- self.settings[self.settings['spec_prefix'] + '/unmerge'],
- env=self.env)
- log.info('unmerge shell script')
- except CatalystError:
- raise
+ cmd([self.settings['controller_file'], 'unmerge'] +
+ self.settings[self.settings['spec_prefix'] + '/unmerge'],
+ env=self.env)
+ log.info('unmerge shell script')
self.resume.enable("unmerge")
def target_setup(self):
@@ -1457,14 +1447,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
command.append(self.settings[target_pkgs])
else:
command.extend(self.settings[target_pkgs])
- try:
- cmd(command, env=self.env)
- fileutils.touch(build_packages_resume)
- self.resume.enable("build_packages")
- except CatalystError:
- raise CatalystError(
- self.settings["spec_prefix"] +
- "build aborting due to error.")
+ cmd(command, env=self.env)
+ fileutils.touch(build_packages_resume)
+ self.resume.enable("build_packages")
def build_kernel(self):
'''Build all configured kernels'''
@@ -1475,19 +1460,14 @@ class StageBase(TargetBase, ClearBase, GenBase):
return
if "boot/kernel" in self.settings:
- try:
- mynames = self.settings["boot/kernel"]
- if isinstance(mynames, str):
- mynames = [mynames]
- # Execute the script that sets up the kernel build environment
- cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
- for kname in [sanitize_name(name) for name in mynames]:
- self._build_kernel(kname=kname)
- self.resume.enable("build_kernel")
- except CatalystError:
- raise CatalystError(
- "build aborting due to kernel build error.",
- print_traceback=True)
+ mynames = self.settings["boot/kernel"]
+ if isinstance(mynames, str):
+ mynames = [mynames]
+ # Execute the script that sets up the kernel build environment
+ cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env)
+ for kname in [sanitize_name(name) for name in mynames]:
+ self._build_kernel(kname=kname)
+ self.resume.enable("build_kernel")
def _build_kernel(self, kname):
"Build a single configured kernel by name"
@@ -1531,12 +1511,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
raise CatalystError("Can't find kernel config: %s" %
self.settings[key])
- try:
- shutil.copy(self.settings[key],
- self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
-
- except IOError:
- raise
+ shutil.copy(self.settings[key],
+ self.settings['chroot_path'] + '/var/tmp/' + kname + '.config')
def _copy_initramfs_overlay(self, kname):
key = 'boot/kernel/' + kname + '/initramfs_overlay'
@@ -1560,13 +1536,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping bootloader operation...')
return
- try:
- cmd([self.settings['controller_file'], 'bootloader',
- self.settings['target_path'].rstrip('/')],
- env=self.env)
- self.resume.enable("bootloader")
- except CatalystError:
- raise CatalystError("Script aborting due to error.")
+ cmd([self.settings['controller_file'], 'bootloader',
+ self.settings['target_path'].rstrip('/')],
+ env=self.env)
+ self.resume.enable("bootloader")
def livecd_update(self):
if "autoresume" in self.settings["options"] \
@@ -1575,14 +1548,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
'Resume point detected, skipping build_packages operation...')
return
- try:
- cmd([self.settings['controller_file'], 'livecd-update'],
- env=self.env)
- self.resume.enable("livecd_update")
-
- except CatalystError:
- raise CatalystError(
- "build aborting due to livecd_update error.")
+ cmd([self.settings['controller_file'], 'livecd-update'],
+ env=self.env)
+ self.resume.enable("livecd_update")
@staticmethod
def _debug_pause_():
diff --git a/catalyst/main.py b/catalyst/main.py
index b0d9015f..0de1040f 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -77,10 +77,10 @@ def build_target(addlargs):
target = addlargs["target"].replace('-', '_')
module = import_module(target)
target = getattr(module, target)(conf_values, addlargs)
- except AttributeError:
+ except AttributeError as e:
raise CatalystError(
"Target \"%s\" not available." % target,
- print_traceback=True)
+ print_traceback=True) from e
except CatalystError:
return False
return target.run()
diff --git a/catalyst/support.py b/catalyst/support.py
index fa652987..fc50fa34 100644
--- a/catalyst/support.py
+++ b/catalyst/support.py
@@ -140,9 +140,9 @@ def read_makeconf(mymakeconffile):
if os.path.exists(mymakeconffile):
try:
return read_bash_dict(mymakeconffile, sourcing_command="source")
- except Exception:
+ except Exception as e:
raise CatalystError("Could not parse make.conf file " +
- mymakeconffile, print_traceback=True)
+ mymakeconffile, print_traceback=True) from e
else:
makeconf = {}
return makeconf
diff --git a/catalyst/targets/livecd_stage2.py b/catalyst/targets/livecd_stage2.py
index e90e9f53..ff4ea62a 100644
--- a/catalyst/targets/livecd_stage2.py
+++ b/catalyst/targets/livecd_stage2.py
@@ -79,11 +79,11 @@ class livecd_stage2(StageBase):
"livecd/modblacklist"].split()
for x in self.settings["livecd/modblacklist"]:
myf.write("\nblacklist "+x)
- except:
+ except Exception as e:
raise CatalystError("Couldn't open " +
self.settings["chroot_path"] +
"/etc/modprobe.d/blacklist.conf.",
- print_traceback=True)
+ print_traceback=True) from e
def set_action_sequence(self):
self.build_sequence.extend([
diff --git a/catalyst/targets/netboot.py b/catalyst/targets/netboot.py
index a2a9fcb3..38d0cb45 100644
--- a/catalyst/targets/netboot.py
+++ b/catalyst/targets/netboot.py
@@ -30,17 +30,14 @@ class netboot(StageBase):
])
def __init__(self, spec, addlargs):
- try:
- if "netboot/packages" in addlargs:
- if isinstance(addlargs['netboot/packages'], str):
- loopy = [addlargs["netboot/packages"]]
- else:
- loopy = addlargs["netboot/packages"]
+ if "netboot/packages" in addlargs:
+ if isinstance(addlargs['netboot/packages'], str):
+ loopy = [addlargs["netboot/packages"]]
+ else:
+ loopy = addlargs["netboot/packages"]
- for x in loopy:
- self.valid_values |= {"netboot/packages/"+x+"/files"}
- except:
- raise CatalystError("configuration error in netboot/packages.")
+ for x in loopy:
+ self.valid_values |= {"netboot/packages/"+x+"/files"}
StageBase.__init__(self, spec, addlargs)
self.settings["merge_path"] = normpath("/tmp/image/")
@@ -89,12 +86,8 @@ class netboot(StageBase):
else:
myfiles.append(self.settings["netboot/extra_files"])
- try:
- cmd([self.settings['controller_file'], 'image'] +
- myfiles, env=self.env)
- except CatalystError:
- raise CatalystError("Failed to copy files to image!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'image'] +
+ myfiles, env=self.env)
self.resume.enable("copy_files_to_image")
@@ -116,12 +109,8 @@ class netboot(StageBase):
# we're done, move the kernels to builds/*
# no auto resume here as we always want the
# freshest images moved
- try:
- cmd([self.settings['controller_file'], 'final'], env=self.env)
- log.notice('Netboot Build Finished!')
- except CatalystError:
- raise CatalystError("Failed to move kernel images!",
- print_traceback=True)
+ cmd([self.settings['controller_file'], 'final'], env=self.env)
+ log.notice('Netboot Build Finished!')
def remove(self):
if "autoresume" in self.settings["options"] \
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
index 7732312c..6b727600 100644
--- a/catalyst/targets/snapshot.py
+++ b/catalyst/targets/snapshot.py
@@ -73,7 +73,7 @@ class snapshot(TargetBase):
except subprocess.CalledProcessError as e:
raise CatalystError(f'{e.cmd} failed with return code'
f'{e.returncode}\n'
- f'{e.output}\n')
+ f'{e.output}\n') from e
def run(self):
if self.settings['snapshot_treeish'] == 'stable':
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/
@ 2021-06-10 1:35 Matt Turner
0 siblings, 0 replies; 5+ messages in thread
From: Matt Turner @ 2021-06-10 1:35 UTC (permalink / raw
To: gentoo-commits
commit: 8a9780b8bef53d427fa7d68b7df85e2d34f4dd57
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Wed Jun 9 06:17:31 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jun 10 01:35:32 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=8a9780b8
catalyst: Replace snakeoil's locks with fasteners
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 <AT> 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(-)
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..34d10389 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.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,
^ permalink raw reply related [flat|nested] 5+ messages in thread* [gentoo-commits] proj/catalyst:master commit in: catalyst/targets/, catalyst/base/, catalyst/
@ 2021-06-11 3:30 Matt Turner
2022-01-30 20:42 ` [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/ Matt Turner
0 siblings, 1 reply; 5+ messages in thread
From: Matt Turner @ 2021-06-11 3:30 UTC (permalink / raw
To: gentoo-commits
commit: b3c917f7fa73d11c69b7e55dc7a00bc18a18edc7
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Wed Jun 9 06:17:31 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jun 10 04:23:54 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=b3c917f7
catalyst: Replace snakeoil's locks with fasteners
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.
Bug: https://bugs.gentoo.org/791583
Signed-off-by: Matt Turner <mattst88 <AT> 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(-)
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,
^ permalink raw reply related [flat|nested] 5+ messages in thread* [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/
2021-06-11 3:30 [gentoo-commits] proj/catalyst:master commit in: catalyst/targets/, catalyst/base/, catalyst/ Matt Turner
@ 2022-01-30 20:42 ` Matt Turner
0 siblings, 0 replies; 5+ messages in thread
From: Matt Turner @ 2022-01-30 20:42 UTC (permalink / raw
To: gentoo-commits
commit: b3c917f7fa73d11c69b7e55dc7a00bc18a18edc7
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Wed Jun 9 06:17:31 2021 +0000
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Thu Jun 10 04:23:54 2021 +0000
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=b3c917f7
catalyst: Replace snakeoil's locks with fasteners
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.
Bug: https://bugs.gentoo.org/791583
Signed-off-by: Matt Turner <mattst88 <AT> 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(-)
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,
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-30 20:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-28 2:09 [gentoo-commits] proj/catalyst:pending/mattst88 commit in: catalyst/targets/, catalyst/, catalyst/base/ Matt Turner
2021-01-28 2:41 ` [gentoo-commits] proj/catalyst:master commit in: catalyst/, catalyst/targets/, catalyst/base/ Matt Turner
2021-01-29 23:50 ` [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/ Matt Turner
-- strict thread matches above, loose matches on Subject: below --
2021-06-10 1:35 Matt Turner
2021-06-11 3:30 [gentoo-commits] proj/catalyst:master commit in: catalyst/targets/, catalyst/base/, catalyst/ Matt Turner
2022-01-30 20:42 ` [gentoo-commits] proj/catalyst:wip/mattst88 commit in: catalyst/base/, catalyst/, catalyst/targets/ Matt Turner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox