Merge pull request #1404 from vkarak/feat/dont-restage

[feat] Add option to disable test stage directory cleanup

Author: Vasileios Karakasis

docs/config_reference.rst

@@ -1040,6 +1040,16 @@ General Configuration
 
 
 
+.. js:attribute:: .general[].clean_stagedir
+
+   :required: No
+   :default: ``true``
+
+   Clean stage directory of tests before populating it.
+
+   .. versionadded:: 3.1
+
+
 .. js:attribute:: .general[].colorize
 
    :required: No

docs/manpage.rst

@@ -219,6 +219,14 @@ Options controlling ReFrame output
 
    This option can also be set using the :envvar:`RFM_KEEP_STAGE_FILES` environment variable or the :js:attr:`keep_stage_files` general configuration parameter.
 
+.. option:: --dont-restage
+
+   Do not restage a test if its stage directory exists.
+   Normally, if the stage directory of a test exists, ReFrame will remove it and recreate it.
+   This option disables this behavior.
+
+   .. versionadded:: 3.1
+
 .. option:: --save-log-files
 
    Save ReFrame log files in the output directory before exiting.
@@ -585,6 +593,21 @@ Here is an alphabetical list of the environment variables recognized by ReFrame:
       ================================== ==================
 
 
+.. envvar:: RFM_CLEAN_STAGEDIR
+
+   Clean stage directory of tests before populating it.
+
+   .. versionadded:: 3.1
+
+   .. table::
+      :align: left
+
+      ================================== ==================
+      Associated command line option     :option:`--dont-restage`
+      Associated configuration parameter :js:attr:`clean_stagedir` general configuration parameter
+      ================================== ==================
+
+
 .. envvar:: RFM_COLORIZE
 
    Enable output coloring.

reframe/core/pipeline.py

@@ -1072,9 +1072,11 @@ def _copy_to_stagedir(self, path):
                           (path, self._stagedir))
         self.logger.debug('symlinking files: %s' % self.readonly_files)
         try:
-            os_ext.copytree_virtual(path, self._stagedir, self.readonly_files)
+            os_ext.copytree_virtual(
+                path, self._stagedir, self.readonly_files, dirs_exist_ok=True
+            )
         except (OSError, ValueError, TypeError) as e:
-            raise PipelineError('virtual copying of files failed') from e
+            raise PipelineError('copying of files failed') from e
 
     def _clone_to_stagedir(self, url):
         self.logger.debug('cloning URL %s to stage directory (%s)' %

reframe/core/runtime.py

@@ -42,6 +42,11 @@ def _makedir(self, *dirs, wipeout=False):
         return ret
 
     def _format_dirs(self, *dirs):
+        if not self.get_option('general/0/clean_stagedir'):
+            # If stagedir is to be reused, no new stage directories will be
+            # used for retries
+            return dirs
+
         try:
             last = dirs[-1]
         except IndexError:
@@ -134,13 +139,14 @@ def stage_prefix(self):
 
         return os.path.abspath(ret)
 
-    def make_stagedir(self, *dirs, wipeout=True):
+    def make_stagedir(self, *dirs):
+        wipeout = self.get_option('general/0/clean_stagedir')
         return self._makedir(self.stage_prefix,
                              *self._format_dirs(*dirs), wipeout=wipeout)
 
-    def make_outputdir(self, *dirs, wipeout=True):
+    def make_outputdir(self, *dirs):
         return self._makedir(self.output_prefix,
-                             *self._format_dirs(*dirs), wipeout=wipeout)
+                             *self._format_dirs(*dirs), wipeout=True)
 
     @property
     def modules_system(self):

reframe/frontend/cli.py

@@ -126,6 +126,11 @@ def main():
         help='Keep stage directories even for successful checks',
         envvar='RFM_KEEP_STAGE_FILES', configvar='general/keep_stage_files'
     )
+    output_options.add_argument(
+        '--dont-restage', action='store_false', dest='clean_stagedir',
+        help='Reuse the test stage directory',
+        envvar='RFM_CLEAN_STAGEDIR', configvar='general/clean_stagedir'
+    )
     output_options.add_argument(
         '--save-log-files', action='store_true', default=False,
         help='Save ReFrame log files to the output directory',

reframe/schemas/config.json

@@ -350,6 +350,7 @@
                         "items": {"type": "string"}
                     },
                     "check_search_recursive": {"type": "boolean"},
+                    "clean_stagedir": {"type": "boolean"},
                     "colorize": {"type": "boolean"},
                     "ignore_check_conflicts": {"type": "boolean"},
                     "keep_stage_files": {"type": "boolean"},
@@ -394,6 +395,7 @@
         "environments/target_systems": ["*"],
         "general/check_search_path": ["${RFM_INSTALL_PREFIX}/checks/"],
         "general/check_search_recursive": false,
