Fix pkg_tar to not add the ./ to the prefix of every member.

pkg/private/archive.py

@@ -210,6 +210,8 @@ def add_root_prefix(self, path: str) -> str:
       modified path.
     """
     path = path.replace(os.path.sep, '/').rstrip('/')
+    while path.startswith('./'):
+      path = path[2:]
     if not self.root_directory or path.startswith('/'):
       return path
     if (path + '/').startswith(self.root_directory):
@@ -393,24 +395,21 @@ def add_tar(self,
           tarinfo.gname = ''
 
         name = self.add_root_prefix(tarinfo.name)
+        tarinfo.name = name
+        self.add_parents(
+            name,
+            mtime=tarinfo.mtime,
+            mode=0o755,
+            uid=tarinfo.uid,
+            gid=tarinfo.gid,
+            uname=tarinfo.uname,
+            gname=tarinfo.gname)
+
         if root is not None:
-          if name.startswith('.'):
-            name = '.' + root + name.lstrip('.')
-            # Add root dir with same permissions if missing. Note that
-            # add_file deduplicates directories and is safe to call here.
-            self.add_file('.' + root,
-                          tarfile.DIRTYPE,
-                          uid=tarinfo.uid,
-                          gid=tarinfo.gid,
-                          uname=tarinfo.uname,
-                          gname=tarinfo.gname,
-                          mtime=tarinfo.mtime,
-                          mode=0o755)
           # Relocate internal hardlinks as well to avoid breaking them.
           link = tarinfo.linkname
           if link.startswith('.') and tarinfo.type == tarfile.LNKTYPE:
             tarinfo.linkname = '.' + root + link.lstrip('.')
-        tarinfo.name = name
 
         # Remove path pax header to ensure that the proposed name is going
         # to be used. Without this, files with long names will not be

pkg/private/tar/build_tar.py

@@ -25,33 +25,56 @@
 from pkg.private import manifest
 
 
+def normpath(path):
+  """Normalize a path to the format we need it.
+
+  os.path.normpath changes / to \ on windows, but tarfile needs / style paths.
+
+  Args:
+    path: (str) path to normalize.
+  """
+  return os.path.normpath(path).replace(os.path.sep, '/')
+
+
 class TarFile(object):
   """A class to generates a TAR file."""
 
   class DebError(Exception):
     pass
 
-  def __init__(self, output, directory, compression, compressor, root_directory,
-               default_mtime):
-    self.directory = directory
+  def __init__(self, output, directory, compression, compressor, default_mtime):
+    # Directory prefix on all output paths
+    d = directory.strip('/')
+    self.directory = (d + '/') if d else None
     self.output = output
     self.compression = compression
     self.compressor = compressor
-    self.root_directory = root_directory
     self.default_mtime = default_mtime
 
   def __enter__(self):
     self.tarfile = archive.TarFileWriter(
         self.output,
         self.compression,
         self.compressor,
-        self.root_directory,
+        root_directory = self.directory,
         default_mtime=self.default_mtime)
     return self
 
   def __exit__(self, t, v, traceback):
     self.tarfile.close()
 
+  def normalize_path(self, path: str) -> str:
+    dest = normpath(path)
+    # paths should not have a leading ./
+    if dest.startswith('./'):
+      dest = dest[2:]
+    # No path should ever come in with slashs on either end, but protect
+    # against that anyway.
+    dest = dest.strip('/')
+    if self.directory:
+      dest = self.directory + dest
+    return dest
+
   def add_file(self, f, destfile, mode=None, ids=None, names=None):
     """Add a file to the tar file.
 
