From 3e0a5dd7c7549636eb70c6a641987da66742f1db Mon Sep 17 00:00:00 2001
From: "Ira W. Snyder" <devel@irasnyder.com>
Date: Thu, 30 Oct 2008 20:47:46 -0700
Subject: [PATCH] Use exceptions for error handling

The previous error handling strategy was very Linux kernel like, showing
influence from what I was working on while I designed this program.

Refactor the program to use exceptions, which significantly reduces the
amount of code needed for error handling. Best practices are now
followed: we only catch exceptions where we need to log a message and
rethrow, or for a complete unit of work.

Signed-off-by: Ira W. Snyder <devel@irasnyder.com>
---
 PAR2Set/Base.py             | 135 +++++++++++-------------------------
 PAR2Set/ExtractFirstBase.py |  31 ++++-----
 PAR2Set/Join.py             |  42 +----------
 PAR2Set/NewRAR.py           |  11 +--
 PAR2Set/NoExtract.py        |  21 +++---
 PAR2Set/OldRAR.py           |  11 +--
 PAR2Set/ZIP.py              |  11 +--
 RarslaveDetector.py         |  25 +++----
 rarslave.py                 |  30 +++++---
 rsutil/common.py            |  31 +++++----
 10 files changed, 124 insertions(+), 224 deletions(-)

diff --git a/PAR2Set/Base.py b/PAR2Set/Base.py
index 533ff4f..a5d3dcb 100644
--- a/PAR2Set/Base.py
+++ b/PAR2Set/Base.py
@@ -109,25 +109,7 @@ class Base (object):
 		"""Verify and Repair a PAR2Set. This is done using the par2repair command by
 		   default"""
 