+        "general/clean_stagedir": true,
         "general/colorize": true,
         "general/ignore_check_conflicts": false,
         "general/keep_stage_files": false,

reframe/utility/os_ext.py

@@ -95,25 +95,45 @@ def osgroup():
 
 
 def copytree(src, dst, symlinks=False, ignore=None, copy_function=shutil.copy2,
-             ignore_dangling_symlinks=False):
-    '''Same as shutil.copytree() but valid also if 'dst' exists.
-
-    In this case it will first remove it and then call the standard
-    shutil.copytree().'''
+             ignore_dangling_symlinks=False, dirs_exist_ok=False):
+    '''Compatibility version of :py:func:`shutil.copytree()` for Python <= 3.8
+    '''
     if src == os.path.commonpath([src, dst]):
         raise ValueError("cannot copy recursively the parent directory "
                          "`%s' into one of its descendants `%s'" % (src, dst))
 
-    if os.path.exists(dst):
-        shutil.rmtree(dst)
+    if sys.version_info[1] >= 8:
+        return shutil.copytree(src, dst, symlinks, ignore, copy_function,
+                               ignore_dangling_symlinks, dirs_exist_ok)
+
+    if not dirs_exist_ok:
+        return shutil.copytree(src, dst, symlinks, ignore, copy_function,
+                               ignore_dangling_symlinks)
+
+    # dirs_exist_ok=True and Python < 3.8
+    if not os.path.exists(dst):
+        return shutil.copytree(src, dst, symlinks, ignore, copy_function,
+                               ignore_dangling_symlinks)
+
+    # dst exists; manually descend into the subdirectories
+    _, subdirs, files = list(os.walk(src))[0]
+    ignore_paths = ignore(src, os.listdir(src)) if ignore else {}
+    for f in files:
+        if f not in ignore_paths:
+            copy_function(os.path.join(src, f), os.path.join(dst, f))
 
