Stop stripping tree artifact root name in pkg_tar and pkg_zip.

Closes bazelbuild#404

While adding tests to make sure we actually could strip it with
pkg_files, I discovered a deficiency in pkg_tar_test. It was not doing
a check on the file names, only content and type.  Fixing that exposed
a pre-existing bug.

Fortunately, the problem uncovered is fixed by bazelbuild#554, so the tests
should pass once that, followed by this PR are submitted.

Allow pkg_files.strip_prefix to work on tree artifact without having to
use `renames`.

Update to 0.7.0 to reflect that this is sort of a big behavioral change.

pkg/mappings.bzl

@@ -122,6 +122,8 @@ def _do_strip_prefix(path, to_strip, src_file):
 
     if path_norm.startswith(to_strip_norm):
         return path_norm[len(to_strip_norm):]
+    elif src_file.is_directory and (path_norm + '/') == to_strip_norm:
+        return ''
     else:
         # Avoid user surprise by failing if prefix stripping doesn't work as
         # expected.

pkg/private/tar/tar.bzl

@@ -141,11 +141,7 @@ def _pkg_tar_impl(ctx):
             for f in src_files:
                 d_path = dest_path(f, data_path, data_path_without_prefix)
                 if f.is_directory:
-                    # Tree artifacts need a name, but the name is never really
-                    # the important part. The likely behavior people want is
-                    # just the content, so we strip the directory name.
-                    dest = "/".join(d_path.split("/")[0:-1])
-                    add_tree_artifact(content_map, dest, f, src.label)
+                    add_tree_artifact(content_map, d_path, f, src.label)
                 else:
                     # Note: This extra remap is the bottleneck preventing this
                     # large block from being a utility method as shown below.

pkg/private/zip/zip.bzl

@@ -68,11 +68,7 @@ def _pkg_zip_impl(ctx):
             for f in src.files.to_list():
                 d_path = dest_path(f, data_path, data_path_without_prefix)
                 if f.is_directory:
-                    # Tree artifacts need a name, but the name is never really
-                    # the important part. The likely behavior people want is
-                    # just the content, so we strip the directory name.
-                    dest = "/".join(d_path.split("/")[0:-1])
-                    add_tree_artifact(content_map, dest, f, src.label)
+                    add_tree_artifact(content_map, d_path, f, src.label)
                 else:
                     add_single_file(content_map, d_path, f, src.label)
 

tests/tar/BUILD

@@ -15,6 +15,7 @@
 """Tests for pkg_tar."""
 
 # buildifier: disable=bzl-visibility
+load("//pkg:mappings.bzl", "pkg_files", "strip_prefix")
 load("//pkg/private/tar:tar.bzl", "SUPPORTED_TAR_COMPRESSIONS", "pkg_tar")
 load("//tests:my_package_name.bzl", "my_package_naming")
 load("//tests/util:defs.bzl", "directory", "fake_artifact")
@@ -267,10 +268,29 @@ pkg_tar(
     name = "test-tar-tree-artifact",
     srcs = [
         ":generate_tree",
+        "//tests:loremipsum_txt",
     ],
     package_dir = "a_tree",
 )
 
+pkg_files(
+    name = "tree_noroot",
+    srcs = [
+        ":generate_tree",
+    ],
+    strip_prefix = "generate_tree",
+)
+
+pkg_tar(
+    name = "test-tar-tree-artifact-noroot",
+    srcs = [
+        ":tree_noroot",
+        "//tests:loremipsum_txt",
+    ],
+    package_dir = "a_tree",
+)
+
+
 py_test(
     name = "pkg_tar_test",
     size = "medium",
@@ -291,6 +311,7 @@ py_test(
         ":test-tar-long-filename",
         ":test-tar-repackaging-long-filename.tar",
         ":test-tar-tree-artifact",
+        ":test-tar-tree-artifact-noroot",
         ":test-tar-with-runfiles",
         "//tests:testdata/tar_test.tar",
         "//tests:testdata/tar_test.tar.bz2",
@@ -324,15 +345,6 @@ test_suite(
     tests = [":windows_tests"],
 )
 
-# Used within experimental/tests/ for external repo genpkg tests
-filegroup(
-    name = "loremipsum_txt",
-    srcs = [
-        "//tests:testdata/loremipsum.txt",
-    ],
-    visibility = ["//visibility:public"],
-)
-
 pkg_tar(
     name = "test-tar-compression-from-extension-targz",
     srcs = [

tests/tar/pkg_tar_test.py

@@ -25,7 +25,7 @@
 class PkgTarTest(unittest.TestCase):
   """Testing for pkg_tar rule."""
 
-  def assertTarFileContent(self, file_name, content):
+  def assertTarFileContent(self, file_name, content, verbose=False):
     """Assert that tarfile contains exactly the entry described by `content`.
 
     Args:
@@ -43,7 +43,8 @@ def assertTarFileContent(self, file_name, content):
     with tarfile.open(file_path, 'r:*') as f:
       i = 0
       for info in f:
-        print('============got', info.name)
+        if verbose:
+          print('  >> from tar file:', info.name)
         error_msg = 'Extraneous file at end of archive %s: %s' % (
             file_path,
             info.name
@@ -61,7 +62,9 @@ def assertTarFileContent(self, file_name, content):
               '%s in archive %s does' % (info.name, file_path),
               'not match expected value `%s`' % v
               ])
-          # self.assertEqual(value, v, error_msg)
+          if verbose:
+            print('   >>> expect', k, v)
+          self.assertEqual(value, v, error_msg)
           if value != v:
             print(error_msg)
         i += 1
@@ -205,18 +208,29 @@ def test_tar_with_tree_artifact(self):
 
     content = [
       {'name': './a_tree', 'isdir': True},
-      {'name': './a_tree/a', 'isdir': True},
-      {'name': './a_tree/a/a'},
-      {'name': './a_tree/a/b', 'isdir': True},
-      {'name': './a_tree/a/b/c'},
-      {'name': './a_tree/b', 'isdir': True},
-      {'name': './a_tree/b/c', 'isdir': True},
-      {'name': './a_tree/b/c/d'},
-      {'name': './a_tree/b/d'},
-      {'name': './a_tree/b/e'},
+      {'name': './a_tree/generate_tree', 'isdir': True},
+      {'name': './a_tree/generate_tree/a', 'isdir': True},
+      {'name': './a_tree/generate_tree/a/a'},
+      {'name': './a_tree/generate_tree/a/b', 'isdir': True},
+      {'name': './a_tree/generate_tree/a/b/c'},
+      {'name': './a_tree/generate_tree/b', 'isdir': True},
+      {'name': './a_tree/generate_tree/b/c', 'isdir': True},
+      {'name': './a_tree/generate_tree/b/c/d'},
+      {'name': './a_tree/generate_tree/b/d'},
+      {'name': './a_tree/generate_tree/b/e'},
+      {'name': './a_tree/loremipsum.txt'},
     ]
     self.assertTarFileContent('test-tar-tree-artifact.tar', content)
 
+    # Now test against the tree artifact with the name stripped.
+    noroot_content = []
+    for c in content[1:]:
+      nc = dict(c)
+      nc['name'] = c['name'].replace('/generate_tree', '')
+      noroot_content.append(nc)
+    self.assertTarFileContent('test-tar-tree-artifact-noroot.tar',
+                              noroot_content)
+
   def test_tar_with_runfiles(self):
     content = [
       {'name': './BUILD' },

version.bzl

@@ -13,4 +13,4 @@
 # limitations under the License.
 """The version of rules_pkg."""
 
-version = "0.6.0"
+version = "0.7.0"