-		PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair')
-
-		# assemble the command
-		# par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES]
-		command = "%s \"%s\" " % (PAR2_CMD, self.p2file)
-
-		for f in self.all_p2files:
-			if f != self.p2file:
-				command += "\"%s\" " % os.path.split (f)[1]
-
-		# run the command
-		ret = rsutil.common.run_command (command, self.dir)
-
-		# check the result
-		if ret != 0:
-			logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file)
-			return -rsutil.common.ECHECK
-
-		return rsutil.common.SUCCESS
+		rsutil.common.run_command(['par2repair'] + self.all_p2files, self.dir)
 
 	def find_deleteable_files (self):
 		"""Find all files which are deletable by using the regular expression from the
@@ -147,32 +129,38 @@ class Base (object):
 
 		assert os.path.isdir (dir)
 
-		done = False
-		valid_y = ['Y', 'YES']
-		valid_n = ['N', 'NO', '']
-
+		# If we are interactive, decide yes or no
 		if interactive:
-			while not done:
+			while True:
 				print 'Do you want to delete the following?:'
 				for f in files:
 					print f
-				s = raw_input ('Delete [y/N]: ').upper()
 
-				if s in valid_y + valid_n:
-					done = True
+				s = raw_input('Delete [y/N]: ').upper()
+
+				# If we got a valid no answer, leave now
+				if s in ['N', 'NO', '']:
+					return
+
+				# If we got a valid yes answer, delete them
+				if s in ['Y', 'YES']:
+					break
 
-			if s in valid_n:
-				return rsutil.common.SUCCESS
+				# Not a good answer, ask again
+				print 'Invalid response'
 
+		# We got a yes answer (or are non-interactive), delete the files
 		for f in files:
-			try:
-				os.remove (os.path.join (dir, f))
-				logging.debug ('Deleteing: %s' % os.path.join (dir, f))
-			except:
-				logging.error ('Failed to delete: %s' % os.path.join (dir, f))
-				return -rsutil.common.EDELETE
 
-		return rsutil.common.SUCCESS
+			# Get the full filename
+			fullname = os.path.join(dir, f)
+
+			# Delete the file
+			try:
+				os.remove(fullname)
+				logging.debug('Deleting: %s' % fullname)
+			except OSError:
+				logging.error('Failed to delete: %s' % fullname)
 
 	def runDelete (self):
 		"""Run the delete operation and return the result"""
@@ -187,78 +175,39 @@ class Base (object):
 		"""Run all of the major sections in the class: repair, extraction, and deletion."""
 
 		# Repair Stage
-		ret = self.runVerifyAndRepair ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Repair stage failed for: %s' % self.p2file)
-			return -rsutil.common.ECHECK
+		try:
+			self.runVerifyAndRepair ()
+		except (RuntimeError, OSError):
+			logging.critical('Repair stage failed for: %s' % self.p2file)
+			raise
 
-		self.update_matches ()
+		self.update_matches()
 
 		# Extraction Stage
-		ret = self.runExtract ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Extraction stage failed for: %s' % self.p2file)
-			return -rsutil.common.EEXTRACT
+		try:
+			self.runExtract ()
+		except (RuntimeError, OSError):
+			logging.critical('Extraction stage failed for: %s' % self.p2file)
+			raise
 
 		self.update_matches ()
 
 		# Deletion Stage
-		ret = self.runDelete ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Deletion stage failed for: %s' % self.p2file)
-			return -rsutil.common.EDELETE
-
-		logging.info ('Successfully completed: %s' % self.p2file)
-		return rsutil.common.SUCCESS
-
-	def safe_create_directory (self, dir):
-		"""Safely create a directory, logging the result.
-
-		   dir -- the directory to create (None is ignored)"""
-
-		if dir == None:
-			return rsutil.common.SUCCESS
-
-		if os.path.isdir (dir):
-			return rsutil.common.SUCCESS
-
 		try:
-			os.makedirs (dir)
-			logging.info ('Created directory: %s' % dir)
-		except OSError:
-			logging.critical ('FAILED to create directory: %s' % dir)
-			return -rsutil.common.ECREATE
+			self.runDelete ()
+		except (RuntimeError, OSError):
+			logging.critical('Deletion stage failed for: %s' % self.p2file)
+			raise
 
-		return rsutil.common.SUCCESS
+		logging.info ('Successfully completed: %s' % self.p2file)
 
-	def runExtract (self, todir=None):
+	def runExtract (self):
 		"""Extract all heads of this set and return the result"""
 
-		# Extract to the head's dir if we don't care where to extract
-		if todir == None:
-			todir = self.dir
-
-		# Create the directory $todir if it doesn't exist
-		ret = self.safe_create_directory (todir)
-
-		if ret != rsutil.common.SUCCESS:
-			return -rsutil.common.EEXTRACT
-
 		# Call the extraction function on each head
 		for h in self.find_extraction_heads ():
 			full_head = rsutil.common.full_abspath (os.path.join (self.dir, h))
-			ret = self.extraction_function (full_head, todir)
-			logging.debug ('Extraction Function returned: %d' % ret)
-
-			# Check error code
-			if ret != rsutil.common.SUCCESS:
-				logging.critical ('Failed extracting: %s' % h)
-				return -rsutil.common.EEXTRACT
-
-		return rsutil.common.SUCCESS
+			self.extraction_function (full_head, self.dir)
 
 	def find_extraction_heads (self):
 		"""Find all extraction heads associated with this set. This must be
diff --git a/PAR2Set/ExtractFirstBase.py b/PAR2Set/ExtractFirstBase.py
index 6b8a661..7acfc63 100644
--- a/PAR2Set/ExtractFirstBase.py
+++ b/PAR2Set/ExtractFirstBase.py
@@ -39,30 +39,29 @@ class ExtractFirstBase (PAR2Set.Base.Base):
 		"""Run the extraction, repair, and delete stages"""
 
 		# Extraction Stage
-		ret = self.runExtract ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Extraction stage failed for: %s' % self.p2file)
-			return -rsutil.common.EEXTRACT
+		try:
+			self.runExtract()
+		except (RuntimeError, OSError):
+			logging.critical('Extraction stage failed for: %s' % self.p2file)
+			raise
 
 		self.update_matches ()
 
 		# Repair Stage