-    shutil.copytree(src, dst, symlinks, ignore, copy_function,
-                    ignore_dangling_symlinks)
+    for d in subdirs:
+        if d not in ignore_paths:
+            copytree(os.path.join(src, d), os.path.join(dst, d),
+                     symlinks, ignore, copy_function,
+                     ignore_dangling_symlinks, dirs_exist_ok)
+
+    return dst
 
 
 def copytree_virtual(src, dst, file_links=[],
                      symlinks=False, copy_function=shutil.copy2,
-                     ignore_dangling_symlinks=False):
+                     ignore_dangling_symlinks=False, dirs_exist_ok=False):
     '''Copy `dst` to `src`, but create symlinks for the files in `file_links`.
 
     If `file_links` is empty, this is equivalent to `copytree()`.  The rest of
@@ -134,35 +154,42 @@ def copytree_virtual(src, dst, file_links=[],
     link_targets = set()
     for f in file_links:
         if os.path.isabs(f):
-            raise ValueError("copytree_virtual() failed: `%s': "
-                             "absolute paths not allowed in file_links" % f)
+            raise ValueError(f'copytree_virtual() failed: {f!r}: '
+                             f'absolute paths not allowed in file_links')
 
         target = os.path.join(src, f)
         if not os.path.exists(target):
-            raise ValueError("copytree_virtual() failed: `%s' "
-                             "does not exist" % target)
+            raise ValueError(f'copytree_virtual() failed: {target!r} '
+                             f'does not exist')
 
         if os.path.commonpath([src, target]) != src:
-            raise ValueError("copytree_virtual() failed: "
-                             "`%s' not under `%s'" % (target, src))
+            raise ValueError(f'copytree_virtual() failed: '
+                             f'{target!r} not under {src!r}')
 
         link_targets.add(os.path.abspath(target))
 
+    if '.' in file_links or '..' in file_links:
+        raise ValueError(f"'.' or '..' are not allowed in file_links")
+
     if not file_links:
         ignore = None
     else:
         def ignore(dir, contents):
-            return [c for c in contents
-                    if os.path.join(dir, c) in link_targets]
+            return {c for c in contents
+                    if os.path.join(dir, c) in link_targets}
 
     # Copy to dst ignoring the file_links
     copytree(src, dst, symlinks, ignore,
-             copy_function, ignore_dangling_symlinks)
+             copy_function, ignore_dangling_symlinks, dirs_exist_ok)
 
     # Now create the symlinks
     for f in link_targets:
         link_name = f.replace(src, dst)
-        os.symlink(f, link_name)
+        try:
+            os.symlink(f, link_name)
+        except FileExistsError:
+            if not dirs_exist_ok:
+                raise
 
 
 def rmtree(*args, max_retries=3, **kwargs):

unittests/test_cli.py

@@ -221,6 +221,31 @@ def test_check_sanity_failure(run_reframe, tmp_path):
     )
 
 
+def test_dont_restage(run_reframe, tmp_path):
+    run_reframe(
+        checkpath=['unittests/resources/checks/frontend_checks.py'],
+        more_options=['-t', 'SanityFailureCheck']
+    )
+
+    # Place a random file in the test's stage directory and rerun with
+    # `--dont-restage` and `--max-retries`
+    stagedir = (tmp_path / 'stage' / 'generic' / 'default' /
+                'builtin-gcc' / 'SanityFailureCheck')
+    (stagedir / 'foobar').touch()
+    returncode, stdout, stderr = run_reframe(
+        checkpath=['unittests/resources/checks/frontend_checks.py'],
+        more_options=['-t', 'SanityFailureCheck',
+                      '--dont-restage', '--max-retries=1']
+    )
+    assert os.path.exists(stagedir / 'foobar')
+    assert not os.path.exists(f'{stagedir}_retry1')
+
+    # And some standard assertions
+    assert 'Traceback' not in stdout
+    assert 'Traceback' not in stderr
+    assert returncode != 0
+
+
 def test_checkpath_symlink(run_reframe, tmp_path):
     # FIXME: This should move to test_loader.py
     checks_symlink = tmp_path / 'checks_symlink'

unittests/test_utility.py

@@ -61,15 +61,7 @@ def test_command_async(self):
     def test_copytree(self):
         dir_src = tempfile.mkdtemp()
         dir_dst = tempfile.mkdtemp()
-
-        with pytest.raises(OSError):
-            shutil.copytree(dir_src, dir_dst)
-
-        try:
-            os_ext.copytree(dir_src, dir_dst)
-        except Exception as e:
-            pytest.fail('custom copytree failed: %s' % e)
-
+        os_ext.copytree(dir_src, dir_dst, dirs_exist_ok=True)
         shutil.rmtree(dir_src)
         shutil.rmtree(dir_dst)
 
@@ -284,6 +276,9 @@ def setUp(self):
         open(os.path.join(self.prefix, 'bar.txt'), 'w').close()
         open(os.path.join(self.prefix, 'foo.txt'), 'w').close()
 
+        # Create also a subdirectory in target, so as to check the recursion
+        os.makedirs(os.path.join(self.target, 'foo'), exist_ok=True)
+
     def verify_target_directory(self, file_links=[]):
         '''Verify the directory structure'''
         assert os.path.exists(os.path.join(self.target, 'bar', 'bar.txt'))
@@ -301,38 +296,54 @@ def verify_target_directory(self, file_links=[]):
             assert target_name == os.readlink(link_name)
 
     def test_virtual_copy_nolinks(self):
-        os_ext.copytree_virtual(self.prefix, self.target)
+        os_ext.copytree_virtual(self.prefix, self.target, dirs_exist_ok=True)
         self.verify_target_directory()
 
+    def test_virtual_copy_nolinks_dirs_exist(self):
+        with pytest.raises(FileExistsError):
+            os_ext.copytree_virtual(self.prefix, self.target)
+
     def test_virtual_copy_valid_links(self):
         file_links = ['bar/', 'foo/bar.txt', 'foo.txt']
-        os_ext.copytree_virtual(self.prefix, self.target, file_links)
+        os_ext.copytree_virtual(self.prefix, self.target,
+                                file_links, dirs_exist_ok=True)
         self.verify_target_directory(file_links)
 
     def test_virtual_copy_inexistent_links(self):
         file_links = ['foobar/', 'foo/bar.txt', 'foo.txt']
         with pytest.raises(ValueError):
-            os_ext.copytree_virtual(self.prefix, self.target, file_links)
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
 
     def test_virtual_copy_absolute_paths(self):
         file_links = [os.path.join(self.prefix, 'bar'),
                       'foo/bar.txt', 'foo.txt']
         with pytest.raises(ValueError):
-            os_ext.copytree_virtual(self.prefix, self.target, file_links)
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
 
     def test_virtual_copy_irrelevenant_paths(self):
         file_links = ['/bin', 'foo/bar.txt', 'foo.txt']
         with pytest.raises(ValueError):
-            os_ext.copytree_virtual(self.prefix, self.target, file_links)
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
 
         file_links = [os.path.dirname(self.prefix), 'foo/bar.txt', 'foo.txt']
         with pytest.raises(ValueError):
-            os_ext.copytree_virtual(self.prefix, self.target, file_links)
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
 
     def test_virtual_copy_linkself(self):
         file_links = ['.']
-        with pytest.raises(OSError):
-            os_ext.copytree_virtual(self.prefix, self.target, file_links)
+        with pytest.raises(ValueError):
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
+
+    def test_virtual_copy_linkparent(self):
+        file_links = ['..']
+        with pytest.raises(ValueError):
+            os_ext.copytree_virtual(self.prefix, self.target,
+                                    file_links, dirs_exist_ok=True)
 
     def tearDown(self):
         shutil.rmtree(self.prefix)