From 4adb577d645159c989206be6efde6519bf1699eb Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Mon, 16 Sep 2024 14:59:28 +0100 Subject: [PATCH 01/20] fix issue when copying read-only files using shutil.copy2 --- easybuild/tools/filetools.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 06ca1dddb5..53cd97e876 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2435,8 +2435,15 @@ def copy_file(path, target_path, force_in_dry_run=False): else: mkdir(os.path.dirname(target_path), parents=True) if path_exists: - shutil.copy2(path, target_path) - _log.info("%s copied to %s", path, target_path) + try: + # on at least some systems, copying read-only files with shutil.copy2() spits a PermissionError + # but the copy still succeeds + shutil.copy2(path, target_path) + _log.info("%s copied to %s", path, target_path) + except PermissionError as err: + # double-check whether the copy actually succeeded + if not os.path.exists(target_path): + raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) elif os.path.islink(path): if os.path.isdir(target_path): target_path = os.path.join(target_path, os.path.basename(path)) From 8cba9e9fc35fd852eb82f6a68bb61fbde96fbf2d Mon Sep 17 00:00:00 2001 From: Jasper Grimm <65227842+jfgrimm@users.noreply.github.com> Date: Mon, 16 Sep 2024 15:17:07 +0100 Subject: [PATCH 02/20] log when ignoring permissions error Co-authored-by: ocaisa --- easybuild/tools/filetools.py | 1 + 1 file changed, 1 insertion(+) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 53cd97e876..5408a8d66a 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2444,6 +2444,7 @@ def copy_file(path, target_path, force_in_dry_run=False): # double-check whether the copy actually succeeded if not os.path.exists(target_path): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) + _log.info("%s copied to %s ignoring permissions error: %s", path, target_path, err) elif os.path.islink(path): if os.path.isdir(target_path): target_path = os.path.join(target_path, os.path.basename(path)) From 55b4c8b39d8f5cba9009415301ce8b6f79190f3f Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Tue, 17 Sep 2024 16:28:07 +0100 Subject: [PATCH 03/20] python2 support --- easybuild/tools/filetools.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 5408a8d66a..415727de6e 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -59,7 +59,7 @@ from functools import partial from easybuild.base import fancylogger -from easybuild.tools import run +from easybuild.tools import LooseVersion, run # import build_log must stay, to use of EasyBuildLog from easybuild.tools.build_log import EasyBuildError, dry_run_msg, print_msg, print_warning from easybuild.tools.config import DEFAULT_WAIT_ON_LOCK_INTERVAL, ERROR, GENERIC_EASYBLOCK_PKG, IGNORE, WARN @@ -2440,11 +2440,16 @@ def copy_file(path, target_path, force_in_dry_run=False): # but the copy still succeeds shutil.copy2(path, target_path) _log.info("%s copied to %s", path, target_path) - except PermissionError as err: + except OSError as err: + # catch the more general OSError instead of PermissionError, since python2 doesn't support + # PermissionError + if LooseVersion(platform.python_version()) >= LooseVersion('3'): + if not isinstance(err, PermissionError): + raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) # double-check whether the copy actually succeeded - if not os.path.exists(target_path): + if not os.path.exists(target_path) or not os.path.samefile(path, target_path): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) - _log.info("%s copied to %s ignoring permissions error: %s", path, target_path, err) + _log.info("%s copied to %s, ignoring permissions error: %s", path, target_path, err) elif os.path.islink(path): if os.path.isdir(target_path): target_path = os.path.join(target_path, os.path.basename(path)) From a198ad5527cbb7868c89bddf15310382a5e39b3d Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Tue, 17 Sep 2024 16:30:32 +0100 Subject: [PATCH 04/20] fix import --- easybuild/tools/filetools.py | 1 + 1 file changed, 1 insertion(+) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 415727de6e..0d54ed1000 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -47,6 +47,7 @@ import inspect import itertools import os +import platform import re import shutil import signal From 5d1f2ce7c878992f9219546298731c7d80f860ee Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Tue, 17 Sep 2024 17:24:02 +0100 Subject: [PATCH 05/20] actually compare file contents in case of permissions error --- easybuild/tools/filetools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 0d54ed1000..285c290a14 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -42,6 +42,7 @@ """ import datetime import difflib +import filecmp import glob import hashlib import inspect @@ -2448,7 +2449,7 @@ def copy_file(path, target_path, force_in_dry_run=False): if not isinstance(err, PermissionError): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) # double-check whether the copy actually succeeded - if not os.path.exists(target_path) or not os.path.samefile(path, target_path): + if not os.path.exists(target_path) or not filecmp.cmp(path, target_path, shallow=False): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) _log.info("%s copied to %s, ignoring permissions error: %s", path, target_path, err) elif os.path.islink(path): From 7bf4073025c6388e1a7c0d8dfa7906ea382c7033 Mon Sep 17 00:00:00 2001 From: casparvl casparvl Date: Wed, 18 Sep 2024 10:20:40 +0000 Subject: [PATCH 06/20] Make sure the run_cmd respects sysroot when picking the shell, i.e. use sysroot/bin/bash instead of /bin/bash if sysroot is set --- easybuild/tools/run.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 30f413be55..eee95048b7 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -226,7 +226,11 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True if cmd_log: cmd_log.write("# output for command: %s\n\n" % cmd_msg) - exec_cmd = "/bin/bash" + sysroot = build_option('sysroot') + if sysroot: + exec_cmd = "%s/bin/bash" % sysroot + else: + exec_cmd = "/bin/bash" if not shell: if isinstance(cmd, list): From 73a53a8bae7ca6b3dd1b172c620801eb44aa7c54 Mon Sep 17 00:00:00 2001 From: casparvl casparvl Date: Wed, 18 Sep 2024 14:16:21 +0000 Subject: [PATCH 07/20] Make sure we only check for sysroot after set_up_configuration is called. Any command run before that with run_cmd should use the regular /bin/bash --- easybuild/tools/run.py | 14 +++++++++----- easybuild/tools/systemtools.py | 33 +++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index eee95048b7..0d34e44b3d 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -134,7 +134,7 @@ def get_output_from_process(proc, read_size=None, asynchronous=False): @run_cmd_cache def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True, log_output=False, path=None, force_in_dry_run=False, verbose=True, shell=None, trace=True, stream_output=None, asynchronous=False, - with_hooks=True): + with_hooks=True, with_sysroot=True): """ Run specified command (in a subshell) :param cmd: command to run @@ -152,6 +152,7 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True :param stream_output: enable streaming command output to stdout :param asynchronous: run command asynchronously (returns subprocess.Popen instance if set to True) :param with_hooks: trigger pre/post run_shell_cmd hooks (if defined) + :param with_sysroot: prepend sysroot to exec_cmd (if defined) """ cwd = os.getcwd() @@ -226,11 +227,14 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True if cmd_log: cmd_log.write("# output for command: %s\n\n" % cmd_msg) - sysroot = build_option('sysroot') - if sysroot: - exec_cmd = "%s/bin/bash" % sysroot + if with_sysroot: + sysroot = build_option('sysroot') + if sysroot: + exec_cmd = "%s/bin/bash" % sysroot + else: + exec_cmd = "/bin/bash" else: - exec_cmd = "/bin/bash" + exec_cmd = "/bin/bash" if not shell: if isinstance(cmd, list): diff --git a/easybuild/tools/systemtools.py b/easybuild/tools/systemtools.py index 24e910fb94..68c42fe416 100644 --- a/easybuild/tools/systemtools.py +++ b/easybuild/tools/systemtools.py @@ -275,7 +275,8 @@ def get_avail_core_count(): core_cnt = int(sum(sched_getaffinity())) else: # BSD-type systems - out, _ = run_cmd('sysctl -n hw.ncpu', force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, _ = run_cmd('sysctl -n hw.ncpu', force_in_dry_run=True, trace=False, stream_output=False, + with_hooks=False, with_sysroot=False) try: if int(out) > 0: core_cnt = int(out) @@ -312,7 +313,8 @@ def get_total_memory(): elif os_type == DARWIN: cmd = "sysctl -n hw.memsize" _log.debug("Trying to determine total memory size on Darwin via cmd '%s'", cmd) - out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False, + with_sysroot=False) if ec == 0: memtotal = int(out.strip()) // (1024**2) @@ -394,7 +396,8 @@ def get_cpu_vendor(): elif os_type == DARWIN: cmd = "sysctl -n machdep.cpu.vendor" - out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, log_ok=False, with_hooks=False) + out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, log_ok=False, + with_hooks=False, with_sysroot=False) out = out.strip() if ec == 0 and out in VENDOR_IDS: vendor = VENDOR_IDS[out] @@ -402,7 +405,7 @@ def get_cpu_vendor(): else: cmd = "sysctl -n machdep.cpu.brand_string" out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, log_ok=False, - with_hooks=False) + with_hooks=False, with_sysroot=False) out = out.strip().split(' ')[0] if ec == 0 and out in CPU_VENDORS: vendor = out @@ -505,7 +508,8 @@ def get_cpu_model(): elif os_type == DARWIN: cmd = "sysctl -n machdep.cpu.brand_string" - out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False, + with_sysroot=False) if ec == 0: model = out.strip() _log.debug("Determined CPU model on Darwin using cmd '%s': %s" % (cmd, model)) @@ -550,7 +554,8 @@ def get_cpu_speed(): elif os_type == DARWIN: cmd = "sysctl -n hw.cpufrequency_max" _log.debug("Trying to determine CPU frequency on Darwin via cmd '%s'" % cmd) - out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False, + with_sysroot=False) out = out.strip() cpu_freq = None if ec == 0 and out: @@ -599,7 +604,7 @@ def get_cpu_features(): cmd = "sysctl -n machdep.cpu.%s" % feature_set _log.debug("Trying to determine CPU features on Darwin via cmd '%s'", cmd) out, ec = run_cmd(cmd, force_in_dry_run=True, trace=False, stream_output=False, log_ok=False, - with_hooks=False) + with_hooks=False, with_sysroot=False) if ec == 0: cpu_feat.extend(out.strip().lower().split()) @@ -626,8 +631,8 @@ def get_gpu_info(): try: cmd = "nvidia-smi --query-gpu=gpu_name,driver_version --format=csv,noheader" _log.debug("Trying to determine NVIDIA GPU info on Linux via cmd '%s'", cmd) - out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, - force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, force_in_dry_run=True, + trace=False, stream_output=False, with_hooks=False, with_sysroot=False) if ec == 0: for line in out.strip().split('\n'): nvidia_gpu_info = gpu_info.setdefault('NVIDIA', {}) @@ -645,15 +650,15 @@ def get_gpu_info(): try: cmd = "rocm-smi --showdriverversion --csv" _log.debug("Trying to determine AMD GPU driver on Linux via cmd '%s'", cmd) - out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, - force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, force_in_dry_run=True, + trace=False, stream_output=False, with_hooks=False, with_sysroot=False) if ec == 0: amd_driver = out.strip().split('\n')[1].split(',')[1] cmd = "rocm-smi --showproductname --csv" _log.debug("Trying to determine AMD GPU info on Linux via cmd '%s'", cmd) - out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, - force_in_dry_run=True, trace=False, stream_output=False, with_hooks=False) + out, ec = run_cmd(cmd, simple=False, log_ok=False, log_all=False, force_in_dry_run=True, + trace=False, stream_output=False, with_hooks=False, with_sysroot=False) if ec == 0: for line in out.strip().split('\n')[1:]: amd_card_series = line.split(',')[1] @@ -900,7 +905,7 @@ def get_tool_version(tool, version_option='--version', ignore_ec=False): Output is returned as a single-line string (newlines are replaced by '; '). """ out, ec = run_cmd(' '.join([tool, version_option]), simple=False, log_ok=False, force_in_dry_run=True, - trace=False, stream_output=False, with_hooks=False) + trace=False, stream_output=False, with_hooks=False, with_sysroot=False) if not ignore_ec and ec: _log.warning("Failed to determine version of %s using '%s %s': %s" % (tool, tool, version_option, out)) return UNKNOWN From 347e64cb8a0a8643dcb0a6e7526c5d612cec07b6 Mon Sep 17 00:00:00 2001 From: casparvl casparvl Date: Wed, 18 Sep 2024 14:22:57 +0000 Subject: [PATCH 08/20] Also add with_sysroot=False here --- easybuild/tools/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index f48433e23c..fd22e384e5 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1969,7 +1969,7 @@ def set_tmpdir(tmpdir=None, raise_error=False): os.close(fd) os.chmod(tmptest_file, 0o700) if not run_cmd(tmptest_file, simple=True, log_ok=False, regexp=False, force_in_dry_run=True, trace=False, - stream_output=False, with_hooks=False): + stream_output=False, with_hooks=False, with_sysroot=False): msg = "The temporary directory (%s) does not allow to execute files. " % tempfile.gettempdir() msg += "This can cause problems in the build process, consider using --tmpdir." if raise_error: From 165fe7910169ef8c7b0e8ff995d5d584e53d6a25 Mon Sep 17 00:00:00 2001 From: casparvl casparvl Date: Wed, 18 Sep 2024 14:29:35 +0000 Subject: [PATCH 09/20] Fix indent --- easybuild/tools/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 0d34e44b3d..5fb738f347 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -234,7 +234,7 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True else: exec_cmd = "/bin/bash" else: - exec_cmd = "/bin/bash" + exec_cmd = "/bin/bash" if not shell: if isinstance(cmd, list): From df9827d88dc79f8f329fffc0d7b8d0301cf843dc Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Wed, 18 Sep 2024 15:59:16 +0100 Subject: [PATCH 10/20] more explicit python version checking to ignore permissions error --- easybuild/tools/filetools.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 285c290a14..9e7214a666 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2438,20 +2438,25 @@ def copy_file(path, target_path, force_in_dry_run=False): mkdir(os.path.dirname(target_path), parents=True) if path_exists: try: - # on at least some systems, copying read-only files with shutil.copy2() spits a PermissionError - # but the copy still succeeds + # on filesystems that support extended file attributes, copying read-only files with + # shutil.copy2() will give a PermissionError, when using Python < 3.7 + # see https://bugs.python.org/issue24538 shutil.copy2(path, target_path) _log.info("%s copied to %s", path, target_path) except OSError as err: # catch the more general OSError instead of PermissionError, since python2 doesn't support # PermissionError - if LooseVersion(platform.python_version()) >= LooseVersion('3'): + pyver = LooseVersion(platform.python_version()) + if pyver >= LooseVersion('3.7'): + raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) + elif LooseVersion('3.7') > pyver >= LooseVersion('3'): if not isinstance(err, PermissionError): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) # double-check whether the copy actually succeeded if not os.path.exists(target_path) or not filecmp.cmp(path, target_path, shallow=False): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) - _log.info("%s copied to %s, ignoring permissions error: %s", path, target_path, err) + msg = "%s copied to %s, ignoring permissions error (likely due to https://bugs.python.org/issue24538): %s" + _log.info(msg, path, target_path, err) elif os.path.islink(path): if os.path.isdir(target_path): target_path = os.path.join(target_path, os.path.basename(path)) From 202ae522cf741e25f67d0cdacbdf05509bcb5e84 Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Wed, 18 Sep 2024 16:00:44 +0100 Subject: [PATCH 11/20] long line --- easybuild/tools/filetools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 9e7214a666..6c4dd9eee2 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2455,7 +2455,8 @@ def copy_file(path, target_path, force_in_dry_run=False): # double-check whether the copy actually succeeded if not os.path.exists(target_path) or not filecmp.cmp(path, target_path, shallow=False): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) - msg = "%s copied to %s, ignoring permissions error (likely due to https://bugs.python.org/issue24538): %s" + msg = ("%s copied to %s, ignoring permissions error (likely due to " + "https://bugs.python.org/issue24538): %s") _log.info(msg, path, target_path, err) elif os.path.islink(path): if os.path.isdir(target_path): From 4a4312e935fd3f4dc14478b20e7a5e0ab2f25f15 Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Wed, 18 Sep 2024 17:06:37 +0100 Subject: [PATCH 12/20] manually copy xattrs where appropriate --- easybuild/tools/filetools.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 6c4dd9eee2..fd9035e6e1 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2446,18 +2446,33 @@ def copy_file(path, target_path, force_in_dry_run=False): except OSError as err: # catch the more general OSError instead of PermissionError, since python2 doesn't support # PermissionError + if not os.stat(path).st_mode & stat.S_IWUSR: + # failure not due to read-only file + raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) + pyver = LooseVersion(platform.python_version()) if pyver >= LooseVersion('3.7'): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) elif LooseVersion('3.7') > pyver >= LooseVersion('3'): if not isinstance(err, PermissionError): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) + # double-check whether the copy actually succeeded if not os.path.exists(target_path) or not filecmp.cmp(path, target_path, shallow=False): raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) - msg = ("%s copied to %s, ignoring permissions error (likely due to " - "https://bugs.python.org/issue24538): %s") - _log.info(msg, path, target_path, err) + + try: + # re-enable user write permissions in target, copy xattrs, then remove write perms again + adjust_permissions(target_path, stat.I_IWUSR) + shutil._copyxattr(path, target_path) + adjust_permissions(target_path, stat.I_IWUSR, add=False) + except OSError as err: + raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) + + msg = ("Failed to copy extended attributes from file %s to %s, due to a bug in shutil (see " + "https://bugs.python.org/issue24538). Copy successful with workaround.") + _log.info(msg, path, target_path) + elif os.path.islink(path): if os.path.isdir(target_path): target_path = os.path.join(target_path, os.path.basename(path)) From 150d7c3dd122302417d35103f25e7ca4fd9efaa9 Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Wed, 18 Sep 2024 17:13:16 +0100 Subject: [PATCH 13/20] typo --- easybuild/tools/filetools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index fd9035e6e1..30d4ec34c4 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2463,9 +2463,9 @@ def copy_file(path, target_path, force_in_dry_run=False): try: # re-enable user write permissions in target, copy xattrs, then remove write perms again - adjust_permissions(target_path, stat.I_IWUSR) + adjust_permissions(target_path, stat.S_IWUSR) shutil._copyxattr(path, target_path) - adjust_permissions(target_path, stat.I_IWUSR, add=False) + adjust_permissions(target_path, stat.S_IWUSR, add=False) except OSError as err: raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) From f7c3cb12abd73a6e769ff2be28d45535ccd9dd26 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Sep 2024 18:33:39 +0200 Subject: [PATCH 14/20] be a bit more careful with sysroot in run_cmd, add logging + add dedicated test for run_cmd using with_sysroot=True --- easybuild/tools/run.py | 15 ++++++++++----- test/framework/run.py | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 5fb738f347..f8d034f981 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -227,14 +227,17 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True if cmd_log: cmd_log.write("# output for command: %s\n\n" % cmd_msg) + exec_cmd = "/bin/bash" + + # if EasyBuild is configured to use an alternate sysroot, + # we should also run shell commands using the bash shell provided in there, + # since /bin/bash may not be compatible with the alternate sysroot if with_sysroot: sysroot = build_option('sysroot') if sysroot: - exec_cmd = "%s/bin/bash" % sysroot - else: - exec_cmd = "/bin/bash" - else: - exec_cmd = "/bin/bash" + sysroot_bin_bash = os.path.join(sysroot, 'bin', 'bash') + if os.path.exists(sysroot_bin_bash): + exec_cmd = sysroot_bin_bash if not shell: if isinstance(cmd, list): @@ -245,6 +248,8 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True else: raise EasyBuildError("Don't know how to prefix with /usr/bin/env for commands of type %s", type(cmd)) + _log.info("Using %s as shell for running cmd: %s", exec_cmd, cmd) + if with_hooks: hooks = load_hooks(build_option('hooks')) hook_res = run_hook(RUN_SHELL_CMD, hooks, pre_step_hook=True, args=[cmd], kwargs={'work_dir': os.getcwd()}) diff --git a/test/framework/run.py b/test/framework/run.py index bab4391cf6..bb2e5c0558 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -796,6 +796,31 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): ]) self.assertEqual(stdout, expected_stdout) + def test_run_cmd_sysroot(self): + """Test with_sysroot option of run_cmd function.""" + + # put fake /bin/bash in place that will be picked up when using run_cmd with with_sysroot=True + bin_bash = os.path.join(self.test_prefix, 'bin', 'bash') + bin_bash_txt = '\n'.join([ + "#!/bin/bash", + "echo 'Hi there I am a fake /bin/bash in %s'" % self.test_prefix, + '/bin/bash "$@"', + ]) + write_file(bin_bash, bin_bash_txt) + adjust_permissions(bin_bash, stat.S_IXUSR) + + update_build_option('sysroot', self.test_prefix) + + (out, ec) = run_cmd("echo hello") + self.assertEqual(ec, 0) + self.assertTrue(out.startswith("Hi there I am a fake /bin/bash in")) + self.assertTrue(out.endswith("\nhello\n")) + + # picking up on alternate sysroot is enabled by default, but can be disabled via with_sysroot=False + (out, ec) = run_cmd("echo hello", with_sysroot=False) + self.assertEqual(ec, 0) + self.assertEqual(out, "hello\n") + def suite(): """ returns all the testcases in this module """ From a3b26f6aead3ebc87478a530ec5ca1522482da47 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Sep 2024 19:21:12 +0200 Subject: [PATCH 15/20] implement test for copy_file to copy read-only file with extended attributes --- test/framework/filetools.py | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 1a0294a02f..f6dd6fd7ac 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -32,6 +32,7 @@ @author: Maxime Boissonneault (Compute Canada, Universite Laval) """ import datetime +import filecmp import glob import logging import os @@ -51,6 +52,8 @@ from easybuild.tools.config import IGNORE, ERROR, build_option, update_build_option from easybuild.tools.multidiff import multidiff from easybuild.tools.py2vs3 import StringIO, std_urllib +from easybuild.tools.run import run_cmd +from easybuild.tools.systemtools import LINUX, get_os_type class FileToolsTest(EnhancedTestCase): @@ -1912,6 +1915,47 @@ def test_copy_file(self): # However, if we add 'force_in_dry_run=True' it should throw an exception self.assertErrorRegex(EasyBuildError, "Could not copy *", ft.copy_file, src, target, force_in_dry_run=True) + def test_copy_file_xattr(self): + """Test copying a file with extended attributes using copy_file.""" + # test copying a read-only files with extended attributes set + # first, create a special file with extended attributes + special_file = os.path.join(self.test_prefix, 'special.txt') + ft.write_file(special_file, 'special') + # make read-only, and set extended attributes + attr = ft.which('attr') + xattr = ft.which('xattr') + # try to attr (Linux) or xattr (macOS) to set extended attributes foo=bar + cmd = None + if attr: + cmd = "attr -s foo -V bar %s" % special_file + elif xattr: + cmd = "xattr -w foo bar %s" % special_file + + if cmd: + (_, ec) = run_cmd(cmd, simple=False, log_all=False, log_ok=False) + + # need to make file read-only after setting extended attribute + ft.adjust_permissions(special_file, stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH, add=False) + + # only proceed if setting extended attribute worked + if ec == 0: + target = os.path.join(self.test_prefix, 'copy.txt') + ft.copy_file(special_file, target) + self.assertTrue(os.path.exists(target)) + self.assertTrue(filecmp.cmp(special_file, target, shallow=False)) + + # only verify wheter extended attributes were also copied on Linux, + # since shutil.copy2 doesn't copy them on macOS; + # see warning at https://docs.python.org/3/library/shutil.html + if get_os_type() == LINUX: + if attr: + cmd = "attr -g foo %s" % target + else: + cmd = "xattr -l %s" % target + (out, ec) = run_cmd(cmd, simple=False, log_all=False, log_ok=False) + self.assertEqual(ec, 0) + self.assertTrue(out.endswith('\nbar\n')) + def test_copy_files(self): """Test copy_files function.""" test_ecs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') From 23b5b787caac930d3b408fc03f19fa51ba9051c4 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Sep 2024 20:57:59 +0200 Subject: [PATCH 16/20] fix condition in copy_file when hitting PermissionError when copying read-only file --- easybuild/tools/filetools.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 30d4ec34c4..973f83e1f5 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2443,11 +2443,11 @@ def copy_file(path, target_path, force_in_dry_run=False): # see https://bugs.python.org/issue24538 shutil.copy2(path, target_path) _log.info("%s copied to %s", path, target_path) + # catch the more general OSError instead of PermissionError, + # since Python 2.7 doesn't support PermissionError except OSError as err: - # catch the more general OSError instead of PermissionError, since python2 doesn't support - # PermissionError - if not os.stat(path).st_mode & stat.S_IWUSR: - # failure not due to read-only file + # if file is writable (not read-only), then we give up since it's not a simple permission error + if os.path.exists(target_path) and os.stat(target_path).st_mode & stat.S_IWUSR: raise EasyBuildError("Failed to copy file %s to %s: %s", path, target_path, err) pyver = LooseVersion(platform.python_version()) From 5e7531e7e2e8355572dc2b82fde0375609a3b123 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 21 Sep 2024 13:34:30 +0200 Subject: [PATCH 17/20] set $LMOD_TERSE_DECORATIONS to 'no' to avoid additional info in output produced by 'ml --terse avail' --- easybuild/tools/modules.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 7318be4f4f..8929ccc560 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1448,6 +1448,9 @@ def __init__(self, *args, **kwargs): setvar('LMOD_REDIRECT', 'no', verbose=False) # disable extended defaults within Lmod (introduced and set as default in Lmod 8.0.7) setvar('LMOD_EXTENDED_DEFAULT', 'no', verbose=False) + # disabled decorations in "ml --terse avail" output + # (introduced in Lmod 8.8, see also https://github.com/TACC/Lmod/issues/690) + setvar('LMOD_TERSE_DECORATIONS', 'no', verbose=False) super(Lmod, self).__init__(*args, **kwargs) version = StrictVersion(self.version) From c2c280ec15dd89c6932fc0edb9799197c2946a0e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 21 Sep 2024 17:14:09 +0200 Subject: [PATCH 18/20] prepare release notes for EasyBuild v4.9.4 + bump version to 4.9.4 --- RELEASE_NOTES | 12 ++++++++++++ easybuild/tools/version.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 7f7ade0c1e..4eed2c680c 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -4,6 +4,18 @@ For more detailed information, please see the git log. These release notes can also be consulted at https://easybuild.readthedocs.io/en/latest/Release_notes.html. +v4.9.4 (22 september 2024) +-------------------------- + +update/bugfix release + +- various enhancements, including: + - set $LMOD_TERSE_DECORATIONS to 'no' to avoid additional info in output produced by 'ml --terse avail' (#4648) +- various bug fixes, including: + - implement workaround for permission error when copying read-only files that have extended attributes set and using Python 3.6 (#4642) + - take into account alternate sysroot for /bin/bash used by run_cmd (#4646) + + v4.9.3 (14 September 2024) -------------------------- diff --git a/easybuild/tools/version.py b/easybuild/tools/version.py index 6f8eefd00e..681233536b 100644 --- a/easybuild/tools/version.py +++ b/easybuild/tools/version.py @@ -45,7 +45,7 @@ # recent setuptools versions will *TRANSFORM* something like 'X.Y.Zdev' into 'X.Y.Z.dev0', with a warning like # UserWarning: Normalizing '2.4.0dev' to '2.4.0.dev0' # This causes problems further up the dependency chain... -VERSION = LooseVersion('4.9.4.dev0') +VERSION = LooseVersion('4.9.4') UNKNOWN = 'UNKNOWN' UNKNOWN_EASYBLOCKS_VERSION = '0.0.UNKNOWN.EASYBLOCKS' From a3bdfefdb97b59f6b43f71771a02928f46e31578 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sun, 22 Sep 2024 18:21:58 +0200 Subject: [PATCH 19/20] bump version to 4.9.5dev --- easybuild/tools/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/version.py b/easybuild/tools/version.py index 681233536b..bf802022b9 100644 --- a/easybuild/tools/version.py +++ b/easybuild/tools/version.py @@ -45,7 +45,7 @@ # recent setuptools versions will *TRANSFORM* something like 'X.Y.Zdev' into 'X.Y.Z.dev0', with a warning like # UserWarning: Normalizing '2.4.0dev' to '2.4.0.dev0' # This causes problems further up the dependency chain... -VERSION = LooseVersion('4.9.4') +VERSION = LooseVersion('4.9.5.dev0') UNKNOWN = 'UNKNOWN' UNKNOWN_EASYBLOCKS_VERSION = '0.0.UNKNOWN.EASYBLOCKS' From 4b7124f3977bf795c41bf02b53f7ff5757bfca57 Mon Sep 17 00:00:00 2001 From: Adam Huffman Date: Sun, 22 Sep 2024 21:06:28 +0100 Subject: [PATCH 20/20] Update RELEASE_NOTES Minor typo fix --- RELEASE_NOTES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 4eed2c680c..d531df52d7 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -4,7 +4,7 @@ For more detailed information, please see the git log. These release notes can also be consulted at https://easybuild.readthedocs.io/en/latest/Release_notes.html. -v4.9.4 (22 september 2024) +v4.9.4 (22 September 2024) -------------------------- update/bugfix release