-		ret = self.runVerifyAndRepair ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Repair stage failed for: %s' % self.p2file)
-			return -rsutil.common.ECHECK
+		try:
+			self.runVerifyAndRepair()
+		except (RuntimeError, OSError):
+			logging.critical('Repair stage failed for: %s' % self.p2file)
+			raise
 
 		self.update_matches ()
 
 		# Deletion Stage
-		ret = self.runDelete ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Deletion stage failed for: %s' % self.p2file)
-			return -rsutil.common.EDELETE
+		try:
+			self.runDelete()
+		except (RuntimeError, OSError):
+			logging.critical('Deletion stage failed for: %s' % self.p2file)
+			raise
 
 		logging.info ('Successfully completed: %s' % self.p2file)
-		return rsutil.common.SUCCESS
 
diff --git a/PAR2Set/Join.py b/PAR2Set/Join.py
index 61a1c42..1dd680a 100644
--- a/PAR2Set/Join.py
+++ b/PAR2Set/Join.py
@@ -75,28 +75,7 @@ class Join (PAR2Set.Base.Base):
 
 		   This is done using the par2repair command by default"""
 
-		PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair')
-
-		# assemble the command
-		# par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES]
-		command = "%s \"%s\" " % (PAR2_CMD, self.p2file)
-
-		for f in self.all_p2files:
-			if f != self.p2file:
-				command += "\"%s\" " % os.path.split (f)[1]
-
-		for f in self.find_joinfiles ():
-			command += "\"%s\" " % os.path.split (f)[1]
-
-		# run the command
-		ret = rsutil.common.run_command (command, self.dir)
-
-		# check the result
-		if ret != 0:
-			logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file)
-			return -rsutil.common.ECHECK
-
-		return rsutil.common.SUCCESS
+		rsutil.common.run_command(['par2repair'] + self.all_p2files + self.find_joinfiles(), self.dir)
 
 	def find_deleteable_files (self):
 		"""Find all files which are deletable by using the regular expression from the
@@ -123,20 +102,5 @@ class Join (PAR2Set.Base.Base):
 		   This command ignores the extraction if file and todir+file are the same
 		   file. This keeps things like mv working smoothly."""
 
-		NOEXTRACT_CMD = rsutil.common.config_get_value ('commands', 'noextract')
-
-		# Make sure that both files are not the same file. If they are, don't run at all.
-		# NOTE: os.path.samefile() doesn't exist on win32, so we can't use it.
-		if rsutil.common.full_abspath (file) == \
-				rsutil.common.full_abspath (os.path.join (todir, file)):
-			return rsutil.common.SUCCESS
-
-		cmd = NOEXTRACT_CMD % (file, todir)
-		ret = rsutil.common.run_command (cmd)
-
-		# Check error code
-		if ret != 0:
-			return -rsutil.common.EEXTRACT
-
-		return rsutil.common.SUCCESS
-
+		# The Join type doesn't need any extraction
+		pass
diff --git a/PAR2Set/NewRAR.py b/PAR2Set/NewRAR.py
index caefb29..7c8f25b 100644
--- a/PAR2Set/NewRAR.py
+++ b/PAR2Set/NewRAR.py
@@ -72,14 +72,5 @@ class NewRAR (PAR2Set.Base.Base):
 		assert os.path.isfile (file)
 		assert os.path.isdir (todir)
 
-		RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar')
-
-		cmd = '%s \"%s\"' % (RAR_CMD, file)
-		ret = rsutil.common.run_command (cmd, todir)
-
-		# Check error code
-		if ret != 0:
-			return -rsutil.common.EEXTRACT
-
-		return rsutil.common.SUCCESS
+		rsutil.common.run_command(['unrar', 'x', '-o+', file], todir)
 
diff --git a/PAR2Set/NoExtract.py b/PAR2Set/NoExtract.py
index 51ee235..9b4ea46 100644
--- a/PAR2Set/NoExtract.py
+++ b/PAR2Set/NoExtract.py
@@ -94,21 +94,20 @@ class NoExtract (PAR2Set.Base.Base):
 		"""Run the Repair and Deletion stages, omitting the Extraction stage"""
 
 		# Repair Stage
-		ret = self.runVerifyAndRepair ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Repair stage failed for: %s' % self.p2file)
-			return -rsutil.common.ECHECK
+		try:
+			self.runVerifyAndRepair()
+		except (RuntimeError, OSError):
+			logging.critical('Repair stage failed for: %s' % self.p2file)
+			raise
 
 		self.update_matches ()
 
 		# Deletion Stage
-		ret = self.runDelete ()
-
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('Deletion stage failed for: %s' % self.p2file)
-			return -rsutil.common.EDELETE
+		try:
+			self.runDelete()
+		except (RuntimeError, OSError):
+			logging.critical('Delete stage failed for: %s' % self.p2file)
+			raise
 
 		logging.info ('Successfully completed: %s' % self.p2file)
-		return rsutil.common.SUCCESS
 
diff --git a/PAR2Set/OldRAR.py b/PAR2Set/OldRAR.py
index b872ccf..b139570 100644
--- a/PAR2Set/OldRAR.py
+++ b/PAR2Set/OldRAR.py
@@ -74,14 +74,5 @@ class OldRAR (PAR2Set.Base.Base):
 		assert os.path.isfile (file)
 		assert os.path.isdir (todir)
 
-		RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar')
-
-		cmd = '%s \"%s\"' % (RAR_CMD, file)
-		ret = rsutil.common.run_command (cmd, todir)
-
-		# Check error code
-		if ret != 0:
-			return -rsutil.common.EEXTRACT
-
-		return rsutil.common.SUCCESS
+		rsutil.common.run_command(['unrar', 'x', '-o+', file], todir)
 
diff --git a/PAR2Set/ZIP.py b/PAR2Set/ZIP.py
index 36acb74..39a83ff 100644
--- a/PAR2Set/ZIP.py
+++ b/PAR2Set/ZIP.py
@@ -69,14 +69,5 @@ class ZIP (PAR2Set.Base.Base):
 		   file -- the file to extract
 		   todir -- the directory to extract into"""
 
