From 8bba41f39198215ea1378ab5c325dde7deaba358 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 18 Aug 2018 10:35:40 -0600 Subject: [PATCH] Preserve perms of files copied to pex chroots. Failing tests are added that now pass. Fixes #536 --- pex/common.py | 8 ++++---- tests/test_common.py | 44 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/pex/common.py b/pex/common.py index f7bb53cf6..a0e06deb7 100644 --- a/pex/common.py +++ b/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) diff --git a/tests/test_common.py b/tests/test_common.py index 23762d6ad..c3ac69041 100644 --- a/tests/test_common.py +++ b/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)