From 93ea0d7026dcb7a9aec3bd4f8615eb62da159a02 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 29 Jun 2018 17:08:48 -0400 Subject: MAINT-8822: Revamp the LLManifest.prefix() calling convention. The way prefix("path_fragment") or prefix(src="path_fragment") has always worked is that unless you explicitly specify dst="", it adds "path_fragment" to the source AND dest prefix stacks! The most recent refactoring of viewer_manifest.py failed to copy CEF because it involved prefix(src="../some lengthy path fragment") -- forgetting to specify dst="" -- which added "../some lengthy path fragment" to the dest prefix stack -- which put it outside the viewer install staging area altogether. Having been bitten too many times by forgetting to add prefix(dst=""), we remove the necessity. The prefix() src=, build= and dst= prefix stacks are now completely independent. Add src_dst= keyword argument for when you DO want to add the same path fragment to both the source and dest prefix stacks. ("Explicit is better than implicit.") Change all existing calls accordingly. Now that the build prefix stack no longer tracks the src prefix stack, we were failing to pick up some things from the build area because NOBODY ever used build=, relying entirely on src= to point both to stuff in the source tree and stuff in the build tree. Try to use build= appropriately. If that proves too confusing, we might eliminate the separate build and artwork (!) prefix stacks entirely, requiring callers to reset the src stack explicitly when switching back and forth. --- indra/lib/python/indra/util/llmanifest.py | 32 +++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 598261e3d7..4c3d3fc217 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -375,7 +375,7 @@ class LLManifest(object): in the file list by path().""" self.excludes.append(glob) - def prefix(self, src='', build=None, dst=None): + def prefix(self, src='', build='', dst='', src_dst=None): """ Usage: @@ -385,8 +385,21 @@ class LLManifest(object): 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. + prefixes are independent; if omitted (or passed as the empty string), + the prefix has no effect. Thus: + + with self.prefix(src='foo'): + # no effect on dst + + with self.prefix(dst='bar'): + # no effect on src + + If you want to set both at once, use src_dst: + + with self.prefix(src_dst='subdir'): + # same as self.prefix(src='subdir', dst='subdir') + # Passing src_dst makes any src or dst argument in the same + # parameter list irrelevant. Also supports the older (pre-Python-2.5) syntax: @@ -400,10 +413,9 @@ class LLManifest(object): 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: - build = src + if src_dst is not None: + src = src_dst + dst = src_dst self.src_prefix.append(src) self.artwork_prefix.append(src) self.build_prefix.append(build) @@ -609,7 +621,6 @@ class LLManifest(object): def process_file(self, src, dst): if self.includes(src, dst): -# print src, "=>", dst for action in self.actions: methodname = action + "_action" method = getattr(self, methodname, None) @@ -677,7 +688,11 @@ class LLManifest(object): # Don't recopy file if it's up-to-date. # If we seem to be not not overwriting files that have been # updated, set the last arg to False, but it will take longer. +## reldst = (dst[len(self.dst_prefix[0]):] +## if dst.startswith(self.dst_prefix[0]) +## else dst).lstrip(r'\/') if os.path.exists(dst) and filecmp.cmp(src, dst, True): +## print "{} (skipping, {} exists)".format(src, reldst) return # only copy if it's not excluded if self.includes(src, dst): @@ -687,6 +702,7 @@ class LLManifest(object): if err.errno != errno.ENOENT: raise +## print "{} => {}".format(src, reldst) shutil.copy2(src, dst) def ccopytree(self, src, dst): -- cgit v1.2.3 From bbadee86c104609cc47be20297470bd7abab8008 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 30 Jun 2018 10:01:11 -0400 Subject: MAINT-8822: Give LLManifest.prefix() (suppressed) debugging output. --- indra/lib/python/indra/util/llmanifest.py | 19 +++++++++++++++---- 1 file changed, 15 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 4c3d3fc217..974ac18e34 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -421,25 +421,34 @@ class LLManifest(object): self.build_prefix.append(build) self.dst_prefix.append(dst) +## self.display_stacks() + # 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) + def display_stacks(self): + width = 1 + max(len(stack) for stack in self.PrefixManager.stacks) + for stack in self.PrefixManager.stacks: + print "{} {}".format((stack + ':').ljust(width), + os.path.join(*getattr(self, stack))) + class PrefixManager(object): + # stack attributes we manage in this LLManifest (sub)class + # instance + stacks = ("src_prefix", "artwork_prefix", "build_prefix", "dst_prefix") + 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 } + for stack in self.stacks } def __nonzero__(self): # If the caller wrote: @@ -472,6 +481,8 @@ class LLManifest(object): # truncate that list back to 'prevlen' del getattr(self.manifest, stack)[prevlen:] +## self.manifest.display_stacks() + def end_prefix(self, descr=None): """Pops a prefix off the stack. If given an argument, checks the argument against the top of the stack. If the argument -- cgit v1.2.3 From 1b2f52257aa84d43cdeec31f1b1424e7ad44fa7f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 15 Aug 2018 16:38:09 -0400 Subject: DRTVWR-447: Streamline some of the logic around extra packages. --- indra/lib/python/indra/util/llmanifest.py | 81 +++++++------------------------ 1 file changed, 17 insertions(+), 64 deletions(-) (limited to 'indra/lib') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 974ac18e34..c24e25ba89 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -232,16 +232,12 @@ def main(): print "Option:", opt, "=", args[opt] # pass in sourceid as an argument now instead of an environment variable - try: - args['sourceid'] = os.environ["sourceid"] - except KeyError: - args['sourceid'] = "" + args['sourceid'] = os.environ.get("sourceid", "") # Build base package. touch = args.get('touch') if touch: print 'Creating base package' - args['package_id'] = "" # base package has no package ID wm = LLManifest.for_platform(args['platform'], args.get('arch'))(args) wm.do(*args['actions']) # Store package file for later if making touched file. @@ -251,64 +247,25 @@ def main(): base_package_file = "" + wm.package_file # handle multiple packages if set - try: - additional_packages = os.environ["additional_packages"] - except KeyError: - additional_packages = "" + # ''.split() produces empty list + additional_packages = os.environ.get("additional_packages", "").split() if additional_packages: # Determine destination prefix / suffix for additional packages. - base_dest_postfix = args['dest'] - base_dest_prefix = "" base_dest_parts = args['dest'].split(os.sep) - if len(base_dest_parts) > 1: - base_dest_postfix = base_dest_parts[len(base_dest_parts) - 1] - base_dest_prefix = base_dest_parts[0] - i = 1 - while i < len(base_dest_parts) - 1: - base_dest_prefix = base_dest_prefix + os.sep + base_dest_parts[i] - i = i + 1 + base_dest_parts.insert(-1, "{}") + base_dest_template = os.sep.join(base_dest_parts) # Determine touched prefix / suffix for additional packages. - base_touch_postfix = "" - base_touch_prefix = "" if touch: - base_touch_postfix = touch - base_touch_parts = touch.split('/') + base_touch_parts = touch.split(os.sep) if "arwin" in args['platform']: - if len(base_touch_parts) > 1: - base_touch_postfix = base_touch_parts[len(base_touch_parts) - 1] - base_touch_prefix = base_touch_parts[0] - i = 1 - while i < len(base_touch_parts) - 1: - base_touch_prefix = base_touch_prefix + '/' + base_touch_parts[i] - i = i + 1 + base_touch_parts.insert(-1, "{}") else: - if len(base_touch_parts) > 2: - base_touch_postfix = base_touch_parts[len(base_touch_parts) - 2] + '/' + base_touch_parts[len(base_touch_parts) - 1] - base_touch_prefix = base_touch_parts[0] - i = 1 - while i < len(base_touch_parts) - 2: - base_touch_prefix = base_touch_prefix + '/' + base_touch_parts[i] - i = i + 1 - # Store base channel name. - base_channel_name = args['channel'] - # Build each additional package. - package_id_list = additional_packages.split(" ") - args['channel'] = base_channel_name - for package_id in package_id_list: - try: - if package_id + "_viewer_channel_suffix" in os.environ: - args['channel_suffix'] = os.environ[package_id + "_viewer_channel_suffix"] - else: - args['channel_suffix'] = None - if package_id + "_sourceid" in os.environ: - args['sourceid'] = os.environ[package_id + "_sourceid"] - else: - args['sourceid'] = None - args['dest'] = base_dest_prefix + os.sep + package_id + os.sep + base_dest_postfix - except KeyError: - sys.stderr.write("Failed to create package for package_id: %s" % package_id) - sys.stderr.flush() - continue + base_touch_parts.insert(-2, "{}") + base_touch_template = os.sep.join(base_touch_parts) + for package_id in additional_packages: + args['channel_suffix'] = os.environ.get(package_id + "_viewer_channel_suffix") + args['sourceid'] = os.environ.get(package_id + "_sourceid") + args['dest'] = base_dest_template.format(package_id) if touch: print 'Creating additional package for "', package_id, '" in ', args['dest'] try: @@ -318,18 +275,14 @@ def main(): 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 - fp = open(faketouch, 'w') - fp.write('set package_file=%s\n' % wm.package_file) - fp.close() + with open(base_touch_template.format(package_id), 'w') as fp: + fp.write('set package_file=%s\n' % wm.package_file) # Write out the package file in this format, so that it can easily be called # and used in a .bat file - yeah, it sucks, but this is the simplest... - touch = args.get('touch') if touch: - fp = open(touch, 'w') - fp.write('set package_file=%s\n' % base_package_file) - fp.close() + with open(touch, 'w') as fp: + fp.write('set package_file=%s\n' % base_package_file) print 'touched', touch return 0 -- cgit v1.2.3 From 52fe35737024abc5712bda770801fdeb703881fc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 17 Aug 2018 14:39:32 -0400 Subject: DRTVWR-447: Use os.path.split(path) instead of path.split(os.sep). On Windows, where 'path' might be separated either with '/' or '\', the latter breaks unless all path separators are in fact the os.sep character '\'. While it would be possible to code something fancy with os.sep and os.altsep, testing the latter for None, much simpler to let os.path.split() handle it. --- indra/lib/python/indra/util/llmanifest.py | 12 ++++++++---- 1 file changed, 8 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 c24e25ba89..611f72269e 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -251,17 +251,21 @@ def main(): additional_packages = os.environ.get("additional_packages", "").split() if additional_packages: # Determine destination prefix / suffix for additional packages. - base_dest_parts = args['dest'].split(os.sep) + base_dest_parts = list(os.path.split(args['dest'])) base_dest_parts.insert(-1, "{}") - base_dest_template = os.sep.join(base_dest_parts) + base_dest_template = os.path.join(*base_dest_parts) # Determine touched prefix / suffix for additional packages. if touch: - base_touch_parts = touch.split(os.sep) + base_touch_parts = list(os.path.split(touch)) + # Because of the special insert() logic below, we don't just want + # [dirpath, basename]; we want [dirpath, directory, basename]. + # Further split the dirpath and replace it in the list. + base_touch_parts[0:1] = os.path.split(base_touch_parts[0]) if "arwin" in args['platform']: base_touch_parts.insert(-1, "{}") else: base_touch_parts.insert(-2, "{}") - base_touch_template = os.sep.join(base_touch_parts) + base_touch_template = os.path.join(*base_touch_parts) for package_id in additional_packages: args['channel_suffix'] = os.environ.get(package_id + "_viewer_channel_suffix") args['sourceid'] = os.environ.get(package_id + "_sourceid") -- cgit v1.2.3