-		ZIP_CMD = rsutil.common.config_get_value ('commands', 'unzip')
-
-		cmd = ZIP_CMD % (file, todir)
-		ret = rsutil.common.run_command (cmd)
-
-		# Check error code
-		if ret != 0:
-			return -rsutil.common.EEXTRACT
-
-		return rsutil.common.SUCCESS
+		rsutil.common.run_command(['unzip', file], todir)
 
diff --git a/RarslaveDetector.py b/RarslaveDetector.py
index 73a442b..dfb2768 100644
--- a/RarslaveDetector.py
+++ b/RarslaveDetector.py
@@ -82,6 +82,7 @@ class RarslaveDetector (object):
 		   the class is valid."""
 
 		detected = False
+		success = False
 
 		for (detector, classname) in self.TYPES:
 			if detector (self.name_matched_files, self.prot_matched_files):
@@ -93,15 +94,18 @@ class RarslaveDetector (object):
 				logging.debug ('Detected type: %s' % p2set)
 
 				# Try to have rarslave do it's thing
-				ret = p2set.runAll ()
+				try:
+					# Have rarslave do it's thing
+					p2set.runAll ()
 
-				# If something already worked, there is no need to continue,
-				# since we've already finished!
-				if ret == rsutil.common.SUCCESS:
+					# It succeeded, exit the loop early
+					success = True
 					break
-				else:
-					logging.error ('Detected type (%s) failed for: %s' % (p2set,
-							self.p2file))
+				except (OSError, RuntimeError):
+					logging.error('Detected type (%s) failed for: %s' % (p2set, self.p2file))
+				except:
+					logging.error('Unknown exception occurred')
+					raise
 
 		# Make sure we detected at least one valid type
 		if not detected:
@@ -109,13 +113,10 @@ class RarslaveDetector (object):
 			logging.debug ('The following information will help in writing a detector:')
 			logging.debug ('name_matches: %s' % self.name_matched_files)
 			logging.debug ('prot_matches: %s' % self.prot_matched_files)
-			return -rsutil.common.EDETECT
 
 		# Make sure that something worked
-		if ret != rsutil.common.SUCCESS:
-			logging.critical ('All types failed for: %s' % self.p2file)
-
-		return ret
+		if success == False:
+			logging.critical('All types failed for: %s' % self.p2file)
 
 def main ():
 	pass
diff --git a/rarslave.py b/rarslave.py
index 7c1b4c8..18bb540 100755
--- a/rarslave.py
+++ b/rarslave.py
@@ -122,17 +122,28 @@ def generate_all_parsets (dir):
 def check_required_progs():
 	"""Check if the required programs are installed"""
 
-	shell_not_found = 32512
 	needed = []
 
-	if rsutil.common.run_command ('par2repair --help > /dev/null 2>&1') == shell_not_found:
-		needed.append ('par2repair')
+	try:
+		rsutil.common.run_command(['par2repair', '--help'])
+	except OSError:
+		needed.append('par2repair')
+	except RuntimeError:
+		pass
 
-	if rsutil.common.run_command ('unrar --help > /dev/null 2>&1') == shell_not_found:
-		needed.append ('unrar')
+	try:
+		rsutil.common.run_command(['unrar', '--help'])
+	except OSError:
+		needed.append('unrar')
+	except RuntimeError:
+		pass
 
-	if rsutil.common.run_command ('unzip --help > /dev/null 2>&1') == shell_not_found:
-		needed.append ('unzip')
+	try:
+		rsutil.common.run_command(['unzip', '--help'])
+	except OSError:
+		needed.append('unzip')
+	except RuntimeError:
+		pass
 
 	if needed:
 		for n in needed:
@@ -169,6 +180,7 @@ def run_options (options):
 
 	if options.check_progs:
 		check_required_progs ()
+		sys.exit (0)
 
 	if options.write_def_config:
 		config.write_config (default=True)
@@ -264,14 +276,14 @@ def main ():
 			parsets = generate_all_parsets (dir)
 			for (p2dir, p2file) in parsets:
 				detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)
-				ret = detector.runMatchingTypes ()
+				detector.runMatchingTypes ()
 
 	# Non-recursive
 	else:
 		parsets = generate_all_parsets (options.work_dir)
 		for (p2dir, p2file) in parsets:
 			detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)
-			ret = detector.runMatchingTypes ()
+			detector.runMatchingTypes ()
 
 	# Print the results
 	if logger.size () > 0:
diff --git a/rsutil/common.py b/rsutil/common.py
index 66b7d02..370d7d6 100644
--- a/rsutil/common.py
+++ b/rsutil/common.py
@@ -29,13 +29,12 @@ __license__   = "GNU GPL v2 (or, at your option, any later version)"
 import os
 import re
 import logging
+import subprocess
+import exceptions
 
 import rsutil.globals
 import rsutil.par2parser
 
-# Global constants
-(SUCCESS, ECHECK, EEXTRACT, EDELETE, ECREATE, EDETECT, EPARSE) = range(7)
-
 def find_matches (regex, li, ignorecase=True):
 
 	if ignorecase:
@@ -66,14 +65,22 @@ def run_command (cmd, indir=None):
 	# Runs the specified command-line in the directory given (or, in the current directory
 	# if none is given). It returns the status code given by the application.
 
-	pwd = os.getcwd ()
-
-	if indir != None:
+	if indir == None:
+		indir = os.getcwd()
+	else:
 		assert os.path.isdir (indir) # MUST be a directory!
-		os.chdir (indir)
 
-	ret = os.system (cmd)
-	os.chdir (pwd)
+	print 'RUNNING COMMAND'
+	print 'Directory: %s' % indir
+	print 'Command: %s' % cmd[0]
+	for f in cmd[1:]:
+		print '-> %s' % f
+
+	ret = subprocess.Popen(cmd, cwd=indir).wait()
+
+	if ret != 0:
+		raise RuntimeError
+
 	return ret
 
 def full_abspath (p):
@@ -143,11 +150,7 @@ def list_eq (l1, l2):
 	if len(l1) != len(l2):
 		return False
 
-	for e in l1:
-		if e not in l2:
-			return False
-
-	return True
+	return set(l1) == set(l2)
 
 # Convience functions for the config
 
-- 
2.34.1