Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't use distutils.dir_util in copy_dir #3310

Merged
merged 4 commits into from
Apr 30, 2020
+75 −39
Merged

don't use distutils.dir_util in copy_dir #3310

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52feb0b
don't use distutils.dir_util in copy_dir (fixes #3306)
boegel Apr 30, 2020
e086444
fix copying of broken symlinks with copy/copy_file
boegel Apr 30, 2020
3c80fef
always use own implementation rather than just using shutil.copytree …
boegel Apr 30, 2020
768129e
also catch shutil.Error in copy_dir
boegel Apr 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 39 additions & 26 deletions easybuild/tools/filetools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"""
import datetime
import difflib
import distutils.dir_util
import fileinput
import glob
import hashlib
Expand Down Expand Up @@ -1925,7 +1924,12 @@ def copy_file(path, target_path, force_in_dry_run=False):
_log.info("Copied contents of file %s to %s", path, target_path)
else:
mkdir(os.path.dirname(target_path), parents=True)
shutil.copy2(path, target_path)
if os.path.exists(path):
shutil.copy2(path, target_path)
elif os.path.islink(path):
# special care for copying broken symlinks
link_target = os.readlink(path)
symlink(link_target, target_path)
_log.info("%s copied to %s", path, target_path)
except (IOError, OSError, shutil.Error) as err:
raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err)
Expand Down Expand Up @@ -1960,16 +1964,13 @@ def copy_dir(path, target_path, force_in_dry_run=False, dirs_exist_ok=False, **k
:param path: the original directory path
:param target_path: path to copy the directory to
:param force_in_dry_run: force running the command during dry run
:param dirs_exist_ok: wrapper around shutil.copytree option, which was added in Python 3.8
:param dirs_exist_ok: boolean indicating whether it's OK if the target directory already exists

On Python >= 3.8 shutil.copytree is always used
On Python < 3.8 if 'dirs_exist_ok' is False - shutil.copytree is used
On Python < 3.8 if 'dirs_exist_ok' is True - distutils.dir_util.copy_tree is used
shutil.copytree is used if the target path does not exist yet;
if the target path already exists, the 'copy' function will be used to copy the contents of
the source path to the target path

Additional specified named arguments are passed down to shutil.copytree if used.

Because distutils.dir_util.copy_tree supports only 'symlinks' named argument,
using any other will raise EasyBuildError.
Additional specified named arguments are passed down to shutil.copytree/copy if used.
"""
if not force_in_dry_run and build_option('extended_dry_run'):
dry_run_msg("copied directory %s to %s" % (path, target_path))
Expand All @@ -1978,38 +1979,49 @@ def copy_dir(path, target_path, force_in_dry_run=False, dirs_exist_ok=False, **k
if not dirs_exist_ok and os.path.exists(target_path):
raise EasyBuildError("Target location %s to copy %s to already exists", target_path, path)

if sys.version_info >= (3, 8):
# on Python >= 3.8, shutil.copytree works fine, thanks to availability of dirs_exist_ok named argument
shutil.copytree(path, target_path, dirs_exist_ok=dirs_exist_ok, **kwargs)
# note: in Python >= 3.8 shutil.copytree works just fine thanks to the 'dirs_exist_ok' argument,
# but since we need to be more careful in earlier Python versions we use our own implementation
# in case the target directory exists and 'dirs_exist_ok' is enabled
if dirs_exist_ok and os.path.exists(target_path):
# if target directory already exists (and that's allowed via dirs_exist_ok),
# we need to be more careful, since shutil.copytree will fail (in Python < 3.8)
# if target directory already exists;
# so, recurse via 'copy' function to copy files/dirs in source path to target path
# (NOTE: don't use distutils.dir_util.copy_tree here, see
# https://github.com/easybuilders/easybuild-framework/issues/3306)

entries = os.listdir(path)

elif dirs_exist_ok:
# use distutils.dir_util.copy_tree with Python < 3.8 if dirs_exist_ok is enabled
# take into account 'ignore' function that is supported by shutil.copytree
# (but not by 'copy_file' function used by 'copy')
ignore = kwargs.get('ignore')
if ignore:
ignored_entries = ignore(path, entries)
entries = [x for x in entries if x not in ignored_entries]

# first get value for symlinks named argument (if any)
preserve_symlinks = kwargs.pop('symlinks', False)
# determine list of paths to copy
paths_to_copy = [os.path.join(path, x) for x in entries]

# check if there are other named arguments (there shouldn't be, only 'symlinks' is supported)
if kwargs:
raise EasyBuildError("Unknown named arguments passed to copy_dir with dirs_exist_ok=True: %s",
', '.join(sorted(kwargs.keys())))
distutils.dir_util.copy_tree(path, target_path, preserve_symlinks=preserve_symlinks)
copy(paths_to_copy, target_path,
force_in_dry_run=force_in_dry_run, dirs_exist_ok=dirs_exist_ok, **kwargs)

else:
# if dirs_exist_ok is not enabled, just use shutil.copytree
# if dirs_exist_ok is not enabled or target directory doesn't exist, just use shutil.copytree
shutil.copytree(path, target_path, **kwargs)

_log.info("%s copied to %s", path, target_path)
except (IOError, OSError) as err:
raise EasyBuildError("Failed to copy directory %s to %s: %s", path, target_path, err)


def copy(paths, target_path, force_in_dry_run=False):
def copy(paths, target_path, force_in_dry_run=False, **kwargs):
"""
Copy single file/directory or list of files and directories to specified location

:param paths: path(s) to copy
:param target_path: target location
:param force_in_dry_run: force running the command during dry run
:param kwargs: additional named arguments to pass down to copy_dir
"""
if isinstance(paths, string_type):
paths = [paths]
Expand All @@ -2020,10 +2032,11 @@ def copy(paths, target_path, force_in_dry_run=False):
full_target_path = os.path.join(target_path, os.path.basename(path))
mkdir(os.path.dirname(full_target_path), parents=True)

if os.path.isfile(path):
# copy broken symlinks only if 'symlinks=True' is used
if os.path.isfile(path) or (os.path.islink(path) and kwargs.get('symlinks')):
copy_file(path, full_target_path, force_in_dry_run=force_in_dry_run)
elif os.path.isdir(path):
copy_dir(path, full_target_path, force_in_dry_run=force_in_dry_run)
copy_dir(path, full_target_path, force_in_dry_run=force_in_dry_run, **kwargs)
else:
raise EasyBuildError("Specified path to copy is not an existing file or directory: %s", path)

Expand Down
47 changes: 35 additions & 12 deletions test/framework/filetools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1508,22 +1508,45 @@ def test_copy_dir(self):
ft.copy_dir(to_copy, testdir, dirs_exist_ok=True)
self.assertTrue(sorted(os.listdir(to_copy)) == sorted(os.listdir(testdir)))

# if the directory already exists and 'dirs_exist_ok' is True and there is another named argument (ignore)
# we expect clean error on Python < 3.8 and pass the test on Python >= 3.8
# NOTE: reused ignore from previous test
# check whether use of 'ignore' works if target path already exists and 'dirs_exist_ok' is enabled
def ignore_func(_, names):
return [x for x in names if '6.4.0-2.28' in x]

shutil.rmtree(testdir)
ft.mkdir(testdir)
if sys.version_info >= (3, 8):
ft.copy_dir(to_copy, testdir, dirs_exist_ok=True, ignore=ignore_func)
self.assertEqual(sorted(os.listdir(testdir)), expected)
self.assertFalse(os.path.exists(os.path.join(testdir, 'GCC-6.4.0-2.28.eb')))
else:
error_pattern = "Unknown named arguments passed to copy_dir with dirs_exist_ok=True: ignore"
self.assertErrorRegex(EasyBuildError, error_pattern, ft.copy_dir, to_copy, testdir,
dirs_exist_ok=True, ignore=ignore_func)
ft.copy_dir(to_copy, testdir, dirs_exist_ok=True, ignore=ignore_func)
self.assertEqual(sorted(os.listdir(testdir)), expected)
self.assertFalse(os.path.exists(os.path.join(testdir, 'GCC-6.4.0-2.28.eb')))

# test copy_dir when broken symlinks are involved
srcdir = os.path.join(self.test_prefix, 'topdir_to_copy')
ft.mkdir(srcdir)
ft.write_file(os.path.join(srcdir, 'test.txt'), '123')
subdir = os.path.join(srcdir, 'subdir')
# introduce broken file symlink
foo_txt = os.path.join(subdir, 'foo.txt')
ft.write_file(foo_txt, 'bar')
ft.symlink(foo_txt, os.path.join(subdir, 'bar.txt'))
ft.remove_file(foo_txt)
# introduce broken dir symlink
subdir_tmp = os.path.join(srcdir, 'subdir_tmp')
ft.mkdir(subdir_tmp)
ft.symlink(subdir_tmp, os.path.join(srcdir, 'subdir_link'))
ft.remove_dir(subdir_tmp)

target_dir = os.path.join(self.test_prefix, 'target_to_copy_to')

# trying this without symlinks=True ends in tears, because bar.txt points to a non-existing file
self.assertErrorRegex(EasyBuildError, "Failed to copy directory", ft.copy_dir, srcdir, target_dir)
ft.remove_dir(target_dir)

ft.copy_dir(srcdir, target_dir, symlinks=True)

# copying directory with broken symlinks should also work if target directory already exists
ft.remove_dir(target_dir)
ft.mkdir(target_dir)
ft.mkdir(subdir)
ft.copy_dir(srcdir, target_dir, symlinks=True, dirs_exist_ok=True)

# also test behaviour of copy_file under --dry-run
build_options = {
Expand All @@ -1542,7 +1565,7 @@ def ignore_func(_, names):
self.mock_stdout(False)

self.assertFalse(os.path.exists(target_dir))
self.assertTrue(re.search("^copied directory .*/GCC to .*/GCC", txt))
self.assertTrue(re.search("^copied directory .*/GCC to .*/%s" % os.path.basename(target_dir), txt))

# forced copy, even in dry run mode
self.mock_stdout(True)
Expand Down