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 From 488d165895db3c2ae2aebbb3b727e95b8a3de6bf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 4 Dec 2017 17:15:47 -0500 Subject: MAINT-7751: Rework DarwinManifest to produce new app bundle structure. Specifically, Second Life.app is now mostly just a wrapper. Its Contents/ Resources contains nested Launcher.app (the VMP) and Viewer.app (the viewer itself). Most of what used to be in the top-level Second Life.app has been relocated to the embedded Viewer.app. VMP stuff has of course been extracted to Launcher.app. The top-level Second Life.app executable is now a tiny script that runs Launcher.app. This structure permits different icons and different Dock flyover text for the launcher and the viewer, hopefully ameliorating a certain amount of user confusion about the dual icons. This requires a corresponding VMP change: on macOS, the VMP must now find both its resources and the viewer executable by walking up from Launcher.app and down again into its sibling Viewer.app. Since Dock flyover text is determined by the embedded app names, allow Product to change these at will. That means we should be able to tweak exactly one variable assignment to change either of those embedded app names, without having to chase down other references scattered throughout the source repo. For that reason, create top-level trampoline SL_Launcher script dynamically: it must reference the launcher app by name. That means we must also perform (the equivalent of) chmod +x on that generated script. The one mystery surrounding this restructuring is that without a top-level Frameworks symlink pointing to the embedded Viewer.app's Frameworks directory (where CEF lives), CEF refuses to start: no splash screen, no MoP. Perhaps we can fix that someday. Use Python's bundled plistlib to generate Info.plist files for the embedded applications. Reorganize stray code stanzas to try to help the structure of the code more or less resemble the structure of the desired result. Add ViewerManifest.relpath() method to determine the relative path from a specified base to the target path. If base omitted, assumes get_dst_prefix() -- handy for creating symlinks. Determining exactly the right number of os.pardir instances to concatenate into the relative pathname for a symlink (or an install_name_tool stamp) was tedious, fragile and unobvious, difficult to desk-check. Using relpath() should make all that more robust. Migrate symlinkf() from free function to ViewerManifest method, refactoring into _symlinkf_prep_dst() and _symlinkf(), adding relsymlinkf(). This lets us add convenience features such as prepending get_dst_prefix() to the dest (the place where we want to create the symlink), defaulting dest to the basename of target and ensuring that the parent of that dest already exists -- as with LLManifest.path(). Moreover, since it makes no sense whatsoever to create an absolute symlink to some path on the build machine, relsymlinkf() creates every symlink relative to dirname(dest). That, in turn, lets us eliminate a certain amount of boilerplate around existing calls. (Also, since we now ensure the parent directory exists, scrap the logic to diagnose "nonexistent parent directory.") Make llmanifest.LLManifest.run_command() not pass shell=True to subprocess, thereby permitting (requiring) the list form rather than the string form. Change all existing calls to list form. This makes calls more readable, for two reasons. First, many of the arguments are taken from script variables; these can simply be dropped into the list instead of indirecting through string interpolation. Second, it eliminates the need to manually escape individual arguments, since subprocess promises to honor the distinction between list elements. Also fix LLManifest.put_in_file() to ensure the containing directory exists. Consolidate some viewer_manifest.py redundancy, e.g. copying the same set of ten DLLs from either of two directories depending on Release vs. Debug. --- indra/lib/python/indra/util/llmanifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 7050ce43b7..598261e3d7 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -529,7 +529,7 @@ class LLManifest(object): print "Running command:", command sys.stdout.flush() try: - subprocess.check_call(command, shell=True) + subprocess.check_call(command) except subprocess.CalledProcessError as err: raise ManifestError( "Command %s returned non-zero status (%s)" % (command, err.returncode) ) @@ -545,6 +545,7 @@ class LLManifest(object): def put_in_file(self, contents, dst, src=None): # write contents as dst dst_path = self.dst_path_of(dst) + self.cmakedirs(os.path.dirname(dst_path)) f = open(dst_path, "wb") try: f.write(contents) -- cgit v1.2.3