@@ -64,18 +87,14 @@ def add_file(self, f, destfile, mode=None, ids=None, names=None):
        names: (username, groupname) for the file to set ownership. `f` will be
          copied to `self.directory/destfile` in the layer.
     """
-    dest = destfile.lstrip('/')  # Remove leading slashes
-    if self.directory and self.directory != '/':
-      dest = self.directory.lstrip('/') + '/' + dest
+    dest = self.normalize_path(destfile)
     # If mode is unspecified, derive the mode from the file's mode.
     if mode is None:
       mode = 0o755 if os.access(f, os.X_OK) else 0o644
     if ids is None:
       ids = (0, 0)
     if names is None:
       names = ('', '')
-    # Note: normpath changes / to \ on windows, but we expect / style paths.
-    dest = os.path.normpath(dest).replace(os.path.sep, '/')
     self.tarfile.add_file(
         dest,
         file_content=f,
@@ -109,7 +128,7 @@ def add_empty_file(self,
       ids = (0, 0)
     if names is None:
       names = ('', '')
-    dest = os.path.normpath(dest).replace(os.path.sep, '/')
+    dest = normpath(dest)
     self.tarfile.add_file(
         dest,
         content='' if kind == tarfile.REGTYPE else None,
@@ -133,21 +152,6 @@ def add_empty_dir(self, destpath, mode=None, ids=None, names=None):
     self.add_empty_file(
         destpath, mode=mode, ids=ids, names=names, kind=tarfile.DIRTYPE)
 
-  def add_empty_root_dir(self, destpath, mode=None, ids=None, names=None):
-    """Add a directory to the root of the tar file.
-
-    Args:
-       destpath: the name of the directory in the layer
-       mode: force to set the specified mode, defaults to 644
-       ids: (uid, gid) for the file to set ownership
-       names: (username, groupname) for the file to set ownership.  An empty
-         directory will be created as `destfile` in the root layer.
-    """
-    original_root_directory = self.tarfile.root_directory
-    self.tarfile.root_directory = destpath
-    self.add_empty_dir(destpath, mode=mode, ids=ids, names=names)
-    self.tarfile.root_directory = original_root_directory
-
   def add_tar(self, tar):
     """Merge a tar file into the destination tar file.
 
@@ -175,7 +179,7 @@ def add_link(self, symlink, destination, mode=None, ids=None, names=None):
       names: (username, groupname) for the file to set ownership.  An empty
         file will be created as `destfile` in the layer.
     """
-    symlink = os.path.normpath(symlink).replace(os.path.sep, '/')
+    symlink = normpath(symlink)
     self.tarfile.add_file(
         symlink,
         tarfile.SYMTYPE,
@@ -224,14 +228,14 @@ def add_tree(self, tree_top, destpath, mode=None, ids=None, names=None):
          copied to `self.directory/destfile` in the layer.
     """
     # We expect /-style paths.
-    tree_top = os.path.normpath(tree_top).replace(os.path.sep, '/')
+    tree_top = normpath(tree_top)
 
     dest = destpath.strip('/')  # redundant, dests should never have / here
     if self.directory and self.directory != '/':
       dest = self.directory.lstrip('/') + '/' + dest
 
     # Again, we expect /-style paths.
-    dest = os.path.normpath(dest).replace(os.path.sep, '/')
+    dest = normpath(dest)
     if ids is None:
       ids = (0, 0)
     if names is None:
@@ -241,7 +245,7 @@ def add_tree(self, tree_top, destpath, mode=None, ids=None, names=None):
     for root, dirs, files in os.walk(tree_top):
       # While `tree_top` uses '/' as a path separator, results returned by
       # `os.walk` and `os.path.join` on Windows may not.
-      root = os.path.normpath(root).replace(os.path.sep, '/')
+      root = normpath(root)
 
       dirs = sorted(dirs)
       rel_path_from_top = root[len(tree_top):].lstrip('/')
@@ -323,8 +327,6 @@ def main():
       help='Set mtime on tar file entries. May be an integer or the'
            ' value "portable", to get the value 2000-01-01, which is'
            ' is usable with non *nix OSes.')
-  parser.add_argument('--empty_root_dir', action='append',
-                      help='An empty dir to add to the layer.')
   parser.add_argument('--tar', action='append',
                       help='A tar file to add to the layer')
   parser.add_argument('--deb', action='append',
@@ -359,8 +361,6 @@ def main():
       '--owner_names', action='append',
       help='Specify the owner names of individual files, e.g. '
            'path/to/file=root.root.')
