Merge pull request #3310 from boegel/no_distutils_dir_utils

don't use distutils.dir_util in copy_dir

Author: lexming

easybuild/tools/filetools.py

@@ -40,7 +40,6 @@
 """
 import datetime
 import difflib
-import distutils.dir_util
 import fileinput
 import glob
 import hashlib
@@ -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)
@@ -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))
@@ -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:
+        except (IOError, OSError, shutil.Error) 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]
@@ -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)
 

test/framework/filetools.py

@@ -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 = {
@@ -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)