From 8c4dd2979705f8d7aef266edbd4b20a306077888 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Fri, 20 Jun 2025 17:35:50 +0200 Subject: [PATCH 01/13] add empty_dir function --- easybuild/tools/filetools.py | 48 +++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 3126c1c020..4cec131611 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -376,6 +376,40 @@ def remove_file(path): raise EasyBuildError("Failed to remove file %s: %s", path, err) +def empty_dir(path): + """Empty directory at specified path, keeping directory itself intact.""" + # early exit in 'dry run' mode + if build_option('extended_dry_run'): + dry_run_msg("directory %s removed" % path, silent=build_option('silent')) + return + + if os.path.exists(path): + ok = False + errors = [] + # Try multiple times to cater for temporary failures on e.g. NFS mounted paths + max_attempts = 3 + for i in range(0, max_attempts): + try: + for item in os.listdir(path): + subpath = os.path.join(path, item) + if os.path.isfile(subpath) or os.path.islink(subpath): + os.remove(subpath) + elif os.path.isdir(subpath): + shutil.rmtree(subpath) + ok = True + except OSError as err: + _log.debug("Failed to empty path %s at attempt %d: %s" % (path, i, err)) + errors.append(err) + time.sleep(2) + # make sure write permissions are enabled on entire directory + adjust_permissions(path, stat.S_IWUSR, add=True, recursive=True, ignore_root=True) + if ok: + _log.info("Path %s successfully emptied." % path) + else: + raise EasyBuildError("Failed to empty directory %s even after %d attempts.\nReasons: %s", + path, max_attempts, errors) + + def remove_dir(path): """Remove directory at specified path.""" # early exit in 'dry run' mode @@ -1858,7 +1892,7 @@ def convert_name(name, upper=False): def adjust_permissions(provided_path, permission_bits, add=True, onlyfiles=False, onlydirs=False, recursive=True, - group_id=None, relative=True, ignore_errors=False): + group_id=None, relative=True, ignore_errors=False, ignore_root=False): """ Change permissions for specified path, using specified permission bits @@ -1870,16 +1904,24 @@ def adjust_permissions(provided_path, permission_bits, add=True, onlyfiles=False :param relative: add/remove permissions relative to current permissions (if False, hard set specified permissions) :param ignore_errors: ignore errors that occur when changing permissions (up to a maximum ratio specified by --max-fail-ratio-adjust-permissions configuration option) + :param ignore_root: don’t change the permissions of the root path (provided_path) Add or remove (if add is False) permission_bits from all files (if onlydirs is False) and directories (if onlyfiles is False) in path """ + if not recursive and ignore_root: + _log.info("Not adjusting permissions for %s (no recursion and ignoring root)", provided_path) + return + provided_path = os.path.abspath(provided_path) if recursive: _log.info("Adjusting permissions recursively for %s", provided_path) - allpaths = [provided_path] + if ignore_root: + allpaths = [] + else: + allpaths = [provided_path] for root, dirs, files in os.walk(provided_path): paths = [] if not onlydirs: @@ -1952,7 +1994,7 @@ def adjust_permissions(provided_path, permission_bits, add=True, onlyfiles=False if failed_paths: raise EasyBuildError("Failed to chmod/chown several paths: %s (last error: %s)", failed_paths, err_msg) - # we ignore some errors, but if there are to many, something is definitely wrong + # we ignore some errors, but if there are too many, something is definitely wrong fail_ratio = fail_cnt / float(len(allpaths)) max_fail_ratio = float(build_option('max_fail_ratio_adjust_permissions')) if fail_ratio > max_fail_ratio: From 90e62ce1cf74b0971f89cd6fc9102af92786582f Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 21 Jun 2025 12:13:06 +0200 Subject: [PATCH 02/13] use empty_dir in make_dir --- easybuild/framework/easyblock.py | 44 ++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 021d524012..efec180e4d 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -93,10 +93,11 @@ from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, check_lock from easybuild.tools.filetools import compute_checksum, convert_name, copy_dir, copy_file, create_lock from easybuild.tools.filetools import create_non_existing_paths, create_patch_info, derive_alt_pypi_url, diff_files -from easybuild.tools.filetools import download_file, encode_class_name, extract_file, find_backup_name_candidate -from easybuild.tools.filetools import get_cwd, get_source_tarball_from_git, is_alt_pypi_url, is_binary, is_parent_path -from easybuild.tools.filetools import is_sha256_checksum, mkdir, move_file, move_logs, read_file, remove_dir -from easybuild.tools.filetools import remove_file, remove_lock, symlink, verify_checksum, weld_paths, write_file +from easybuild.tools.filetools import download_file, empty_dir, encode_class_name, extract_file +from easybuild.tools.filetools import find_backup_name_candidate, get_cwd, get_source_tarball_from_git, is_alt_pypi_url +from easybuild.tools.filetools import is_binary, is_parent_path, is_sha256_checksum, mkdir, move_file, move_logs +from easybuild.tools.filetools import read_file, remove_dir, remove_file, remove_lock, symlink, verify_checksum +from easybuild.tools.filetools import weld_paths, write_file from easybuild.tools.hooks import ( BUILD_STEP, CLEANUP_STEP, CONFIGURE_STEP, EASYBLOCK, EXTENSIONS_STEP, EXTRACT_STEP, FETCH_STEP, INSTALL_STEP, MODULE_STEP, MODULE_WRITE, PACKAGE_STEP, PATCH_STEP, PERMISSIONS_STEP, POSTITER_STEP, POSTPROC_STEP, PREPARE_STEP, @@ -1171,8 +1172,13 @@ def make_builddir(self): # unless we're building in installation directory and we iterating over a list of (pre)config/build/installopts, # otherwise we wipe the already partially populated installation directory, # see https://github.com/easybuilders/easybuild-framework/issues/2556 - if not (self.build_in_installdir and self.iter_idx > 0): - # make sure we no longer sit in the build directory before cleaning it. + if self.build_in_installdir and self.iter_idx > 0: + pass + elif self.build_in_installdir: + # building in installation directory, so empty it but don't remove it + self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], isinstalldir=True) + else: + # make sure we no longer sit in the build directory before removing it. change_dir(self.orig_workdir) self.make_dir(self.builddir, self.cfg['cleanupoldbuild']) @@ -1220,9 +1226,10 @@ def make_installdir(self, dontcreate=None): if self.build_in_installdir: self.cfg['keeppreviousinstall'] = True dontcreate = (dontcreate is None and self.cfg['dontcreateinstalldir']) or dontcreate - self.make_dir(self.installdir, self.cfg['cleanupoldinstall'], dontcreateinstalldir=dontcreate) + self.make_dir(self.installdir, self.cfg['cleanupoldinstall'], dontcreateinstalldir=dontcreate, + isinstalldir=True) - def make_dir(self, dir_name, clean, dontcreateinstalldir=False): + def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=False): """ Create the directory. """ @@ -1233,15 +1240,20 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False): return elif build_option('module_only') or self.cfg['module_only']: self.log.info("Not touching existing directory %s in module-only mode...", dir_name) - elif clean: - remove_dir(dir_name) - self.log.info("Removed old directory %s", dir_name) else: - self.log.info("Moving existing directory %s out of the way...", dir_name) - timestamp = time.strftime("%Y%m%d-%H%M%S") - backupdir = "%s.%s" % (dir_name, timestamp) - move_file(dir_name, backupdir) - self.log.info("Moved old directory %s to %s", dir_name, backupdir) + if not clean: + self.log.info("Creating backup of directory %s...", dir_name) + timestamp = time.strftime("%Y%m%d-%H%M%S") + backupdir = "%s.%s" % (dir_name, timestamp) + copy_dir(dir_name, backupdir) + self.log.info("Copied old directory %s to %s", dir_name, backupdir) + if isinstalldir: + # empty the installation directory, but never remove it + empty_dir(dir_name) + self.log.info("Emptied old directory %s", dir_name) + else: + remove_dir(dir_name) + self.log.info("Removed old directory %s", dir_name) if dontcreateinstalldir: olddir = dir_name From 9501a71c6dafbe8020f6e6a5ee6cb7a7cfad1943 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 21 Jun 2025 13:56:46 +0200 Subject: [PATCH 03/13] modifying permissions on install dir is no problem --- easybuild/tools/filetools.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 4cec131611..f91c394d3b 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -402,7 +402,7 @@ def empty_dir(path): errors.append(err) time.sleep(2) # make sure write permissions are enabled on entire directory - adjust_permissions(path, stat.S_IWUSR, add=True, recursive=True, ignore_root=True) + adjust_permissions(path, stat.S_IWUSR, add=True, recursive=True) if ok: _log.info("Path %s successfully emptied." % path) else: @@ -1892,7 +1892,7 @@ def convert_name(name, upper=False): def adjust_permissions(provided_path, permission_bits, add=True, onlyfiles=False, onlydirs=False, recursive=True, - group_id=None, relative=True, ignore_errors=False, ignore_root=False): + group_id=None, relative=True, ignore_errors=False): """ Change permissions for specified path, using specified permission bits @@ -1904,24 +1904,16 @@ def adjust_permissions(provided_path, permission_bits, add=True, onlyfiles=False :param relative: add/remove permissions relative to current permissions (if False, hard set specified permissions) :param ignore_errors: ignore errors that occur when changing permissions (up to a maximum ratio specified by --max-fail-ratio-adjust-permissions configuration option) - :param ignore_root: don’t change the permissions of the root path (provided_path) Add or remove (if add is False) permission_bits from all files (if onlydirs is False) and directories (if onlyfiles is False) in path """ - if not recursive and ignore_root: - _log.info("Not adjusting permissions for %s (no recursion and ignoring root)", provided_path) - return - provided_path = os.path.abspath(provided_path) if recursive: _log.info("Adjusting permissions recursively for %s", provided_path) - if ignore_root: - allpaths = [] - else: - allpaths = [provided_path] + allpaths = [provided_path] for root, dirs, files in os.walk(provided_path): paths = [] if not onlydirs: From 9a936f5a09b6253189e5f0dee71ffe4d5f44872e Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 21 Jun 2025 14:58:03 +0200 Subject: [PATCH 04/13] fix test --- easybuild/framework/easyblock.py | 1 + test/framework/toy_build.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index efec180e4d..32f1a6fc78 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1251,6 +1251,7 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=Fal # empty the installation directory, but never remove it empty_dir(dir_name) self.log.info("Emptied old directory %s", dir_name) + set_gid_sticky_bits(dir_name, set_gid, sticky, recursive=True) else: remove_dir(dir_name) self.log.info("Removed old directory %s", dir_name) diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 914daa306e..d0c9dfea5b 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -798,6 +798,9 @@ def test_toy_gid_sticky_bits(self): self.assertFalse(perms & stat.S_ISVTX, "no sticky bit on %s" % fullpath) # git/sticky bits are set, but only on (re)created directories + # first remove install dir because it is not recreated + toy_installdir = os.path.join(self.test_installpath, *subdirs[3][0]) + remove_dir(toy_installdir) self.run_test_toy_build_with_output(extra_args=['--set-gid-bit', '--sticky-bit']) for subdir, bits_set in subdirs: fullpath = os.path.join(self.test_installpath, *subdir) From 919d5eb7655fec1dcad9280e4a46ab6bcc742d6b Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 21 Jun 2025 15:05:00 +0200 Subject: [PATCH 05/13] oops --- easybuild/framework/easyblock.py | 1 - 1 file changed, 1 deletion(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 32f1a6fc78..efec180e4d 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1251,7 +1251,6 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=Fal # empty the installation directory, but never remove it empty_dir(dir_name) self.log.info("Emptied old directory %s", dir_name) - set_gid_sticky_bits(dir_name, set_gid, sticky, recursive=True) else: remove_dir(dir_name) self.log.info("Removed old directory %s", dir_name) From 0aa808b6fc052f47ee094cf8e7d5533156bbf740 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Thu, 26 Jun 2025 13:12:49 +0200 Subject: [PATCH 06/13] add test for empty_dir --- easybuild/base/testing.py | 4 ++-- easybuild/tools/filetools.py | 2 +- test/framework/filetools.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/easybuild/base/testing.py b/easybuild/base/testing.py index 92789fd4ce..71700794d3 100644 --- a/easybuild/base/testing.py +++ b/easybuild/base/testing.py @@ -114,13 +114,13 @@ def assertEqual(self, a, b, msg=None): raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF]))) from None def assertExists(self, path, msg=None): - """Assert the given path exists""" + """Assert that the given path exists""" if msg is None: msg = "'%s' should exist" % path self.assertTrue(os.path.exists(path), msg) def assertNotExists(self, path, msg=None): - """Assert the given path exists""" + """Assert that the given path does not exist""" if msg is None: msg = "'%s' should not exist" % path self.assertFalse(os.path.exists(path), msg) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index f91c394d3b..db13534d2a 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -380,7 +380,7 @@ def empty_dir(path): """Empty directory at specified path, keeping directory itself intact.""" # early exit in 'dry run' mode if build_option('extended_dry_run'): - dry_run_msg("directory %s removed" % path, silent=build_option('silent')) + dry_run_msg("directory %s emptied" % path, silent=build_option('silent')) return if os.path.exists(path): diff --git a/test/framework/filetools.py b/test/framework/filetools.py index ca2b9bcb47..212dd4587b 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2546,6 +2546,40 @@ def test_extract_file(self): self.assertFalse(stderr) self.assertFalse(stdout) + def test_empty_dir(self): + """Test empty_dir function""" + test_dir = os.path.join(self.test_prefix, 'test123') + testfile = os.path.join(test_dir, 'foo') + testfile_hidden = os.path.join(test_dir, '.foo') + test_link = os.path.join(test_dir, 'foolink') + test_subdir = os.path.join(test_dir, 'foodir') + + ft.mkdir(test_subdir) + ft.write_file(testfile, 'bar') + ft.write_file(testfile_hidden, 'bar') + ft.symlink(testfile, test_link) + ft.empty_dir(test_dir) + self.assertExists(test_dir) + self.assertNotExists(testfile) + self.assertNotExists(testfile_hidden) + self.assertNotExists(test_link) + + # also test behaviour under --dry-run + build_options = { + 'extended_dry_run': True, + 'silent': False, + } + init_config(build_options=build_options) + + self.mock_stdout(True) + ft.mkdir(test_dir) + ft.empty_dir(test_dir) + txt = self.get_stdout() + self.mock_stdout(False) + + regex = re.compile("^directory [^ ]* emptied$") + self.assertTrue(regex.match(txt), f"Pattern '{regex.pattern}' found in: {txt}") + def test_remove(self): """Test remove_file, remove_dir and join remove functions.""" testfile = os.path.join(self.test_prefix, 'foo') From df8d04cacc3275215b91b64cdfdee2861fa9c342 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Thu, 26 Jun 2025 13:46:48 +0200 Subject: [PATCH 07/13] fix --- test/framework/filetools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 212dd4587b..78baa9b8ea 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2554,7 +2554,7 @@ def test_empty_dir(self): test_link = os.path.join(test_dir, 'foolink') test_subdir = os.path.join(test_dir, 'foodir') - ft.mkdir(test_subdir) + ft.mkdir(test_subdir, parents=True) ft.write_file(testfile, 'bar') ft.write_file(testfile_hidden, 'bar') ft.symlink(testfile, test_link) From 2397aeb426a85cd45e3fa8f948d68e563491a46e Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Thu, 14 Aug 2025 11:45:36 +0200 Subject: [PATCH 08/13] first try to remove it, if that fails empty it --- easybuild/framework/easyblock.py | 4 ++-- easybuild/tools/filetools.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index efec180e4d..77f4b76074 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -93,7 +93,7 @@ from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, check_lock from easybuild.tools.filetools import compute_checksum, convert_name, copy_dir, copy_file, create_lock from easybuild.tools.filetools import create_non_existing_paths, create_patch_info, derive_alt_pypi_url, diff_files -from easybuild.tools.filetools import download_file, empty_dir, encode_class_name, extract_file +from easybuild.tools.filetools import download_file, clean_dir, encode_class_name, extract_file from easybuild.tools.filetools import find_backup_name_candidate, get_cwd, get_source_tarball_from_git, is_alt_pypi_url from easybuild.tools.filetools import is_binary, is_parent_path, is_sha256_checksum, mkdir, move_file, move_logs from easybuild.tools.filetools import read_file, remove_dir, remove_file, remove_lock, symlink, verify_checksum @@ -1249,7 +1249,7 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=Fal self.log.info("Copied old directory %s to %s", dir_name, backupdir) if isinstalldir: # empty the installation directory, but never remove it - empty_dir(dir_name) + clean_dir(dir_name) self.log.info("Emptied old directory %s", dir_name) else: remove_dir(dir_name) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index db13534d2a..8322491d81 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -440,6 +440,18 @@ def remove_dir(path): path, max_attempts, errors) +def clean_dir(path): + """ + Try to remove directory at specified path. + If that fails, empty directory instead. + """ + try: + remove_dir(path) + except EasyBuildError: + _log.debug("Failed to remove directory %s, emptying it instead", path) + empty_dir(path) + + def remove(paths): """ Remove single file/directory or list of files and directories From 2c70d86d302c3725305705ea8f91e29619f7b4ca Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Thu, 14 Aug 2025 16:35:26 +0200 Subject: [PATCH 09/13] undo changes in toy_build.py --- easybuild/framework/easyblock.py | 4 ++-- test/framework/toy_build.py | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 77f4b76074..2975d0ef6c 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1175,7 +1175,7 @@ def make_builddir(self): if self.build_in_installdir and self.iter_idx > 0: pass elif self.build_in_installdir: - # building in installation directory, so empty it but don't remove it + # building in installation directory, so clean it self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], isinstalldir=True) else: # make sure we no longer sit in the build directory before removing it. @@ -1248,7 +1248,7 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=Fal copy_dir(dir_name, backupdir) self.log.info("Copied old directory %s to %s", dir_name, backupdir) if isinstalldir: - # empty the installation directory, but never remove it + # clean the installation directory: first try to remove it; if that fails, empty it clean_dir(dir_name) self.log.info("Emptied old directory %s", dir_name) else: diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index d0c9dfea5b..914daa306e 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -798,9 +798,6 @@ def test_toy_gid_sticky_bits(self): self.assertFalse(perms & stat.S_ISVTX, "no sticky bit on %s" % fullpath) # git/sticky bits are set, but only on (re)created directories - # first remove install dir because it is not recreated - toy_installdir = os.path.join(self.test_installpath, *subdirs[3][0]) - remove_dir(toy_installdir) self.run_test_toy_build_with_output(extra_args=['--set-gid-bit', '--sticky-bit']) for subdir, bits_set in subdirs: fullpath = os.path.join(self.test_installpath, *subdir) From 0b8331b64b2f0d2b87ce4446f5f93f0083742d75 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 30 Aug 2025 20:43:41 +0200 Subject: [PATCH 10/13] add test for clean_dir --- easybuild/framework/easyblock.py | 10 +++++----- easybuild/tools/filetools.py | 4 ++-- test/framework/filetools.py | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 2975d0ef6c..fc4ef15a77 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -90,10 +90,10 @@ from easybuild.tools.config import DATA, SOFTWARE from easybuild.tools.environment import restore_env, sanitize_env from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256 -from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, check_lock +from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, check_lock, clean_dir from easybuild.tools.filetools import compute_checksum, convert_name, copy_dir, copy_file, create_lock from easybuild.tools.filetools import create_non_existing_paths, create_patch_info, derive_alt_pypi_url, diff_files -from easybuild.tools.filetools import download_file, clean_dir, encode_class_name, extract_file +from easybuild.tools.filetools import download_file, encode_class_name, extract_file from easybuild.tools.filetools import find_backup_name_candidate, get_cwd, get_source_tarball_from_git, is_alt_pypi_url from easybuild.tools.filetools import is_binary, is_parent_path, is_sha256_checksum, mkdir, move_file, move_logs from easybuild.tools.filetools import read_file, remove_dir, remove_file, remove_lock, symlink, verify_checksum @@ -1176,7 +1176,7 @@ def make_builddir(self): pass elif self.build_in_installdir: # building in installation directory, so clean it - self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], isinstalldir=True) + self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], clean_instead_of_remove=True) else: # make sure we no longer sit in the build directory before removing it. change_dir(self.orig_workdir) @@ -1229,7 +1229,7 @@ def make_installdir(self, dontcreate=None): self.make_dir(self.installdir, self.cfg['cleanupoldinstall'], dontcreateinstalldir=dontcreate, isinstalldir=True) - def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=False): + def make_dir(self, dir_name, clean, dontcreateinstalldir=False, clean_instead_of_remove=False): """ Create the directory. """ @@ -1247,7 +1247,7 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, isinstalldir=Fal backupdir = "%s.%s" % (dir_name, timestamp) copy_dir(dir_name, backupdir) self.log.info("Copied old directory %s to %s", dir_name, backupdir) - if isinstalldir: + if clean_instead_of_remove: # clean the installation directory: first try to remove it; if that fails, empty it clean_dir(dir_name) self.log.info("Emptied old directory %s", dir_name) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 8322491d81..234976bf16 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -447,8 +447,8 @@ def clean_dir(path): """ try: remove_dir(path) - except EasyBuildError: - _log.debug("Failed to remove directory %s, emptying it instead", path) + except EasyBuildError as err: + _log.debug(f"Removing directory {path} failed, will try to empty it instead: {err}") empty_dir(path) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 78baa9b8ea..d16bea41fe 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2563,6 +2563,7 @@ def test_empty_dir(self): self.assertNotExists(testfile) self.assertNotExists(testfile_hidden) self.assertNotExists(test_link) + self.assertNotExists(test_subdir) # also test behaviour under --dry-run build_options = { @@ -2656,6 +2657,25 @@ def test_remove(self): ft.adjust_permissions(self.test_prefix, stat.S_IWUSR, add=True) + def test_clean_dir(self): + """Test clean_dir function""" + test_dir = os.path.join(self.test_prefix, 'test123') + testfile = os.path.join(test_dir, 'foo') + testfile_hidden = os.path.join(test_dir, '.foo') + test_link = os.path.join(test_dir, 'foolink') + test_subdir = os.path.join(test_dir, 'foodir') + + ft.mkdir(test_subdir, parents=True) + ft.write_file(testfile, 'bar') + ft.write_file(testfile_hidden, 'bar') + ft.symlink(testfile, test_link) + ft.clean_dir(test_dir) + ft.mkdir(test_dir, parents=True) + self.assertNotExists(testfile) + self.assertNotExists(testfile_hidden) + self.assertNotExists(test_link) + self.assertNotExists(test_subdir) + def test_index_functions(self): """Test *_index functions.""" From 0519d9f4a4d71664e2bb70f14cadf1df965b67b4 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sat, 30 Aug 2025 20:48:32 +0200 Subject: [PATCH 11/13] fix --- easybuild/framework/easyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index fc4ef15a77..ad592e83c6 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1227,7 +1227,7 @@ def make_installdir(self, dontcreate=None): self.cfg['keeppreviousinstall'] = True dontcreate = (dontcreate is None and self.cfg['dontcreateinstalldir']) or dontcreate self.make_dir(self.installdir, self.cfg['cleanupoldinstall'], dontcreateinstalldir=dontcreate, - isinstalldir=True) + clean_instead_of_remove=True) def make_dir(self, dir_name, clean, dontcreateinstalldir=False, clean_instead_of_remove=False): """ From 49fd013ce820293243334b99507a99cec9bdcbc1 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Wed, 3 Sep 2025 19:38:52 +0200 Subject: [PATCH 12/13] make suggested changes --- easybuild/framework/easyblock.py | 18 +++++++++------- easybuild/tools/filetools.py | 36 ++++++++++---------------------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index ad592e83c6..9482721994 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1174,13 +1174,11 @@ def make_builddir(self): # see https://github.com/easybuilders/easybuild-framework/issues/2556 if self.build_in_installdir and self.iter_idx > 0: pass - elif self.build_in_installdir: - # building in installation directory, so clean it - self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], clean_instead_of_remove=True) else: # make sure we no longer sit in the build directory before removing it. change_dir(self.orig_workdir) - self.make_dir(self.builddir, self.cfg['cleanupoldbuild']) + # if we’re building in installation directory, clean it + self.make_dir(self.builddir, self.cfg['cleanupoldbuild'], clean_instead_of_remove=self.build_in_installdir) trace_msg("build dir: %s" % self.builddir) @@ -1245,15 +1243,19 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, clean_instead_of self.log.info("Creating backup of directory %s...", dir_name) timestamp = time.strftime("%Y%m%d-%H%M%S") backupdir = "%s.%s" % (dir_name, timestamp) - copy_dir(dir_name, backupdir) - self.log.info("Copied old directory %s to %s", dir_name, backupdir) + if clean_instead_of_remove: + copy_dir(dir_name, backupdir) + self.log.info(f"Copied old directory {dir_name} to {backupdir}") + else: + move_file(dir_name, backupdir) + self.log.info(f"Moved old directory {dir_name} to {backupdir}") if clean_instead_of_remove: # clean the installation directory: first try to remove it; if that fails, empty it clean_dir(dir_name) - self.log.info("Emptied old directory %s", dir_name) + self.log.info(f"Cleaned old directory {dir_name}") else: remove_dir(dir_name) - self.log.info("Removed old directory %s", dir_name) + self.log.info(f"Removed old directory {dir_name}") if dontcreateinstalldir: olddir = dir_name diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 234976bf16..5f5c9f9789 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -380,34 +380,20 @@ def empty_dir(path): """Empty directory at specified path, keeping directory itself intact.""" # early exit in 'dry run' mode if build_option('extended_dry_run'): - dry_run_msg("directory %s emptied" % path, silent=build_option('silent')) + dry_run_msg(f"directory {path} emptied", silent=build_option('silent')) return if os.path.exists(path): - ok = False - errors = [] - # Try multiple times to cater for temporary failures on e.g. NFS mounted paths - max_attempts = 3 - for i in range(0, max_attempts): - try: - for item in os.listdir(path): - subpath = os.path.join(path, item) - if os.path.isfile(subpath) or os.path.islink(subpath): - os.remove(subpath) - elif os.path.isdir(subpath): - shutil.rmtree(subpath) - ok = True - except OSError as err: - _log.debug("Failed to empty path %s at attempt %d: %s" % (path, i, err)) - errors.append(err) - time.sleep(2) - # make sure write permissions are enabled on entire directory - adjust_permissions(path, stat.S_IWUSR, add=True, recursive=True) - if ok: - _log.info("Path %s successfully emptied." % path) - else: - raise EasyBuildError("Failed to empty directory %s even after %d attempts.\nReasons: %s", - path, max_attempts, errors) + try: + for item in os.listdir(path): + subpath = os.path.join(path, item) + if os.path.isfile(subpath) or os.path.islink(subpath): + remove_file(subpath) + elif os.path.isdir(subpath): + remove_dir(subpath) + _log.info(f"Path {path} successfully emptied.") + except OSError as err: + raise EasyBuildError(f"Failed to empty directory {path}: {err}") def remove_dir(path): From 66ae73dc1e1751cf68ad93b4aad70c0799205741 Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Wed, 3 Sep 2025 20:16:40 +0200 Subject: [PATCH 13/13] remove_dir only if clean --- easybuild/framework/easyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 9482721994..5886733ff9 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1253,7 +1253,7 @@ def make_dir(self, dir_name, clean, dontcreateinstalldir=False, clean_instead_of # clean the installation directory: first try to remove it; if that fails, empty it clean_dir(dir_name) self.log.info(f"Cleaned old directory {dir_name}") - else: + elif clean: remove_dir(dir_name) self.log.info(f"Removed old directory {dir_name}")