-  parser.add_argument('--root_directory', default='./',
-                      help='Default root directory is named "."')
   parser.add_argument('--stamp_from', default='',
                       help='File to find BUILD_STAMP in')
   options = parser.parse_args()
@@ -408,8 +408,10 @@ def main():
 
   # Add objects to the tar file
   with TarFile(
-      options.output, helpers.GetFlagValue(options.directory),
-      options.compression, options.compressor, options.root_directory,
+      options.output, 
+      directory = helpers.GetFlagValue(options.directory),
+      compression = options.compression,
+      compressor = options.compressor,
       default_mtime=default_mtime) as output:
 
     def file_attributes(filename):
@@ -427,8 +429,6 @@ def file_attributes(filename):
         for entry in manifest:
           output.add_manifest_entry(entry, file_attributes)
 
-    for f in options.empty_root_dir or []:
-      output.add_empty_root_dir(f, **file_attributes(f))
     for tar in options.tar or []:
       output.add_tar(tar)
     for deb in options.deb or []:

pkg/private/tar/tar.bzl

@@ -70,7 +70,6 @@ def _pkg_tar_impl(ctx):
 
     # Start building the arguments.
     args = ctx.actions.args()
-    args.add("--root_directory", ctx.attr.package_base)
     args.add("--output", output_file.path)
     args.add("--mode", ctx.attr.mode)
     args.add("--owner", ctx.attr.owner)
