From 4adb577d645159c989206be6efde6519bf1699eb Mon Sep 17 00:00:00 2001 From: jfgrimm Date: Mon, 16 Sep 2024 14:59:28 +0100 Subject: [PATCH 01/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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/55] 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 From 58f39ef468b052b7c36a4d3123d53146d66e642b Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 23 Sep 2024 18:05:19 +0200 Subject: [PATCH 21/55] relax error pattern used in test_run_shell_cmd_delete_cwd so test also works on macOS --- test/framework/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/run.py b/test/framework/run.py index b2abd77b73..e26373f6e6 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -2087,7 +2087,7 @@ def test_run_shell_cmd_delete_cwd(self): f"rm -rf {workdir} && echo 'Working directory removed.'" ) - error_pattern = rf"Failed to return to {workdir} after executing command" + error_pattern = rf"Failed to return to .*/{os.path.basename(self.test_prefix)}/workdir after executing command" mkdir(workdir, parents=True) with self.mocked_stdout_stderr(): From 27c32ef91b174bf31777c9a4e6c57eebbbd0392d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 22 May 2024 19:12:03 +0200 Subject: [PATCH 22/55] Add option for preferring EBPYTHONPREFIXES --- easybuild/framework/easyconfig/default.py | 1 + easybuild/tools/config.py | 1 + easybuild/tools/options.py | 2 ++ 3 files changed, 4 insertions(+) diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index 438bcea4e1..43f6093588 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -115,6 +115,7 @@ 'patches': [[], "List of patches to apply", BUILD], 'prebuildopts': ['', 'Extra options pre-passed to build command.', BUILD], 'preconfigopts': ['', 'Extra options pre-passed to configure.', BUILD], + 'prefer_ebpythonprefixes': [True, "Prefer EBPYTHONPREFIXES over PYTHONPATH when possible.", BUILD], 'preinstallopts': ['', 'Extra prefix options for installation.', BUILD], 'pretestopts': ['', 'Extra prefix options for test.', BUILD], 'postinstallcmds': [[], 'Commands to run after the install step.', BUILD], diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 13f9722bce..67473b4dc0 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -336,6 +336,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'modules_tool_version_check', 'mpi_tests', 'pre_create_installdir', + 'prefer_ebpythonprefixes', 'show_progress_bar', 'trace', ], diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index a61daa6d1c..c5fc72061d 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -490,6 +490,8 @@ def override_options(self): None, 'store_true', False), 'pre-create-installdir': ("Create installation directory before submitting build jobs", None, 'store_true', True), + 'prefer-ebpythonprefixes': (("Prefer EBPYTHONPREFIXES over PYTHONPATH when possible"), + None, 'store_true', True), 'pretend': (("Does the build/installation in a test directory located in $HOME/easybuildinstall"), None, 'store_true', False, 'p'), 'read-only-installdir': ("Set read-only permissions on installation directory after installation", From 46525093d7b622534f31b45ac45bbac1b261d2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 22 May 2024 19:55:17 +0200 Subject: [PATCH 23/55] Add automatic detection of python site-package dir in installations --- easybuild/framework/easyblock.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 88da255d1d..6e7ab1d654 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1438,6 +1438,22 @@ def make_module_extra(self, altroot=None, altversion=None): value, type(value)) lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) + # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present + if 'PYTHONPATH' not in self.module_generator.added_paths_per_key and \ + 'EBPYTHONPREFIXES' not in self.module_generator.added_paths_per_key: + python_paths = [path for path in glob.glob('lib*/python*/site-packages') + if re.match(r'lib(64)?/python\d+\.\d+/site-packages', path)] + use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ + self.cfg['prefer_ebpythonprefixes'] + + if len(python_paths) > 1 and not use_ebpythonprefixes: + raise EasyBuildError('Multiple python paths requires EBPYHONPREFIXES: ' + ', '.join(python_paths)) + elif python_paths: + if use_ebpythonprefixes: + lines.append(self.module_generator.append_paths('EBPYHONPREFIXES', '.')) + else: + lines.append(self.module_generator.append_paths('PYTHONPATH', python_paths)) + modloadmsg = self.cfg['modloadmsg'] if modloadmsg: # add trailing newline to prevent that shell prompt is 'glued' to module load message From 1acb45a1d39abf4f889dc193edac28cf2b2155de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 3 Jul 2024 19:54:49 +0200 Subject: [PATCH 24/55] Switch to using prepend_paths instead of append_paths --- easybuild/framework/easyblock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 6e7ab1d654..a4cf035958 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1447,12 +1447,12 @@ def make_module_extra(self, altroot=None, altversion=None): self.cfg['prefer_ebpythonprefixes'] if len(python_paths) > 1 and not use_ebpythonprefixes: - raise EasyBuildError('Multiple python paths requires EBPYHONPREFIXES: ' + ', '.join(python_paths)) + raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) elif python_paths: if use_ebpythonprefixes: - lines.append(self.module_generator.append_paths('EBPYHONPREFIXES', '.')) + lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '.')) else: - lines.append(self.module_generator.append_paths('PYTHONPATH', python_paths)) + lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: From d1c889f8fe0939a76aab3031af384baf8a8cacd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 4 Jul 2024 02:34:05 +0200 Subject: [PATCH 25/55] Fix relative paths when globbing for python paths --- easybuild/framework/easyblock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index a4cf035958..26fb7c9e5b 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1441,8 +1441,9 @@ def make_module_extra(self, altroot=None, altversion=None): # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present if 'PYTHONPATH' not in self.module_generator.added_paths_per_key and \ 'EBPYTHONPREFIXES' not in self.module_generator.added_paths_per_key: - python_paths = [path for path in glob.glob('lib*/python*/site-packages') - if re.match(r'lib(64)?/python\d+\.\d+/site-packages', path)] + python_paths = [path[len(self.installdir)+1:] + for path in glob.glob(f'{self.installdir}/lib*/python*/site-packages') + if re.match(self.installdir + r'/lib(64)?/python\d+\.\d+/site-packages', path)] use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ self.cfg['prefer_ebpythonprefixes'] From 0310bed9a1c9c21bb961dfe04ce00ab34e9a3b76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 4 Jul 2024 15:08:55 +0200 Subject: [PATCH 26/55] Simplify generated EBPYTHONPREFIXES path --- 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 26fb7c9e5b..6cc01f7498 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1451,7 +1451,7 @@ def make_module_extra(self, altroot=None, altversion=None): raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) elif python_paths: if use_ebpythonprefixes: - lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '.')) + lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) else: lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) From a1e4ecced7e941aba8ecafe08421d27472dc654a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sat, 6 Jul 2024 20:07:18 +0200 Subject: [PATCH 27/55] Rework logic to allow nonstandard PYTHONPATHs to be added while still preserving the automatic adding of standard paths. --- easybuild/framework/easyblock.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 6cc01f7498..66b090f915 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1439,21 +1439,20 @@ def make_module_extra(self, altroot=None, altversion=None): lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present - if 'PYTHONPATH' not in self.module_generator.added_paths_per_key and \ - 'EBPYTHONPREFIXES' not in self.module_generator.added_paths_per_key: - python_paths = [path[len(self.installdir)+1:] - for path in glob.glob(f'{self.installdir}/lib*/python*/site-packages') - if re.match(self.installdir + r'/lib(64)?/python\d+\.\d+/site-packages', path)] - use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ - self.cfg['prefer_ebpythonprefixes'] - - if len(python_paths) > 1 and not use_ebpythonprefixes: - raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) - elif python_paths: - if use_ebpythonprefixes: - lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) - else: - lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) + python_paths = [path[len(self.installdir)+1:] + for path in glob.glob(f'{self.installdir}/lib*/python*/site-packages') + if re.match(self.installdir + r'/lib(64)?/python\d+\.\d+/site-packages', path)] + use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ + self.cfg['prefer_ebpythonprefixes'] + + if len(python_paths) > 1 and not use_ebpythonprefixes: + raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) + elif python_paths: + # Add paths unless they were already added + if use_ebpythonprefixes and '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: + lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) + elif python_paths[0] not in self.module_generator.added_paths_per_key['PYTHONPATH']: + lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: From 4ed16be0294cd9efc9ea4e95154daf01d5108aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 7 Jul 2024 18:00:28 +0200 Subject: [PATCH 28/55] Use defaultdict with sets for added_paths_per_key Simplifies code when using it in easyblocks --- easybuild/tools/module_generator.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index a05d9b8941..e9e285b5cb 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -39,6 +39,7 @@ import os import re import tempfile +from collections import defaultdict from contextlib import contextmanager from easybuild.tools import LooseVersion from textwrap import wrap @@ -153,7 +154,7 @@ def start_module_creation(self): raise EasyBuildError('Module creation already in process. ' 'You cannot create multiple modules at the same time!') # Mapping of keys/env vars to paths already added - self.added_paths_per_key = dict() + self.added_paths_per_key = defaultdict(set) txt = self.MODULE_SHEBANG if txt: txt += '\n' @@ -212,7 +213,7 @@ def _filter_paths(self, key, paths): print_warning('Module creation has not been started. Call start_module_creation first!') return paths - added_paths = self.added_paths_per_key.setdefault(key, set()) + added_paths = self.added_paths_per_key[key] # paths can be a string if isinstance(paths, str): if paths in added_paths: From 9f3cf6aeb9fe17b72123a9d99c3963ceee25e00a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Tue, 13 Aug 2024 11:23:11 +0200 Subject: [PATCH 29/55] Exclude base python installations themselves from automatic PYTHONPATH --- easybuild/framework/easyblock.py | 39 ++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 66b090f915..898fb7b802 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1438,21 +1438,30 @@ def make_module_extra(self, altroot=None, altversion=None): value, type(value)) lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) - # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present - python_paths = [path[len(self.installdir)+1:] - for path in glob.glob(f'{self.installdir}/lib*/python*/site-packages') - if re.match(self.installdir + r'/lib(64)?/python\d+\.\d+/site-packages', path)] - use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ - self.cfg['prefer_ebpythonprefixes'] - - if len(python_paths) > 1 and not use_ebpythonprefixes: - raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) - elif python_paths: - # Add paths unless they were already added - if use_ebpythonprefixes and '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: - lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) - elif python_paths[0] not in self.module_generator.added_paths_per_key['PYTHONPATH']: - lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) + # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist + if self.name not in ['Python', 'Miniconda', 'Anaconda']: # nothing extra needed for base python installations + # Exclude symlinks lib -> lib64 or vice verse to avoid duplicates + python_paths = [] + if not os.path.islink(f'{self.installdir}/lib'): + python_paths += [path[len(self.installdir)+1:] + for path in glob.glob(f'{self.installdir}/lib/python*/site-packages') + if re.match(self.installdir + r'/lib/python\d+\.\d+/site-packages', path)] + if not os.path.islink(f'{self.installdir}/lib64'): + python_paths += [path[len(self.installdir)+1:] + for path in glob.glob(f'{self.installdir}/lib64/python*/site-packages') + if re.match(self.installdir + r'/lib64/python\d+\.\d+/site-packages', path)] + + use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ + self.cfg['prefer_ebpythonprefixes'] + + if len(python_paths) > 1 and not use_ebpythonprefixes: + raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) + elif python_paths: + # Add paths unless they were already added + if use_ebpythonprefixes and '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: + lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) + elif python_paths[0] not in self.module_generator.added_paths_per_key['PYTHONPATH']: + lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: From 037827b6a47d38550e66ddb0b68d03a5b297c4a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 15 Aug 2024 15:23:06 +0200 Subject: [PATCH 30/55] Fix logic when someone else added PYTHONPATH/EBPYTHONPREFIXES --- easybuild/framework/easyblock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 898fb7b802..0397d74b94 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1458,8 +1458,9 @@ def make_module_extra(self, altroot=None, altversion=None): raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) elif python_paths: # Add paths unless they were already added - if use_ebpythonprefixes and '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: - lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) + if use_ebpythonprefixes: + if '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: + lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) elif python_paths[0] not in self.module_generator.added_paths_per_key['PYTHONPATH']: lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) From 38da36b85528b731b294d531732304d756780d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 15 Aug 2024 20:26:57 +0200 Subject: [PATCH 31/55] Add runtime_only flag to dependencies() --- easybuild/framework/easyconfig/easyconfig.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 0d54443517..4a4c4eabd6 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1105,15 +1105,19 @@ def filter_deps(self, deps): return retained_deps - def dependencies(self, build_only=False): + def dependencies(self, build_only=False, runtime_only=False): """ Returns an array of parsed dependencies (after filtering, if requested) dependency = {'name': '', 'version': '', 'system': (False|True), 'versionsuffix': '', 'toolchain': ''} Iterable builddependencies are flattened when not iterating. :param build_only: only return build dependencies, discard others + :param runtime_only: only return runtime dependencies, discard others """ - deps = self.builddependencies() + if runtime_only: + deps = [] + else: + deps = self.builddependencies() if not build_only: # use += rather than .extend to get a new list rather than updating list of build deps in place... From 46a52dffb6cac36aa0403401dfec1119d6792018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 15 Aug 2024 20:28:02 +0200 Subject: [PATCH 32/55] Switch to more reliable way of checking for direct dependencies. --- easybuild/framework/easyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 0397d74b94..42ae536487 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1451,8 +1451,8 @@ def make_module_extra(self, altroot=None, altversion=None): for path in glob.glob(f'{self.installdir}/lib64/python*/site-packages') if re.match(self.installdir + r'/lib64/python\d+\.\d+/site-packages', path)] - use_ebpythonprefixes = get_software_root('Python') and build_option('prefer_ebpythonprefixes') and \ - self.cfg['prefer_ebpythonprefixes'] + use_ebpythonprefixes = 'Python' in self.cfg.dependencies(runtime_only=True) and \ + build_option('prefer_ebpythonprefixes') and self.cfg['prefer_ebpythonprefixes'] if len(python_paths) > 1 and not use_ebpythonprefixes: raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) From fb51411436212ed81987427d1d62c5228f1e87ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 15 Aug 2024 20:59:07 +0200 Subject: [PATCH 33/55] Drop extra check for multiple PYTHONPATH's --- easybuild/framework/easyblock.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 42ae536487..5b77cc5d0d 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1454,9 +1454,7 @@ def make_module_extra(self, altroot=None, altversion=None): use_ebpythonprefixes = 'Python' in self.cfg.dependencies(runtime_only=True) and \ build_option('prefer_ebpythonprefixes') and self.cfg['prefer_ebpythonprefixes'] - if len(python_paths) > 1 and not use_ebpythonprefixes: - raise EasyBuildError('Multiple python paths requires EBPYTHONPREFIXES: ' + ', '.join(python_paths)) - elif python_paths: + if python_paths: # Add paths unless they were already added if use_ebpythonprefixes: if '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: From a0640063708bdf981040995539bc44bf0e2f528b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 15 Aug 2024 21:09:17 +0200 Subject: [PATCH 34/55] Fix check for Python runtime dep --- easybuild/framework/easyblock.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 5b77cc5d0d..44b1c6d155 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1451,7 +1451,8 @@ def make_module_extra(self, altroot=None, altversion=None): for path in glob.glob(f'{self.installdir}/lib64/python*/site-packages') if re.match(self.installdir + r'/lib64/python\d+\.\d+/site-packages', path)] - use_ebpythonprefixes = 'Python' in self.cfg.dependencies(runtime_only=True) and \ + runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] + use_ebpythonprefixes = 'Python' in runtime_deps and \ build_option('prefer_ebpythonprefixes') and self.cfg['prefer_ebpythonprefixes'] if python_paths: From d281fc8c542006730711c0908a8c477a86161514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 16 Aug 2024 12:17:49 +0200 Subject: [PATCH 35/55] Switch method of detecting base python install --- 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 44b1c6d155..9f7ce4a282 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1439,7 +1439,7 @@ def make_module_extra(self, altroot=None, altversion=None): lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist - if self.name not in ['Python', 'Miniconda', 'Anaconda']: # nothing extra needed for base python installations + if not os.path.isfile(f'{self.installdir}/bin/python'): # only needed when not a base python installation # Exclude symlinks lib -> lib64 or vice verse to avoid duplicates python_paths = [] if not os.path.islink(f'{self.installdir}/lib'): From 11cffaa8b8ee6e7a2ae0890f3149383623c4b015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 16 Aug 2024 12:24:47 +0200 Subject: [PATCH 36/55] Add all the found python paths even when using PYTHONPATH --- easybuild/framework/easyblock.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 9f7ce4a282..71bc79d3bf 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1460,8 +1460,10 @@ def make_module_extra(self, altroot=None, altversion=None): if use_ebpythonprefixes: if '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) - elif python_paths[0] not in self.module_generator.added_paths_per_key['PYTHONPATH']: - lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_paths)) + else: + for python_path in python_paths: + if python_path not in self.module_generator.added_paths_per_key['PYTHONPATH']: + lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_path)) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: From f90a12bcf5661858e7a84078df3e6e60d3285ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 16 Aug 2024 13:14:13 +0200 Subject: [PATCH 37/55] Drop lib64 variant as it's not supported by sitecustomize.py Our sitecustomize.py only considers the lib/pythonx.xx/site-packages paths anyway, so we must not consider any more when adding the automatic paths. --- easybuild/framework/easyblock.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 71bc79d3bf..baf7e93446 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1440,16 +1440,9 @@ def make_module_extra(self, altroot=None, altversion=None): # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist if not os.path.isfile(f'{self.installdir}/bin/python'): # only needed when not a base python installation - # Exclude symlinks lib -> lib64 or vice verse to avoid duplicates - python_paths = [] - if not os.path.islink(f'{self.installdir}/lib'): - python_paths += [path[len(self.installdir)+1:] - for path in glob.glob(f'{self.installdir}/lib/python*/site-packages') - if re.match(self.installdir + r'/lib/python\d+\.\d+/site-packages', path)] - if not os.path.islink(f'{self.installdir}/lib64'): - python_paths += [path[len(self.installdir)+1:] - for path in glob.glob(f'{self.installdir}/lib64/python*/site-packages') - if re.match(self.installdir + r'/lib64/python\d+\.\d+/site-packages', path)] + python_paths = [path[len(self.installdir)+1:] + for path in glob.glob(f'{self.installdir}/lib/python*/site-packages') + if re.match(self.installdir + r'/lib/python\d+\.\d+/site-packages', path)] runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] use_ebpythonprefixes = 'Python' in runtime_deps and \ From 1d78e6c2c2ac4777e060a24827718457e23075af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 16 Aug 2024 15:20:47 +0200 Subject: [PATCH 38/55] Default --prefer-ebpythonprefixes to false --- 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 c5fc72061d..551cb14a58 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -491,7 +491,7 @@ def override_options(self): 'pre-create-installdir': ("Create installation directory before submitting build jobs", None, 'store_true', True), 'prefer-ebpythonprefixes': (("Prefer EBPYTHONPREFIXES over PYTHONPATH when possible"), - None, 'store_true', True), + None, 'store_true', False), 'pretend': (("Does the build/installation in a test directory located in $HOME/easybuildinstall"), None, 'store_true', False, 'p'), 'read-only-installdir': ("Set read-only permissions on installation directory after installation", From 7b7a59f756d94edf1ae49a2e3d32b23e06cbd71e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 19 Sep 2024 18:29:40 +0200 Subject: [PATCH 39/55] Rename options and parameters for ebpythonprefixes change --- easybuild/framework/easyblock.py | 13 +++++++------ easybuild/framework/easyconfig/default.py | 2 +- easybuild/tools/config.py | 8 +++++++- easybuild/tools/options.py | 7 +++++-- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index baf7e93446..1538334eb6 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -72,7 +72,7 @@ from easybuild.tools.build_details import get_build_stats from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, dry_run_msg, dry_run_warning, dry_run_set_dirs from easybuild.tools.build_log import print_error, print_msg, print_warning -from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES +from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES, PYTHONPATH, EBPYTHONPREFIXES from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES from easybuild.tools.config import EASYBUILD_SOURCES_URL # noqa from easybuild.tools.config import build_option, build_path, get_log_filename, get_repository, get_repositorypath @@ -1446,17 +1446,18 @@ def make_module_extra(self, altroot=None, altversion=None): runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] use_ebpythonprefixes = 'Python' in runtime_deps and \ - build_option('prefer_ebpythonprefixes') and self.cfg['prefer_ebpythonprefixes'] + build_option('prefer_python_search_path') == EBPYTHONPREFIXES and not self.cfg['force_pythonpath'] if python_paths: # Add paths unless they were already added if use_ebpythonprefixes: - if '' not in self.module_generator.added_paths_per_key['EBPYTHONPREFIXES']: - lines.append(self.module_generator.prepend_paths('EBPYTHONPREFIXES', '')) + path = '' # EBPYTHONPREFIXES are relative to the install dir + if path not in self.module_generator.added_paths_per_key[EBPYTHONPREFIXES]: + lines.append(self.module_generator.prepend_paths(EBPYTHONPREFIXES, path)) else: for python_path in python_paths: - if python_path not in self.module_generator.added_paths_per_key['PYTHONPATH']: - lines.append(self.module_generator.prepend_paths('PYTHONPATH', python_path)) + if python_path not in self.module_generator.added_paths_per_key[PYTHONPATH]: + lines.append(self.module_generator.prepend_paths(PYTHONPATH, python_path)) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index 43f6093588..db1e12c324 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -115,7 +115,7 @@ 'patches': [[], "List of patches to apply", BUILD], 'prebuildopts': ['', 'Extra options pre-passed to build command.', BUILD], 'preconfigopts': ['', 'Extra options pre-passed to configure.', BUILD], - 'prefer_ebpythonprefixes': [True, "Prefer EBPYTHONPREFIXES over PYTHONPATH when possible.", BUILD], + 'force_pythonpath': [False, "Force use of PYTHONPATH for python seearch path when possible.", BUILD], 'preinstallopts': ['', 'Extra prefix options for installation.', BUILD], 'pretestopts': ['', 'Extra prefix options for test.', BUILD], 'postinstallcmds': [[], 'Commands to run after the install step.', BUILD], diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 67473b4dc0..384ee065d3 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -175,6 +175,10 @@ OUTPUT_STYLE_RICH = 'rich' OUTPUT_STYLES = (OUTPUT_STYLE_AUTO, OUTPUT_STYLE_BASIC, OUTPUT_STYLE_NO_COLOR, OUTPUT_STYLE_RICH) +PYTHONPATH = 'PYTHONPATH' +EBPYTHONPREFIXES = 'EBPYTHONPREFIXES' +PYTHON_SEARCH_PATH_TYPES = [PYTHONPATH, EBPYTHONPREFIXES] + class Singleton(ABCMeta): """Serves as metaclass for classes that should implement the Singleton pattern. @@ -336,7 +340,6 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'modules_tool_version_check', 'mpi_tests', 'pre_create_installdir', - 'prefer_ebpythonprefixes', 'show_progress_bar', 'trace', ], @@ -408,6 +411,9 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): OUTPUT_STYLE_AUTO: [ 'output_style', ], + PYTHONPATH: [ + 'prefer_python_search_path', + ] } # build option that do not have a perfectly matching command line option BUILD_OPTIONS_OTHER = { diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 551cb14a58..6a39bfe80e 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -78,6 +78,7 @@ from easybuild.tools.config import OUTPUT_STYLE_AUTO, OUTPUT_STYLES, WARN from easybuild.tools.config import get_pretend_installpath, init, init_build_options, mk_full_default_path from easybuild.tools.config import BuildOptions, ConfigurationVariables +from easybuild.tools.config import PYTHON_SEARCH_PATH_TYPES, PYTHONPATH from easybuild.tools.configobj import ConfigObj, ConfigObjError from easybuild.tools.docs import FORMAT_JSON, FORMAT_MD, FORMAT_RST, FORMAT_TXT from easybuild.tools.docs import avail_cfgfile_constants, avail_easyconfig_constants, avail_easyconfig_licenses @@ -490,8 +491,10 @@ def override_options(self): None, 'store_true', False), 'pre-create-installdir': ("Create installation directory before submitting build jobs", None, 'store_true', True), - 'prefer-ebpythonprefixes': (("Prefer EBPYTHONPREFIXES over PYTHONPATH when possible"), - None, 'store_true', False), + 'prefer-python-search-path': (("Prefer using specified environment variable when possible to specify where" + " Python packages were installed; see also " + "https://docs.easybuild.io/python-search-path"), + 'choice', 'store_or_None', PYTHONPATH, PYTHON_SEARCH_PATH_TYPES), 'pretend': (("Does the build/installation in a test directory located in $HOME/easybuildinstall"), None, 'store_true', False, 'p'), 'read-only-installdir': ("Set read-only permissions on installation directory after installation", From dc488f5da8a3b0038940a9e740f563193efc286b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Mon, 23 Sep 2024 16:41:05 +0200 Subject: [PATCH 40/55] Switch to using relpath to make code robust --- 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 1538334eb6..f63c15fffd 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1440,7 +1440,7 @@ def make_module_extra(self, altroot=None, altversion=None): # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist if not os.path.isfile(f'{self.installdir}/bin/python'): # only needed when not a base python installation - python_paths = [path[len(self.installdir)+1:] + python_paths = [os.path.relpath(path, self.installdir) for path in glob.glob(f'{self.installdir}/lib/python*/site-packages') if re.match(self.installdir + r'/lib/python\d+\.\d+/site-packages', path)] From 7e417d3f3d532c6ed19bf006dd14569a3d5571ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 27 Sep 2024 01:05:02 +0200 Subject: [PATCH 41/55] Use relative paths when looking for python paths Not only simplifies the code but is also the obviously correct way which i should have used from the start. --- easybuild/framework/easyblock.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index f63c15fffd..a6da6ce63a 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1439,10 +1439,9 @@ def make_module_extra(self, altroot=None, altversion=None): lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist - if not os.path.isfile(f'{self.installdir}/bin/python'): # only needed when not a base python installation - python_paths = [os.path.relpath(path, self.installdir) - for path in glob.glob(f'{self.installdir}/lib/python*/site-packages') - if re.match(self.installdir + r'/lib/python\d+\.\d+/site-packages', path)] + if not os.path.isfile(os.path.join(self.installdir, 'bin/python')): # only needed when not a python install + python_paths = [path for path in glob.glob('lib/python*/site-packages', root_dir=self.installdir) + if re.match(r'lib/python\d+\.\d+/site-packages', path)] runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] use_ebpythonprefixes = 'Python' in runtime_deps and \ From 9813bc8e3cef36aed320ab6ff36a567f9385a976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 27 Sep 2024 12:04:09 +0200 Subject: [PATCH 42/55] Replace glob(root_dir=...) which only is supported in python 3.10 Instead, using absolute paths and then make them relative. --- easybuild/framework/easyblock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index a6da6ce63a..65ae805380 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1440,8 +1440,9 @@ def make_module_extra(self, altroot=None, altversion=None): # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist if not os.path.isfile(os.path.join(self.installdir, 'bin/python')): # only needed when not a python install - python_paths = [path for path in glob.glob('lib/python*/site-packages', root_dir=self.installdir) - if re.match(r'lib/python\d+\.\d+/site-packages', path)] + candidate_paths = (os.path.relpath(path, self.installdir) + for path in glob.glob(f'{self.installdir}/lib/python*/site-packages')) + python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] use_ebpythonprefixes = 'Python' in runtime_deps and \ From 206736eb69ddf1e497b3ac07fb846d07bde13cca Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 30 Sep 2024 11:03:54 +0200 Subject: [PATCH 43/55] add test to verify that $PYTHONPATH or $EBPYTHONPREFIXES are set correctly by generated module file --- test/framework/toy_build.py | 63 +++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index afe0b86bef..76ef640b29 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -4238,6 +4238,69 @@ def test_eb_error(self): stderr = stderr.getvalue() self.assertTrue(regex.search(stderr), f"Pattern '{regex.pattern}' should be found in {stderr}") + def test_toy_python(self): + """ + Test whether $PYTHONPATH or $EBPYTHONPREFIXES are set correctly. + """ + # generate fake Python module that we can use as runtime dependency for toy + # (required condition for use of $EBPYTHONPREFIXES) + fake_mods_path = os.path.join(self.test_prefix, 'modules') + fake_python_mod = os.path.join(fake_mods_path, 'Python', '3.6') + if get_module_syntax() == 'Lua': + fake_python_mod += '.lua' + write_file(fake_python_mod, '') + else: + write_file(fake_python_mod, '#%Module') + self.modtool.use(fake_mods_path) + + test_ecs = os.path.join(os.path.dirname(__file__), 'easyconfigs', 'test_ecs') + toy_ec = os.path.join(test_ecs, 't', 'toy', 'toy-0.0.eb') + + test_ec_txt = read_file(toy_ec) + test_ec_txt += "\npostinstallcmds.append('mkdir -p %(installdir)s/lib/python3.6/site-packages')" + test_ec_txt += "\npostinstallcmds.append('touch %(installdir)s/lib/python3.6/site-packages/foo.py')" + + test_ec = os.path.join(self.test_prefix, 'test.eb') + write_file(test_ec, test_ec_txt) + self.run_test_toy_build_with_output(ec_file=test_ec) + + toy_mod = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0') + if get_module_syntax() == 'Lua': + toy_mod += '.lua' + toy_mod_txt = read_file(toy_mod) + + pythonpath_regex = re.compile('^prepend.path.*PYTHONPATH.*lib/python3.6/site-packages', re.M) + + self.assertTrue(pythonpath_regex.search(toy_mod_txt), + f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") + + # also check when opting in to use $EBPYTHONPREFIXES instead of $PYTHONPATH + args = ['--prefer-python-search-path=EBPYTHONPREFIXES'] + self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args) + toy_mod_txt = read_file(toy_mod) + # if Python is not listed as a runtime dependency then $PYTHONPATH is still used, + # because the Python dependency used must be aware of $EBPYTHONPREFIXES + # (see sitecustomize.py installed by Python easyblock) + self.assertTrue(pythonpath_regex.search(toy_mod_txt), + f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") + + test_ec_txt += "\ndependencies = [('Python', '3.6', '', SYSTEM)]" + write_file(test_ec, test_ec_txt) + self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args) + toy_mod_txt = read_file(toy_mod) + + ebpythonprefixes_regex = re.compile('^prepend.path.*EBPYTHONPREFIXES.*root', re.M) + self.assertTrue(ebpythonprefixes_regex.search(toy_mod_txt), + f"Pattern '{ebpythonprefixes_regex.pattern}' found in: {toy_mod_txt}") + + test_ec_txt += "\nforce_pythonpath = True" + write_file(test_ec, test_ec_txt) + self.run_test_toy_build_with_output(ec_file=test_ec) + toy_mod_txt = read_file(toy_mod) + + self.assertTrue(pythonpath_regex.search(toy_mod_txt), + f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") + def suite(): """ return all the tests in this file """ From 5eb6d078d9a21bbeab7bf3cf31ad213056d76e22 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 30 Sep 2024 11:09:48 +0200 Subject: [PATCH 44/55] move logic to set $PYTHONPATH or $EBPYTHONPREFIX to dedicated make_module_pythonpath method --- easybuild/framework/easyblock.py | 53 ++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 65ae805380..87e5660e2b 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1390,6 +1390,37 @@ def make_module_description(self): """ return self.module_generator.get_description() + def make_module_pythonpath(self): + """ + Add lines for module file to update $PYTHONPATH or $EBPYTHONPREFIXES, + if they aren't already present and the standard lib/python*/site-packages subdirectory exists + """ + lines = [] + if not os.path.isfile(os.path.join(self.installdir, 'bin', 'python')): # only needed when not a python install + python_subdir_pattern = os.path.join(self.installdir, 'lib', 'python*', 'site-packages') + candidate_paths = (os.path.relpath(path, self.installdir) for path in glob.glob(python_subdir_pattern)) + python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + + runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] + use_ebpythonprefixes = all([ + 'Python' in runtime_deps, + build_option('prefer_python_search_path') == EBPYTHONPREFIXES, + not self.cfg['force_pythonpath'] + ]) + + if python_paths: + # add paths unless they were already added + if use_ebpythonprefixes: + path = '' # EBPYTHONPREFIXES are relative to the install dir + if path not in self.module_generator.added_paths_per_key[EBPYTHONPREFIXES]: + lines.append(self.module_generator.prepend_paths(EBPYTHONPREFIXES, path)) + else: + for python_path in python_paths: + if python_path not in self.module_generator.added_paths_per_key[PYTHONPATH]: + lines.append(self.module_generator.prepend_paths(PYTHONPATH, python_path)) + + return lines + def make_module_extra(self, altroot=None, altversion=None): """ Set extra stuff in module file, e.g. $EBROOT*, $EBVERSION*, etc. @@ -1438,26 +1469,8 @@ def make_module_extra(self, altroot=None, altversion=None): value, type(value)) lines.append(self.module_generator.append_paths(key, value, allow_abs=self.cfg['allow_append_abs_path'])) - # Add automatic PYTHONPATH or EBPYTHONPREFIXES if they aren't already present and python paths exist - if not os.path.isfile(os.path.join(self.installdir, 'bin/python')): # only needed when not a python install - candidate_paths = (os.path.relpath(path, self.installdir) - for path in glob.glob(f'{self.installdir}/lib/python*/site-packages')) - python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] - - runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] - use_ebpythonprefixes = 'Python' in runtime_deps and \ - build_option('prefer_python_search_path') == EBPYTHONPREFIXES and not self.cfg['force_pythonpath'] - - if python_paths: - # Add paths unless they were already added - if use_ebpythonprefixes: - path = '' # EBPYTHONPREFIXES are relative to the install dir - if path not in self.module_generator.added_paths_per_key[EBPYTHONPREFIXES]: - lines.append(self.module_generator.prepend_paths(EBPYTHONPREFIXES, path)) - else: - for python_path in python_paths: - if python_path not in self.module_generator.added_paths_per_key[PYTHONPATH]: - lines.append(self.module_generator.prepend_paths(PYTHONPATH, python_path)) + # add lines to update $PYTHONPATH or $EBPYTHONPREFIXES + lines.extend(self.make_module_pythonpath()) modloadmsg = self.cfg['modloadmsg'] if modloadmsg: From 76fec7b84747f036c83b85238499c4a303b73285 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 30 Sep 2024 15:28:03 +0200 Subject: [PATCH 45/55] take into account multi_deps when determining whether to use $PYTHONPATH or $EBPYTHONPREFIXES --- easybuild/framework/easyblock.py | 25 ++++++++++++++++++++----- test/framework/toy_build.py | 30 +++++++++++++++++++----------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 87e5660e2b..0dc1e2e2a5 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1401,12 +1401,27 @@ def make_module_pythonpath(self): candidate_paths = (os.path.relpath(path, self.installdir) for path in glob.glob(python_subdir_pattern)) python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + # determine whether Python is a runtime dependency; + # if so, we assume it was installed with EasyBuild, and hence is aware of $EBPYTHONPREFIXES runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] - use_ebpythonprefixes = all([ - 'Python' in runtime_deps, - build_option('prefer_python_search_path') == EBPYTHONPREFIXES, - not self.cfg['force_pythonpath'] - ]) + + # don't use $EBPYTHONPREFIXES unless we can and it's preferred or necesary (due to use of multi_deps) + use_ebpythonprefixes = False + multi_deps = self.cfg['multi_deps'] + + if self.cfg['force_pythonpath']: + self.log.info("Using $PYTHONPATH to specify Python search path, since 'force_pythonpath' is set") + + elif 'Python' in runtime_deps: + self.log.info("Found Python runtime dependency, so considering $EBPYTHONPREFIXES...") + + if build_option('prefer_python_search_path') == EBPYTHONPREFIXES: + self.log.info("Preferred Python search path is $EBPYTHONPREFIXES, so using that") + use_ebpythonprefixes = True + + elif multi_deps and 'Python' in multi_deps: + self.log.info("Python is listed in 'multi_deps', so using $EBPYTHONPREFIXES instead of $PYTHONPATH") + use_ebpythonprefixes = True if python_paths: # add paths unless they were already added diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 76ef640b29..e9b857ecee 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -4242,15 +4242,16 @@ def test_toy_python(self): """ Test whether $PYTHONPATH or $EBPYTHONPREFIXES are set correctly. """ - # generate fake Python module that we can use as runtime dependency for toy + # generate fake Python modules that we can use as runtime dependency for toy # (required condition for use of $EBPYTHONPREFIXES) fake_mods_path = os.path.join(self.test_prefix, 'modules') - fake_python_mod = os.path.join(fake_mods_path, 'Python', '3.6') - if get_module_syntax() == 'Lua': - fake_python_mod += '.lua' - write_file(fake_python_mod, '') - else: - write_file(fake_python_mod, '#%Module') + for pyver in ('2.7', '3.6'): + fake_python_mod = os.path.join(fake_mods_path, 'Python', pyver) + if get_module_syntax() == 'Lua': + fake_python_mod += '.lua' + write_file(fake_python_mod, '') + else: + write_file(fake_python_mod, '#%Module') self.modtool.use(fake_mods_path) test_ecs = os.path.join(os.path.dirname(__file__), 'easyconfigs', 'test_ecs') @@ -4284,8 +4285,8 @@ def test_toy_python(self): self.assertTrue(pythonpath_regex.search(toy_mod_txt), f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") - test_ec_txt += "\ndependencies = [('Python', '3.6', '', SYSTEM)]" - write_file(test_ec, test_ec_txt) + # if Python is listed as runtime dependency, then $EBPYTHONPREFIXES is used if it's preferred + write_file(test_ec, test_ec_txt + "\ndependencies = [('Python', '3.6', '', SYSTEM)]") self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args) toy_mod_txt = read_file(toy_mod) @@ -4293,11 +4294,18 @@ def test_toy_python(self): self.assertTrue(ebpythonprefixes_regex.search(toy_mod_txt), f"Pattern '{ebpythonprefixes_regex.pattern}' found in: {toy_mod_txt}") - test_ec_txt += "\nforce_pythonpath = True" - write_file(test_ec, test_ec_txt) + # if Python is listed in multi_deps, then $EBPYTHONPREFIXES is used, even if it's not explicitely preferred + write_file(test_ec, test_ec_txt + "\nmulti_deps = {'Python': ['2.7', '3.6']}") self.run_test_toy_build_with_output(ec_file=test_ec) toy_mod_txt = read_file(toy_mod) + self.assertTrue(ebpythonprefixes_regex.search(toy_mod_txt), + f"Pattern '{ebpythonprefixes_regex.pattern}' found in: {toy_mod_txt}") + + write_file(test_ec, test_ec_txt + "\ndependencies = [('Python', '3.6', '', SYSTEM)]\nforce_pythonpath = True") + self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args) + toy_mod_txt = read_file(toy_mod) + self.assertTrue(pythonpath_regex.search(toy_mod_txt), f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") From 11f132ea0ee733a849f715daaed89498c8763f06 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 30 Sep 2024 16:03:48 +0200 Subject: [PATCH 46/55] remove force_pythonpath easyconfig paramter, can be re-introduced later if there's a real need for it --- easybuild/framework/easyblock.py | 5 +---- easybuild/framework/easyconfig/default.py | 1 - test/framework/toy_build.py | 7 ------- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 0dc1e2e2a5..a786cbea2b 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1409,10 +1409,7 @@ def make_module_pythonpath(self): use_ebpythonprefixes = False multi_deps = self.cfg['multi_deps'] - if self.cfg['force_pythonpath']: - self.log.info("Using $PYTHONPATH to specify Python search path, since 'force_pythonpath' is set") - - elif 'Python' in runtime_deps: + if 'Python' in runtime_deps: self.log.info("Found Python runtime dependency, so considering $EBPYTHONPREFIXES...") if build_option('prefer_python_search_path') == EBPYTHONPREFIXES: diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index db1e12c324..438bcea4e1 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -115,7 +115,6 @@ 'patches': [[], "List of patches to apply", BUILD], 'prebuildopts': ['', 'Extra options pre-passed to build command.', BUILD], 'preconfigopts': ['', 'Extra options pre-passed to configure.', BUILD], - 'force_pythonpath': [False, "Force use of PYTHONPATH for python seearch path when possible.", BUILD], 'preinstallopts': ['', 'Extra prefix options for installation.', BUILD], 'pretestopts': ['', 'Extra prefix options for test.', BUILD], 'postinstallcmds': [[], 'Commands to run after the install step.', BUILD], diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index e9b857ecee..edd9e94fcc 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -4302,13 +4302,6 @@ def test_toy_python(self): self.assertTrue(ebpythonprefixes_regex.search(toy_mod_txt), f"Pattern '{ebpythonprefixes_regex.pattern}' found in: {toy_mod_txt}") - write_file(test_ec, test_ec_txt + "\ndependencies = [('Python', '3.6', '', SYSTEM)]\nforce_pythonpath = True") - self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args) - toy_mod_txt = read_file(toy_mod) - - self.assertTrue(pythonpath_regex.search(toy_mod_txt), - f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") - def suite(): """ return all the tests in this file """ From 04e3b66fdfbd5bd04e7ff3a41da90507205667f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 29 Sep 2024 11:40:33 +0200 Subject: [PATCH 47/55] Add bash functions to env.sh --- easybuild/tools/run.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 68b3bd215a..8798c22fc4 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -208,6 +208,11 @@ def create_cmd_scripts(cmd_str, work_dir, env, tmpdir, out_file, err_file): if env is None: env = os.environ.copy() + # Decode any declared bash functions + proc = subprocess.Popen('declare -f', stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, + env=env, shell=True, executable='bash') + (bash_functions, _) = proc.communicate() + env_fp = os.path.join(tmpdir, 'env.sh') with open(env_fp, 'w') as fid: # unset all environment variables in current environment first to start from a clean slate; @@ -219,6 +224,8 @@ def create_cmd_scripts(cmd_str, work_dir, env, tmpdir, out_file, err_file): fid.write('\n'.join(f'export {key}={shlex.quote(value)}' for key, value in sorted(env.items()) if not key.endswith('%')) + '\n') + fid.write(bash_functions.decode(errors='ignore') + '\n') + fid.write('\n\nPS1="eb-shell> "') # define $EB_CMD_OUT_FILE to contain path to file with command output From b59d9e7e45c00a6a47f8865e685dacb82814bb19 Mon Sep 17 00:00:00 2001 From: crivella Date: Tue, 1 Oct 2024 11:57:49 +0200 Subject: [PATCH 48/55] Add stdin flushing and closing to `run_shell_cmd` --- easybuild/tools/run.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 21eac92a70..287c6244bd 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -414,6 +414,9 @@ def to_cmd_str(cmd): if stdin: proc.stdin.write(stdin) + proc.stdin.flush() + if not qa_patterns: + proc.stdin.close() exit_code = None stdout, stderr = b'', b'' From 0125a30570f50b5fe8b0c2e4a071dab5c2885fdc Mon Sep 17 00:00:00 2001 From: crivella Date: Tue, 1 Oct 2024 12:40:38 +0200 Subject: [PATCH 49/55] Added test --- test/framework/run.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/framework/run.py b/test/framework/run.py index f1ee6955ec..16d6753eb5 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -1367,6 +1367,25 @@ def test_run_shell_cmd_stream(self): for line in expected: self.assertIn(line, stdout) + def test_run_shell_cmd_eof_stdin(self): + """Test use of run_shell_cmd with streaming output and blocking stdin read.""" + cmd = 'timeout 1 cat -' + + inp = 'hello\nworld\n' + # test with streaming output + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd, stream_output=True, stdin=inp, fail_on_error=False) + + self.assertEqual(res.exit_code, 0, "Streaming output: Command timed out") + self.assertEqual(res.output, inp) + + # test with non-streaming output (proc.communicate() is used) + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd, stdin=inp, fail_on_error=False) + + self.assertEqual(res.exit_code, 0, "Non-streaming output: Command timed out") + self.assertEqual(res.output, inp) + def test_run_cmd_async(self): """Test asynchronously running of a shell command via run_cmd + complete_cmd.""" From 5cc7073d405b886e9ad7180f082e37c90820769a Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 1 Oct 2024 18:50:14 +0200 Subject: [PATCH 50/55] enhance test for cmd.sh script produced by run_shell_cmd to verify that 'module' function is available --- .github/workflows/unit_tests.yml | 4 +++- test/framework/run.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 0bcbf3a466..e0c572e6ad 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -152,7 +152,9 @@ jobs: cd $HOME # initialize environment for modules tool if [ -f $HOME/moduleshome ]; then export MODULESHOME=$(cat $HOME/moduleshome); fi - source $(cat $HOME/mod_init); type module + source $(cat $HOME/mod_init) + type module + module --version # make sure 'eb' is available via $PATH, and that $PYTHONPATH is set (some tests expect that); # also pick up changes to $PATH set by sourcing $MOD_INIT export PREFIX=/tmp/$USER/$GITHUB_SHA diff --git a/test/framework/run.py b/test/framework/run.py index e26373f6e6..2928e140fd 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -52,6 +52,7 @@ from easybuild.tools.build_log import EasyBuildError, init_logging, stop_logging from easybuild.tools.config import update_build_option from easybuild.tools.filetools import adjust_permissions, change_dir, mkdir, read_file, remove_dir, write_file +from easybuild.tools.modules import EnvironmentModules, Lmod from easybuild.tools.run import RunShellCmdResult, RunShellCmdError, check_async_cmd, check_log_for_errors from easybuild.tools.run import complete_cmd, fileprefix_from_cmd, get_output_from_process, parse_log_for_error from easybuild.tools.run import run_cmd, run_cmd_qa, run_shell_cmd, subprocess_terminate @@ -233,6 +234,20 @@ def test_run_shell_cmd_basic(self): pwd = res[0].strip()[5:] self.assertTrue(os.path.samefile(pwd, self.test_prefix)) + cmd = f"{cmd_script} -c 'module --version'" + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd, fail_on_error=False) + self.assertEqual(res.exit_code, 0) + + if isinstance(self.modtool, Lmod): + regex = re.compile("^Modules based on Lua: Version [0-9]", re.M) + elif isinstance(self.modtool, EnvironmentModules): + regex = re.compile("^Modules Release [0-9]", re.M) + else: + self.fail("Unknown modules tool used!") + + self.assertTrue(regex.search(res.output), f"Pattern '{regex.pattern}' should be found in {res.output}") + # test running command that emits non-UTF-8 characters # this is constructed to reproduce errors like: # UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe2 From e40aba3f4f0a0ce2f37ba14c7b33527a724859e1 Mon Sep 17 00:00:00 2001 From: Georgios Kafanas Date: Wed, 25 Sep 2024 05:13:16 +0200 Subject: [PATCH 51/55] [refactor:job_backend] Use Slurm as the default job back-end - The GC3Pie back-end for job management that has been the default so far is no longer actively supported. Use the Slurm back-end as default instead. - Add deprecation message when the GC3Pie class is instantiated. Issue: https://github.com/easybuilders/easybuild-framework/issues/4565 --- easybuild/tools/config.py | 2 +- easybuild/tools/job/gc3pie.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 13f9722bce..5a395e6925 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -96,7 +96,7 @@ DEFAULT_ENV_FOR_SHEBANG = '/usr/bin/env' DEFAULT_ENVVAR_USERS_MODULES = 'HOME' DEFAULT_INDEX_MAX_AGE = 7 * 24 * 60 * 60 # 1 week (in seconds) -DEFAULT_JOB_BACKEND = 'GC3Pie' +DEFAULT_JOB_BACKEND = 'Slurm' DEFAULT_JOB_EB_CMD = 'eb' DEFAULT_LOGFILE_FORMAT = ("easybuild", "easybuild-%(name)s-%(version)s-%(date)s.%(time)s.log") DEFAULT_MAX_FAIL_RATIO_PERMS = 0.5 diff --git a/easybuild/tools/job/gc3pie.py b/easybuild/tools/job/gc3pie.py index f6b57e0f2c..2ba866bb92 100644 --- a/easybuild/tools/job/gc3pie.py +++ b/easybuild/tools/job/gc3pie.py @@ -104,6 +104,10 @@ def __init__(self, *args, **kwargs): def _check_version(self): """Check whether GC3Pie version complies with required version.""" + deprication_msg = "The GC3Pie job back-end is no longer maintained and will be deprecated" + deprication_msg += ", please use a different backend" + _log.deprecated(deprication_msg, '5.0') + try: from pkg_resources import get_distribution, DistributionNotFound pkg = get_distribution('gc3pie') From 098fb37941228c0e456f711fdafa864742112748 Mon Sep 17 00:00:00 2001 From: Georgios Kafanas Date: Fri, 27 Sep 2024 09:13:21 +0200 Subject: [PATCH 52/55] Fix typos and punctuation Co-authored-by: Kenneth Hoste --- easybuild/tools/job/gc3pie.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/job/gc3pie.py b/easybuild/tools/job/gc3pie.py index 2ba866bb92..f51deebbdb 100644 --- a/easybuild/tools/job/gc3pie.py +++ b/easybuild/tools/job/gc3pie.py @@ -104,9 +104,9 @@ def __init__(self, *args, **kwargs): def _check_version(self): """Check whether GC3Pie version complies with required version.""" - deprication_msg = "The GC3Pie job back-end is no longer maintained and will be deprecated" - deprication_msg += ", please use a different backend" - _log.deprecated(deprication_msg, '5.0') + deprecation_msg = "The GC3Pie job backend is no longer maintained and will be removed" + deprecation_msg += ", please use a different job backend" + _log.deprecated(deprecation_msg, '5.0') try: from pkg_resources import get_distribution, DistributionNotFound From e6391c3ff4f5da25c68d11831e317f91b1665f51 Mon Sep 17 00:00:00 2001 From: Georgios Kafanas Date: Fri, 27 Sep 2024 12:49:56 +0200 Subject: [PATCH 53/55] [test:job_backend] Prevent deprecation warning from causing test failures --- test/framework/parallelbuild.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/framework/parallelbuild.py b/test/framework/parallelbuild.py index ce3f8331d4..2f4d90884f 100644 --- a/test/framework/parallelbuild.py +++ b/test/framework/parallelbuild.py @@ -262,7 +262,10 @@ def test_build_easyconfigs_in_parallel_gc3pie(self): topdir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) test_easyblocks_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'sandbox') cmd = "PYTHONPATH=%s:%s:$PYTHONPATH eb %%(spec)s -df" % (topdir, test_easyblocks_path) + + self.allow_deprecated_behaviour() build_easyconfigs_in_parallel(cmd, ordered_ecs, prepare_first=False) + self.disallow_deprecated_behaviour() toy_modfile = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0') if get_module_syntax() == 'Lua': @@ -279,8 +282,10 @@ def test_build_easyconfigs_in_parallel_gc3pie(self): write_file(test_ecfile, ectxt) ecs = resolve_dependencies(process_easyconfig(test_ecfile), self.modtool) + self.allow_deprecated_behaviour() error = "1 jobs failed: toy-1.2.3" self.assertErrorRegex(EasyBuildError, error, build_easyconfigs_in_parallel, cmd, ecs, prepare_first=False) + self.disallow_deprecated_behaviour() def test_submit_jobs(self): """Test submit_jobs""" From f52adc679a9df37f07ac22ff87cd55f5060d07fd Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 2 Oct 2024 18:26:31 +0200 Subject: [PATCH 54/55] fix marking GC3Pie job backend as deprecated: version indicates when EasyBuild will no longer support it anymore (v6.0) --- easybuild/tools/job/gc3pie.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/job/gc3pie.py b/easybuild/tools/job/gc3pie.py index f51deebbdb..60f7432217 100644 --- a/easybuild/tools/job/gc3pie.py +++ b/easybuild/tools/job/gc3pie.py @@ -106,7 +106,7 @@ def _check_version(self): deprecation_msg = "The GC3Pie job backend is no longer maintained and will be removed" deprecation_msg += ", please use a different job backend" - _log.deprecated(deprecation_msg, '5.0') + _log.deprecated(deprecation_msg, '6.0') try: from pkg_resources import get_distribution, DistributionNotFound From 0b5d69baeb4e87b13276843a909c4aefd7b9c0ff Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 2 Oct 2024 18:27:28 +0200 Subject: [PATCH 55/55] allow triggering deprecated behaviour for entire test_build_easyconfigs_in_parallel_gc3pie + catch deprecation warnings via mocked_stdout_stderr --- test/framework/parallelbuild.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/framework/parallelbuild.py b/test/framework/parallelbuild.py index 2f4d90884f..975e46375b 100644 --- a/test/framework/parallelbuild.py +++ b/test/framework/parallelbuild.py @@ -223,6 +223,8 @@ def test_build_easyconfigs_in_parallel_gc3pie(self): print("GC3Pie not available, skipping test") return + self.allow_deprecated_behaviour() + # put GC3Pie config in place to use local host and fork/exec resourcedir = os.path.join(self.test_prefix, 'gc3pie') gc3pie_cfgfile = os.path.join(self.test_prefix, 'gc3pie_local.ini') @@ -263,9 +265,8 @@ def test_build_easyconfigs_in_parallel_gc3pie(self): test_easyblocks_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'sandbox') cmd = "PYTHONPATH=%s:%s:$PYTHONPATH eb %%(spec)s -df" % (topdir, test_easyblocks_path) - self.allow_deprecated_behaviour() - build_easyconfigs_in_parallel(cmd, ordered_ecs, prepare_first=False) - self.disallow_deprecated_behaviour() + with self.mocked_stdout_stderr(): + build_easyconfigs_in_parallel(cmd, ordered_ecs, prepare_first=False) toy_modfile = os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0') if get_module_syntax() == 'Lua': @@ -282,10 +283,9 @@ def test_build_easyconfigs_in_parallel_gc3pie(self): write_file(test_ecfile, ectxt) ecs = resolve_dependencies(process_easyconfig(test_ecfile), self.modtool) - self.allow_deprecated_behaviour() error = "1 jobs failed: toy-1.2.3" - self.assertErrorRegex(EasyBuildError, error, build_easyconfigs_in_parallel, cmd, ecs, prepare_first=False) - self.disallow_deprecated_behaviour() + with self.mocked_stdout_stderr(): + self.assertErrorRegex(EasyBuildError, error, build_easyconfigs_in_parallel, cmd, ecs, prepare_first=False) def test_submit_jobs(self): """Test submit_jobs"""