Clean up running commands under viewer_manifest (at least a little)

* do not redirect stderr to stdout
* catch errors generated in platform specific code and display them more nicely
* run_command no longer captures output (only used in one place;
  replaced that with direct use of subprocess)

2 files changed, 36 insertions, 35 deletions

diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py
index 0a39db2b21..e035b8fd7f 100755
--- a/indra/lib/python/indra/util/llmanifest.py
+++ b/indra/lib/python/indra/util/llmanifest.py
@@ -43,11 +43,15 @@ import subprocess
 
 class ManifestError(RuntimeError):
     """Use an exception more specific than generic Python RuntimeError"""
-    pass
+    def __init__(self, msg):
+        self.msg = msg
+        super(ManifestError, self).__init__(self.msg)
 
 class MissingError(ManifestError):
     """You specified a file that doesn't exist"""
-    pass
+    def __init__(self, msg):
+        self.msg = msg
+        super(MissingError, self).__init__(self.msg)
 
 def path_ancestors(path):
     drive, path = os.path.splitdrive(os.path.normpath(path))
@@ -309,8 +313,11 @@ def main():
                 continue
             if touch:
                 print 'Creating additional package for "', package_id, '" in ', args['dest']
-            wm = LLManifest.for_platform(args['platform'], args.get('arch'))(args)
-            wm.do(*args['actions'])
+            try:
+                wm = LLManifest.for_platform(args['platform'], args.get('arch'))(args)
+                wm.do(*args['actions'])
+            except ManifestError as err:
+                sys.exit(str(err))
             if touch:
                 print 'Created additional package ', wm.package_file, ' for ', package_id
                 faketouch = base_touch_prefix + '/' + package_id + '/' + base_touch_postfix
@@ -449,29 +456,17 @@ class LLManifest(object):
         return path
 
     def run_command(self, command):
-        """ Runs an external command, and returns the output.  Raises
-        an exception if the command returns a nonzero status code.  For
-        debugging/informational purposes, prints out the command's
-        output as it is received."""
+        """ 
+        Runs an external command.  
+        Raises ManifestError exception if the command returns a nonzero status.
+        """
         print "Running command:", command
         sys.stdout.flush()
-        child = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-                                 shell=True)
-        lines = []
-        while True:
-            lines.append(child.stdout.readline())
-            if lines[-1] == '':
-                break
-            else:
-                print lines[-1],
-        output = ''.join(lines)
-        child.stdout.close()
-        status = child.wait()
-        if status:
-            raise ManifestError(
-                "Command %s returned non-zero status (%s) \noutput:\n%s"
-                % (command, status, output) )
-        return output
+        try:
+            subprocess.check_call(command, shell=True)
+        except subprocess.CalledProcessError as err:
+            raise ManifestError( "Command %s returned non-zero status (%s)"
+                                % (command, err.returncode) )
 
     def created_path(self, path):
         """ Declare that you've created a path in order to
@@ -621,7 +616,7 @@ class LLManifest(object):
         if self.includes(src, dst):
             try:
                 os.unlink(dst)
-            except OSError, err:
+            except OSError as err:
                 if err.errno != errno.ENOENT:
                     raise
 
@@ -642,7 +637,7 @@ class LLManifest(object):
             dstname = os.path.join(dst, name)
             try:
                 self.ccopymumble(srcname, dstname)
-            except (IOError, os.error), why:
+            except (IOError, os.error) as why:
                 errors.append((srcname, dstname, why))
         if errors:
             raise ManifestError, errors
diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py
index b47faed8b7..900e9f7b1b 100755
--- a/indra/newview/viewer_manifest.py
+++ b/indra/newview/viewer_manifest.py
@@ -36,6 +36,8 @@ import re
 import tarfile
 import time
 import random
+import subprocess
+
 viewer_dir = os.path.dirname(__file__)
 # Add indra/lib/python to our path so we don't have to muck with PYTHONPATH.
 # Put it FIRST because some of our build hosts have an ancient install of
@@ -341,9 +343,9 @@ class WindowsManifest(ViewerManifest):
                     else:
                         test_assembly_binding(src, "Microsoft.VC80.CRT", "")
                     raise Exception("Unknown condition")
-                except NoManifestException, err:
+                except NoManifestException as err:
                     pass
-                except NoMatchingAssemblyException, err:
+                except NoMatchingAssemblyException as err:
                     pass
                     
                 self.ccopy(src,dst)
@@ -405,14 +407,14 @@ class WindowsManifest(ViewerManifest):
                 self.path('libaprutil-1.dll')
                 self.path('libapriconv-1.dll')
                 
-            except RuntimeError, err:
+            except RuntimeError as err:
                 print err.message
                 print "Skipping llcommon.dll (assuming llcommon was linked statically)"
 
             # Mesh 3rd party libs needed for auto LOD and collada reading
             try:
                 self.path("glod.dll")
-            except RuntimeError, err:
+            except RuntimeError as err:
                 print err.message
                 print "Skipping GLOD library (assumming linked statically)"
 
@@ -728,7 +730,7 @@ class WindowsManifest(ViewerManifest):
         for attempt in xrange(nsis_attempts):
             try:
                 self.run_command('"' + NSIS_path + '" /V2 ' + self.dst_path_of(tempfile))
-            except ManifestError, err:
+            except ManifestError as err:
                 if attempt+1 < nsis_attempts:
                     print >> sys.stderr, "nsis failed, waiting %d seconds before retrying" % nsis_retry_wait
                     time.sleep(nsis_retry_wait)
@@ -1106,7 +1108,11 @@ class DarwinManifest(ViewerManifest):
                 'vol':volname})
 
         # mount the image and get the name of the mount point and device node
-        hdi_output = self.run_command('hdiutil attach -private %r' % sparsename)
+        try:
+            hdi_output = subprocess.check_output(['hdiutil', 'attach', '-private', sparsename])
+        except subprocess.CalledProcessError as err:
+            sys.exit("failed to mount image at '%s'" % sparsename)
+            
         try:
             devfile = re.search("/dev/disk([0-9]+)[^s]", hdi_output).group(0).strip()
             volpath = re.search('HFS\s+(.+)', hdi_output).group(1).strip()
@@ -1220,7 +1226,7 @@ class DarwinManifest(ViewerManifest):
                                    'bundle': app_in_dmg
                                    })
                             signed=True # if no exception was raised, the codesign worked
-                        except ManifestError, err:
+                        except ManifestError as err:
                             if sign_attempts:
                                 print >> sys.stderr, "codesign failed, waiting %d seconds before retrying" % sign_retry_wait
                                 time.sleep(sign_retry_wait)
@@ -1483,7 +1489,7 @@ def symlinkf(src, dst):
     """
     try:
         os.symlink(src, dst)
-    except OSError, err:
+    except OSError as err:
         if err.errno != errno.EEXIST:
             raise
         # We could just blithely attempt to remove and recreate the target