@@ -245,8 +244,9 @@ pkg_tar_impl = rule(
     implementation = _pkg_tar_impl,
     attrs = {
         "strip_prefix": attr.string(),
-        "package_base": attr.string(default = "./"),
-        "package_dir": attr.string(),
+        "package_dir": attr.string(
+            doc = """Prefix to be prepend to all paths written."""
+        ),
         "package_dir_file": attr.label(allow_single_file = True),
         "deps": attr.label_list(allow_files = tar_filetype),
         "srcs": attr.label_list(allow_files = True),

tests/deb/pkg_deb_test.py

@@ -133,17 +133,17 @@ def assert_tar_stream_content(self, data, expected, match_order=True):
   def test_expected_files(self):
     # Check the set of 'test-tar-basic-*' smoke test.
     expected = [
-        {'name': './etc', 'isdir': True,
+        {'name': 'etc', 'isdir': True,
          'uid': 24, 'gid': 42, 'uname': 'foobar', 'gname': 'fizzbuzz'},
-        {'name': './etc/nsswitch.conf',
+        {'name': 'etc/nsswitch.conf',
          'mode': 0o644,
          'uid': 24, 'gid': 42, 'uname': 'foobar', 'gname': 'fizzbuzz'
          },
-        {'name': './usr', 'isdir': True,
+        {'name': 'usr', 'isdir': True,
          'uid': 42, 'gid': 24, 'uname': 'fizzbuzz', 'gname': 'foobar'},
-        {'name': './usr/bin', 'isdir': True},
-        {'name': './usr/bin/java', 'linkname': '/path/to/bin/java'},
-        {'name': './usr/fizzbuzz',
+        {'name': 'usr/bin', 'isdir': True},
+        {'name': 'usr/bin/java', 'linkname': '/path/to/bin/java'},
+        {'name': 'usr/fizzbuzz',
          'mode': 0o755,
          'uid': 42, 'gid': 24, 'uname': 'fizzbuzz', 'gname': 'foobar'},
     ]

tests/tar/archive_test.py

@@ -163,7 +163,7 @@ def assertSimpleFileContent(self, names):
       for n in names:
         f.add_file(n, content=n)
     # pylint: disable=g-complex-comprehension
-    content = ([{"name": n,
+    content = ([{"name": n.lstrip('./'),
                  "size": len(n.encode("utf-8")),
                  "data": n.encode("utf-8")}
                 for n in names])
@@ -188,18 +188,18 @@ def testDottedFiles(self):
     content = [
         {"name": "a"},
         {"name": "/b"},
-        {"name": "./c"},
-        {"name": "./.d"},
+        {"name": "c"},
+        {"name": ".d"},
         {"name": "..e"},
         {"name": ".f"}
     ]
     self.assertTarFileContent(self.tempfile, content)
 
   def testMergeTar(self):
     content = [
-        {"name": "./a", "data": b"a"},
-        {"name": "./ab", "data": b"ab"},
-        ]
+        {"name": "a", "data": b"a"},
+        {"name": "ab", "data": b"ab"},
+    ]
     for ext in [("." + comp if comp else "") for comp in archive.COMPRESSIONS]:
       with archive.TarFileWriter(self.tempfile) as f:
         datafile = self.data_files.Rlocation(
@@ -209,14 +209,14 @@ def testMergeTar(self):
 
   def testMergeTarRelocated(self):
     content = [
-        {"name": "./foo", "mode": 0o755},
-        {"name": "./foo/a", "data": b"a"},
-        {"name": "./foo/ab", "data": b"ab"},
+        {"name": "foo", "mode": 0o755},
+        {"name": "foo/a", "data": b"a"},
+        {"name": "foo/ab", "data": b"ab"},
         ]
-    with archive.TarFileWriter(self.tempfile) as f:
+    with archive.TarFileWriter(self.tempfile, root_directory="foo") as f:
       datafile = self.data_files.Rlocation(
           "rules_pkg/tests/testdata/tar_test.tar")
-      f.add_tar(datafile, name_filter=lambda n: n != "./b", root="/foo")
+      f.add_tar(datafile, name_filter=lambda n: n != "./b")
     self.assertTarFileContent(self.tempfile, content)
 
   def testDefaultMtimeNotProvided(self):
@@ -238,7 +238,9 @@ def testPreserveTarMtimesTrueByDefault(self):
       f.add_tar(input_tar_path)
       input_tar = tarfile.open(input_tar_path, "r")
       for file_name in f.members:
-        input_file = input_tar.getmember(file_name)
+        # The test case file still uses "./a" format.  If we fix that, then
+        # update the next line accordingly.
+        input_file = input_tar.getmember("./" + file_name)
         output_file = f.tar.getmember(file_name)
         self.assertEqual(input_file.mtime, output_file.mtime)
 
@@ -324,8 +326,8 @@ def testPackageDirFileAttribute(self):
         "rules_pkg/tests/tar/test_tar_package_dir_file.tar")
 
     expected_content = [
-        {"name": "./package"},
-        {"name": "./package/nsswitch.conf"},
+        {"name": "package"},
+        {"name": "package/nsswitch.conf"},
     ]
 
     self.assertTarFileContent(package_dir, expected_content)
@@ -336,16 +338,20 @@ def testCustomCompression(self):
         "rules_pkg/tests/testdata/tar_test.tar")
     compressed = self.data_files.Rlocation(
         "rules_pkg/tests/tar/test_tar_compression.tar")
-    expected_content = [
-        {"name": "./" + x, "data": x.encode("utf-8")} for x in ["a", "b", "ab"]
-    ]
     with open(compressed, "rb") as f_in, open(self.tempfile, "wb") as f_out:
       # "Decompress" by skipping garbage bytes
       f_in.seek(len(compressor.GARBAGE))
       f_out.write(f_in.read())
 
+    expected_content = [
+        {"name": "./" + x, "data": x.encode("utf-8")} for x in ["a", "b", "ab"]
+    ]
     self.assertTarFileContent(original, expected_content)
+    expected_content = [
+        {"name": x, "data": x.encode("utf-8")} for x in ["a", "b", "ab"]
+    ]
     self.assertTarFileContent(self.tempfile, expected_content)
 
+
 if __name__ == "__main__":
   unittest.main()