Merge lp:~steve-mcintyre/linaro-chromiumos/error_checking into lp:linaro-chromiumos

[Steve McIntyre]

Should have pushed this ages ago...

More error handling; make sure that we abort properly on errors.

[Loïc Minier (lool)]

Remove the "print" before the exception throwing; you can either deal with the error in higher level logic, or print the exception message from the main script, or just let the exception go out to the default handler which will include the error message.

Usually in scripts people prefer the subprocess module rather than os.system() which is very limited; subprocess.Popen() is very flexible allowing you to do pipes or capture stdout / stderr for instance, and if you only need to run a command and throw an exception if it fails it provides a wrapper around Popen called check_call() which does that. There are other utilities to get command output as well.

I suggest you use check_call instead of os.system; this will allow closing stderr or stdout in the future when we deem sensible, or piping them to a log file etc.

Unmerged revisions

18. By Steve McIntyre on 2011-03-30

Better error-handling

Check for errors from os.system() calls and throw exceptions on
failure.
Throw exceptions rather than return errors from functions.

=== modified file 'remote/run_build'
--- remote/run_build	2011-03-23 15:48:31 +0000
+++ remote/run_build	2011-03-30 15:41:27 +0000
@@ -13,7 +13,8 @@
 def run(command, as_root=False):
     if as_root:
         command = "sudo -- %s" % command
-    os.system(command)
+    if os.system(command) != 0:
+        raise Exception("command failed")
 
 def main(argv):
     options = {}

=== modified file 'remote/setup_build'
--- remote/setup_build	2011-03-24 17:52:47 +0000
+++ remote/setup_build	2011-03-30 15:41:27 +0000
@@ -63,7 +63,8 @@
 def run(command, as_root=False):
     if as_root:
         command = "sudo -- %s" % command
-    os.system(command)
+    if os.system(command) != 0:
+        raise Exception("command failed")
 
 def logged_run(log_file, command):
     # XXX needs to escape command properly
@@ -81,7 +82,7 @@
 def cmd_create_tree(options, tree_dir):
     if os.path.exists(tree_dir):
         print 'cmd_create_tree: tree %s already exists. Abort.' % tree_dir
-        return errno.EEXIST
+        raise Exception("Tree directory %s already exists" % tree_dir)
 
     os.makedirs(tree_dir, 0755)
     run('apt-get update', as_root=True)
@@ -92,7 +93,7 @@
 def cmd_get_source(options, tree_dir):
     if not os.path.exists(tree_dir):
         print 'cmd_get_source: tree %s does not exist. Abort.' % tree_dir
-        return errno.ENOENT
+        raise Exception("Tree directory %s does not exist" % tree_dir)
 
     basedir = os.path.dirname(os.path.realpath(sys.argv[0]))
 
@@ -114,7 +115,7 @@
 def cmd_create_chroot(options, tree_dir):
     if not os.path.exists(tree_dir):
         print 'cmd_create_chroot: tree %s does not exist. Abort.' % tree_dir
-        return errno.ENOENT
+        raise Exception("Tree directory %s does not exist" % tree_dir)
 
     create_logdir(tree_dir)
     os.chdir(tree_dir)
@@ -126,12 +127,12 @@
 def cmd_build(options, tree_dir):
     if not os.path.exists(tree_dir):
         print 'cmd_build: tree %s does not exist. Abort.' % tree_dir
-        return errno.ENOENT
+        raise Exception("Tree directory %s does not exist" % tree_dir)
 
     chroot_path = os.path.join(tree_dir, 'chroot')
     if not os.path.exists(chroot_path):
         print 'cmd_build: chroot %s does not exist. Abort.' % chroot_path
-        return errno.ENOENT
+        raise Exception("Chroot %s does not exist" % chroot_path)
 
     create_logdir(tree_dir)
     os.chdir(tree_dir)