Update ADD handling in Dockerfiles and test bundle creation

blimp/tools/create-bundle.py

Index: blimp/tools/create-bundle.py
diff --git a/blimp/tools/create-bundle.py b/blimp/tools/create-bundle.py
index 5aab7fcb17938874eb42d0c8559aa68fbe074854..f6bc53deba40711189b6ec387fa9b0de7d7b9a54 100755
--- a/blimp/tools/create-bundle.py
+++ b/blimp/tools/create-bundle.py
@@ -15,19 +15,23 @@ import os
 import subprocess
 import sys
 
-def ReadDependencies(manifest):
-  """Read the manifest and return the list of dependencies.
+def ReadDependencies(manifest, relative_path):
+  """Read the manifest. Takes each dependency found within prepends it with

[maniscalco, 2016/07/19 21:52:36]

nit: Need a comma between "within" and "prepends".

[Jess, 2016/07/19 22:32:41]

Done.

+  relative path (optional), normalizes the resulting combined path, and adds
+  it to the returned dependency list.
   :raises IOError: if the manifest could not be read.
   """
-  deps = []
+  dependency_list = []
   with open(manifest) as f:
     for line in f.readlines():
       # Strip comments.
-      dep = line.partition('#')[0].strip()
+      dependency = line.partition('#')[0].strip()
       # Ignore empty strings.
-      if dep:
-        deps.append(dep)
-  return deps
+      if dependency:
+        if relative_path != '':

[maniscalco, 2016/07/19 21:52:36]

I'm no python expert, but I believe the idiomatic way to check for empty string
is:

if not relative_path:

(similar to how we do "if dependency" above to check for non-empty string)

[Jess, 2016/07/19 22:32:41]

Acknowledged. Comment below overrides.

+          dependency = os.path.normpath(os.path.join(relative_path, dependency))
+        dependency_list.append(dependency)
+  return dependency_list
 
 def main():
   parser = argparse.ArgumentParser(description=__doc__)
@@ -50,9 +54,19 @@ def main():
                       help=('name and path of bundle to create'),
                       required=True,
                       metavar='FILE')
+  parser.add_argument('--tar-contents-rooted-in',
+                      help=('optional path prefix to use inside the resulting '
+                            'tar file.'),

[maniscalco, 2016/07/19 21:52:36]

nit: Remove the period at the end of the help text to be consistent with the
other help text above.

[Jess, 2016/07/19 22:32:41]

Done.

+                      required=False,
+                      metavar='DIR')
   args = parser.parse_args()
 
-  deps = ReadDependencies(args.manifest)
+  dependencies_path = args.build_dir
+  if args.tar_contents_rooted_in:
+    dependencies_path = args.tar_contents_rooted_in
+  relative_path = os.path.relpath(args.build_dir, dependencies_path)

[maniscalco, 2016/07/19 21:52:36]

So relative_path is always non-empty, right?  If that's the case, can you remove
the check from ReadDependencies above?  In other words, the second argument to
ReadDependencies is no longer optional, right?

[Jess, 2016/07/19 22:32:41]

Ah, even cleaner.

+
+  dependency_list = ReadDependencies(args.manifest, relative_path)
 
   try:
     env = os.environ.copy()
@@ -65,7 +79,7 @@ def main():
         # use as part of a "docker build". That is group readable with
         # executable files also being group executable.
         "--mode=g+rX",
-        "-C", args.build_dir] + deps
+        "-C", dependencies_path] + dependency_list
     for f in args.filelist:
       dirname, basename = os.path.split(f)
       subprocess_args.extend(["-C", dirname, basename])