Preserve perms of files copied to pex chroots. (#540)

Failing tests are added that now pass.

Fixes #536

pex/common.py

@@ -25,7 +25,7 @@ def die(msg, exit_code=1):
 def safe_copy(source, dest, overwrite=False):
   def do_copy():
     temp_dest = dest + uuid4().hex
-    shutil.copyfile(source, temp_dest)
+    shutil.copy(source, temp_dest)
     os.rename(temp_dest, dest)
 
   # If the platform supports hard-linking, use that and fall back to copying.
@@ -279,7 +279,7 @@ def _ensure_parent(self, path):
   def copy(self, src, dst, label=None):
     """Copy file ``src`` to ``chroot/dst`` with optional label.
 
-    May raise anything shutil.copyfile can raise, e.g.
+    May raise anything shutil.copy can raise, e.g.
       IOError(Errno 21 'EISDIR')
 
     May raise ChrootTaggingException if dst is already in a fileset
@@ -288,7 +288,7 @@ def copy(self, src, dst, label=None):
     dst = self._normalize(dst)
     self._tag(dst, label)
     self._ensure_parent(dst)
-    shutil.copyfile(src, os.path.join(self.chroot, dst))
+    shutil.copy(src, os.path.join(self.chroot, dst))
 
   def link(self, src, dst, label=None):
     """Hard link file from ``src`` to ``chroot/dst`` with optional label.
@@ -348,7 +348,7 @@ def __str__(self):
   def delete(self):
     shutil.rmtree(self.chroot)
 
-  def zip(self, filename, mode='wb'):
+  def zip(self, filename, mode='w'):
     with open_zip(filename, mode) as zf:
       for f in sorted(self.files()):
         zf.write(os.path.join(self.chroot, f), arcname=f, compress_type=zipfile.ZIP_DEFLATED)

tests/test_common.py

@@ -7,7 +7,7 @@
 
 import pytest
 
-from pex.common import PermPreservingZipFile, chmod_plus_x, rename_if_empty, touch
+from pex.common import Chroot, PermPreservingZipFile, chmod_plus_x, rename_if_empty, touch
 from pex.testing import temporary_dir
 
 try:
@@ -86,3 +86,45 @@ def test_perm_preserving_zipfile_extract():
 
       assert extract_perms(one) == extract_perms(os.path.join(extract_dir, 'one'))
       assert extract_perms(two) == extract_perms(os.path.join(extract_dir, 'two'))
+
+
+def assert_chroot_perms(copyfn):
+  with temporary_dir() as src:
+    one = os.path.join(src, 'one')
+    touch(one)
+
+    two = os.path.join(src, 'two')
+    touch(two)
+    chmod_plus_x(two)
+
+    with temporary_dir() as dst:
+      chroot = Chroot(dst)
+      copyfn(chroot, one, 'one')
+      copyfn(chroot, two, 'two')
+      assert extract_perms(one) == extract_perms(os.path.join(chroot.path(), 'one'))
+      assert extract_perms(two) == extract_perms(os.path.join(chroot.path(), 'two'))
+
+      zip_path = os.path.join(src, 'chroot.zip')
+      chroot.zip(zip_path)
+      with temporary_dir() as extract_dir:
+        with contextlib.closing(PermPreservingZipFile(zip_path)) as zf:
+          zf.extractall(extract_dir)
+
+          assert extract_perms(one) == extract_perms(os.path.join(extract_dir, 'one'))
+          assert extract_perms(two) == extract_perms(os.path.join(extract_dir, 'two'))
+
+
+def test_chroot_perms_copy():
+  assert_chroot_perms(Chroot.copy)
+
+
+def test_chroot_perms_link_same_device():
+  assert_chroot_perms(Chroot.link)
+
+
+def test_chroot_perms_link_cross_device():
+  with mock.patch('os.link', spec_set=True, autospec=True) as mock_link:
+    expected_errno = errno.EXDEV
+    mock_link.side_effect = OSError(expected_errno, os.strerror(expected_errno))
+
+    assert_chroot_perms(Chroot.link)