Skip to content

Commit

Permalink
Preserve perms of files copied to pex chroots. (#540)
Browse files Browse the repository at this point in the history
Failing tests are added that now pass.

Fixes #536
  • Loading branch information
jsirois authored Aug 21, 2018
1 parent 263c594 commit 0a8eee3
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
8 changes: 4 additions & 4 deletions pex/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
44 changes: 43 additions & 1 deletion tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

0 comments on commit 0a8eee3

Please sign in to comment.