[Docker] archive: appends '/' at end of directories

This ensure to correctly detect duplicate for directory entries
that might or might not contains ending slash.

--
MOS_MIGRATED_REVID=104502468

5 files changed, 103 insertions, 0 deletions

diff --git a/tools/build_defs/docker/BUILD b/tools/build_defs/docker/BUILD
index 921d347a60..91ea88cfe2 100644
--- a/tools/build_defs/docker/BUILD
+++ b/tools/build_defs/docker/BUILD
@@ -36,6 +36,7 @@ TEST_DATA = [
     "//tools/build_defs/docker/testdata:data_path_image",
     "//tools/build_defs/docker/testdata:no_data_path_image",
     "//tools/build_defs/docker/testdata:dummy_repository",
+    "//tools/build_defs/docker/testdata:extras_with_deb",
 ]
 
 sh_test(
diff --git a/tools/build_defs/docker/archive.py b/tools/build_defs/docker/archive.py
index b2c2b388f9..0a32e404aa 100644
--- a/tools/build_defs/docker/archive.py
+++ b/tools/build_defs/docker/archive.py
@@ -156,6 +156,9 @@ class TarFileWriter(object):
 
   def _addfile(self, info, fileobj=None):
     """Add a file in the tar file if there is no conflict."""
+    if not info.name.endswith('/') and info.type == tarfile.DIRTYPE:
+      # Enforce the ending / for directories so we correctly deduplicate.
+      info.name += '/'
     if info.name not in self.members:
       self.tar.addfile(info, fileobj)
       self.members.add(info.name)
diff --git a/tools/build_defs/docker/build_test.sh b/tools/build_defs/docker/build_test.sh
index 9d9d1429c6..409d6e2382 100755
--- a/tools/build_defs/docker/build_test.sh
+++ b/tools/build_defs/docker/build_test.sh
@@ -372,4 +372,21 @@ function test_data_path() {
     ./test/test
 }
 
+function test_extras_with_deb() {
+  local test_data="${TEST_DATA_DIR}/extras_with_deb.tar"
+  local sha=$(tar xOf ${test_data} ./top)
+
+  # The content of the layer should have no duplicate
+  local layer_listing="$(get_layer_listing "extras_with_deb" "${sha}" | sort)"
+  check_eq "${layer_listing}" \
+"./
+./etc/
+./etc/nsswitch.conf
+./tmp/
+./usr/
+./usr/bin/
+./usr/bin/java -> /path/to/bin/java
+./usr/titi"
+}
+
 run_suite "build_test"
diff --git a/tools/build_defs/docker/testdata/BUILD b/tools/build_defs/docker/testdata/BUILD
index 9ea8f162d4..c18fbfacf2 100644
--- a/tools/build_defs/docker/testdata/BUILD
+++ b/tools/build_defs/docker/testdata/BUILD
@@ -291,3 +291,25 @@ docker_build(
         "/usr/bin/java": "/bar",
     },
 )
+
+# Generate a dummy debian package with a test/ directory
+py_binary(
+    name = "gen_deb",
+    srcs = ["gen_deb.py"],
+)
+
+genrule(
+    name = "generate_deb",
+    outs = ["gen.deb"],
+    cmd = "$(location :gen_deb) $@",
+    tools = [":gen_deb"],
+)
+
+# Bot gen.deb and test-data has a test directory, it should appear
+# only once in the resulting layer.
+docker_build(
+    name = "extras_with_deb",
+    data_path = ".",
+    debs = [":gen.deb"],
+    tars = ["extras.tar"],
+)
diff --git a/tools/build_defs/docker/testdata/gen_deb.py b/tools/build_defs/docker/testdata/gen_deb.py
new file mode 100644
index 0000000000..7468e22ae3
--- /dev/null
+++ b/tools/build_defs/docker/testdata/gen_deb.py
@@ -0,0 +1,60 @@
+# Copyright 2015 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""A simple cross-platform helper to create a dummy debian package."""
+from StringIO import StringIO
+import sys
+import tarfile
+
+
+def AddArFileEntry(fileobj, filename, content=''):
+  """Add a AR file entry to fileobj."""
+  fileobj.write((filename + '/').ljust(16))    # filename (SysV)
+  fileobj.write('0'.ljust(12))                 # timestamp
+  fileobj.write('0'.ljust(6))                  # owner id
+  fileobj.write('0'.ljust(6))                  # group id
+  fileobj.write('0644'.ljust(8))               # mode
+  fileobj.write(str(len(content)).ljust(10))   # size
+  fileobj.write('\x60\x0a')                    # end of file entry
+  fileobj.write(content)
+  if len(content) % 2 != 0:
+    fileobj.write('\n')  # 2-byte alignment padding
+
+if __name__ == '__main__':
+  # Create data.tar
+  tar = StringIO()
+  with tarfile.open('data.tar', mode='w', fileobj=tar) as f:
+    tarinfo = tarfile.TarInfo('usr/')
+    tarinfo.type = tarfile.DIRTYPE
+    f.addfile(tarinfo)
+    tarinfo = tarfile.TarInfo('usr/titi')
+    f.addfile(tarinfo, fileobj=StringIO('toto\n'))
+  data = tar.getvalue()
+  tar.close()
+  # Create control.tar
+  tar = StringIO()
+  with tarfile.open('control.tar', mode='w', fileobj=tar) as f:
+    tarinfo = tarfile.TarInfo('control')
+    f.addfile(tarinfo, fileobj=StringIO('\n'.join([
+        'Package: test'
+        'Description: Just a dummy test'
+        ])))
+  control = tar.getvalue()
+  tar.close()
+
+  # Write the final AR archive (the deb package)
+  with open(sys.argv[1], 'w') as f:
+    f.write('!<arch>\n')  # Magic AR header
+    AddArFileEntry(f, 'debian-binary', '2.0')
+    AddArFileEntry(f, 'control.tar', control)
+    AddArFileEntry(f, 'data.tar', data)