From e044902aaf1aa963d41a30a249b564e1b8910de7 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 24 May 2017 09:41:44 -0400 Subject: SL-702: refactor to make the viewer-manager easier for TPVs to integrate --- indra/lib/python/indra/util/llmanifest.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 62bd09471a..0a39db2b21 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -143,6 +143,9 @@ ARGUMENTS=[ default=None), dict(name='versionfile', description="""The name of a file containing the full version number."""), + dict(name='bundleid', + description="""The Mac OS X Bundle identifier.""", + default="com.secondlife.indra.viewer"), dict(name='signature', description="""This specifies an identity to sign the viewer with, if any. If no value is supplied, the default signature will be used, if any. Currently -- cgit v1.2.3 From 6ac682619275580eb42f2aaa93b6a251df6239b8 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Thu, 21 Sep 2017 15:50:29 -0400 Subject: 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) --- indra/lib/python/indra/util/llmanifest.py | 49 ++++++++++++++----------------- 1 file changed, 22 insertions(+), 27 deletions(-) (limited to 'indra/lib') 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 -- cgit v1.2.3 From 38d9454cbf30f470a723d912be2080f63ae3fa47 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Thu, 21 Sep 2017 19:24:04 -0400 Subject: remove redundant exception constructor code, and generalize exception catching for platform specific code --- indra/lib/python/indra/util/llmanifest.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index e035b8fd7f..d4e61aedd1 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -49,9 +49,7 @@ class ManifestError(RuntimeError): class MissingError(ManifestError): """You specified a file that doesn't exist""" - def __init__(self, msg): - self.msg = msg - super(MissingError, self).__init__(self.msg) + pass def path_ancestors(path): drive, path = os.path.splitdrive(os.path.normpath(path)) @@ -316,7 +314,7 @@ def main(): try: wm = LLManifest.for_platform(args['platform'], args.get('arch'))(args) wm.do(*args['actions']) - except ManifestError as err: + except Exception as err: sys.exit(str(err)) if touch: print 'Created additional package ', wm.package_file, ' for ', package_id -- cgit v1.2.3 From d9d6df313cbced45c9fd57212e51ac417fc896d6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 9 Oct 2017 16:28:35 -0400 Subject: MAINT-7831: Allow LLManifest.prefix() to be a context manager. LLManifest.prefix() dates back to before Python had a 'with' statement or the notion of a context manager. That's why every prefix() call requires a corresponding end_prefix() call. Existing usage is of the form: if self.prefix(...some args...): self.path(...) ... self.end_prefix() The use of an 'if' statement is solely to allow the coder to indent the statements between the self.prefix() call and the corresponding call to self.end_prefix() -- there is no intention to make that code conditional. self.prefix() unconditionally returned True to facilitate that usage. But now that we have the 'with' statement, this all feels a little silly. Make prefix() return an instance of a context-manager class so that it's reasonable to say instead: with self.prefix(...some args...): self.path(...) ... and have the Right Things happen simply by leaving the 'with' block. The only tricky part is code to preserve compatibility with old-style usage: * The context manager has a __nonzero__() method so that if it's tested in an 'if' statement, it can unconditionally return True. * On leaving the 'with' block, rather than simply popping the top of each prefix stack, the context manager restores its length to the same length it had before that prefix() call. This allows for (erroneous but hardly unlikely) usage of the form: with self.prefix(...some args...): self.path(...) ... self.end_prefix() Restoring the previous length makes the context manager insensitive to whether or not end_prefix() has popped the most recent prefix() entries. --- indra/lib/python/indra/util/llmanifest.py | 82 ++++++++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 7 deletions(-) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index d4e61aedd1..7050ce43b7 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -376,12 +376,30 @@ class LLManifest(object): self.excludes.append(glob) def prefix(self, src='', build=None, dst=None): - """ Pushes a prefix onto the stack. Until end_prefix is - called, all relevant method calls (esp. to path()) will prefix - paths with the entire prefix stack. Source and destination - prefixes can be different, though if only one is provided they - are both equal. To specify a no-op, use an empty string, not - None.""" + """ + Usage: + + with self.prefix(...args as described...): + self.path(...) + + For the duration of the 'with' block, pushes a prefix onto the stack. + Within that block, all relevant method calls (esp. to path()) will + prefix paths with the entire prefix stack. Source and destination + prefixes can be different, though if only one is provided they are + both equal. To specify a no-op, use an empty string, not None. + + Also supports the older (pre-Python-2.5) syntax: + + if self.prefix(...args as described...): + self.path(...) + self.end_prefix(...) + + Before the arrival of the 'with' statement, one was required to code + self.prefix() and self.end_prefix() in matching pairs to push and to + pop the prefix stacks, respectively. The older prefix() method + returned True specifically so that the caller could indent the + relevant block of code with 'if', just for aesthetic purposes. + """ if dst is None: dst = src if build is None: @@ -390,7 +408,57 @@ class LLManifest(object): self.artwork_prefix.append(src) self.build_prefix.append(build) self.dst_prefix.append(dst) - return True # so that you can wrap it in an if to get indentation + + # The above code is unchanged from the original implementation. What's + # new is the return value. We're going to return an instance of + # PrefixManager that binds this LLManifest instance and Does The Right + # Thing on exit. + return self.PrefixManager(self) + + class PrefixManager(object): + def __init__(self, manifest): + self.manifest = manifest + # stack attributes we manage in this LLManifest (sub)class + # instance + stacks = ("src_prefix", "artwork_prefix", "build_prefix", "dst_prefix") + # If the caller wrote: + # with self.prefix(...): + # as intended, then bind the state of each prefix stack as it was + # just BEFORE the call to prefix(). Since prefix() appended an + # entry to each prefix stack, capture len()-1. + self.prevlen = { stack: len(getattr(self.manifest, stack)) - 1 + for stack in stacks } + + def __nonzero__(self): + # If the caller wrote: + # if self.prefix(...): + # then a value of this class had better evaluate as 'True'. + return True + + def __enter__(self): + # nobody uses 'with self.prefix(...) as variable:' + return None + + def __exit__(self, type, value, traceback): + # First, if the 'with' block raised an exception, just propagate. + # Do NOT swallow it. + if type is not None: + return False + + # Okay, 'with' block completed successfully. Restore previous + # state of each of the prefix stacks in self.stacks. + # Note that we do NOT simply call pop() on them as end_prefix() + # does. This is to cope with the possibility that the coder + # changed 'if self.prefix(...):' to 'with self.prefix(...):' yet + # forgot to remove the self.end_prefix(...) call at the bottom of + # the block. In that case, calling pop() again would be Bad! But + # if we restore the length of each stack to what it was before the + # current prefix() block, it doesn't matter whether end_prefix() + # was called or not. + for stack, prevlen in self.prevlen.items(): + # find the attribute in 'self.manifest' named by 'stack', and + # truncate that list back to 'prevlen' + del getattr(self.manifest, stack)[prevlen:] def end_prefix(self, descr=None): """Pops a prefix off the stack. If given an argument, checks -- cgit v1.2.3