Skip to content

Commit

Permalink
Merge pull request easybuilders#4654 from boegel/5.0.x
Browse files Browse the repository at this point in the history
sync with develop (20240923)
  • Loading branch information
Micket authored Sep 23, 2024
2 parents a2550eb + 58f39ef commit d3c0127
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 5 deletions.
12 changes: 12 additions & 0 deletions RELEASE_NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -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)
--------------------------

Expand Down
15 changes: 14 additions & 1 deletion easybuild/_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def get_output_from_process(proc, read_size=None, asynchronous=False, print_depr
@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
Expand All @@ -149,6 +149,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)
"""

_log.deprecated("run_cmd is deprecated, use run_shell_cmd from easybuild.tools.run instead", '6.0')
Expand Down Expand Up @@ -228,6 +229,16 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True

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:
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):
exec_cmd = None
Expand All @@ -237,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()})
Expand Down
41 changes: 39 additions & 2 deletions easybuild/tools/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@
"""
import datetime
import difflib
import filecmp
import glob
import hashlib
import inspect
import itertools
import os
import platform
import re
import shutil
import signal
Expand All @@ -61,6 +63,7 @@
import urllib.request as std_urllib

from easybuild.base import fancylogger
from easybuild.tools import LooseVersion
# import build_log must stay, to use of EasyBuildLog
from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, CWD_NOTFOUND_ERROR
from easybuild.tools.build_log import dry_run_msg, print_msg, print_warning
Expand Down Expand Up @@ -2427,8 +2430,42 @@ 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 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)
# catch the more general OSError instead of PermissionError,
# since Python 2.7 doesn't support PermissionError
except OSError as err:
# 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())
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)

try:
# re-enable user write permissions in target, copy xattrs, then remove write perms again
adjust_permissions(target_path, stat.S_IWUSR)
shutil._copyxattr(path, target_path)
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)

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))
Expand Down
3 changes: 3 additions & 0 deletions easybuild/tools/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,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 = LooseVersion(self.version)
Expand Down
2 changes: 1 addition & 1 deletion easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -2046,7 +2046,7 @@ def set_tmpdir(tmpdir=None, raise_error=False):
os.chmod(tmptest_file, 0o700)
res = run_shell_cmd(tmptest_file, fail_on_error=False, in_dry_run=True, hidden=True, stream_output=False,
with_hooks=False)
if res.exit_code:
if res.exit_code != EasyBuildExit.SUCCESS:
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:
Expand Down
46 changes: 46 additions & 0 deletions test/framework/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
@author: Maxime Boissonneault (Compute Canada, Universite Laval)
"""
import datetime
import filecmp
import glob
import logging
import os
Expand All @@ -52,6 +53,8 @@
from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.config import IGNORE, ERROR, WARN, build_option, update_build_option
from easybuild.tools.multidiff import multidiff
from easybuild.tools.run import run_shell_cmd
from easybuild.tools.systemtools import LINUX, get_os_type


class FileToolsTest(EnhancedTestCase):
Expand Down Expand Up @@ -1976,6 +1979,49 @@ 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:
with self.mocked_stdout_stderr():
res = run_shell_cmd(cmd, fail_on_error=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 res.exit_code == 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
with self.mocked_stdout_stderr():
res = run_shell_cmd(cmd, fail_on_error=False)
self.assertEqual(res.exit_code, 0)
self.assertTrue(res.output.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')
Expand Down
32 changes: 31 additions & 1 deletion test/framework/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2087,12 +2087,42 @@ 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():
self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir)

def test_run_cmd_sysroot(self):
"""Test with_sysroot option of run_cmd function."""

# use of run_cmd/run_cmd_qa is deprecated, so we need to allow it here
self.allow_deprecated_behaviour()

# 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)

with self.mocked_stdout_stderr():
(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
with self.mocked_stdout_stderr():
(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 """
Expand Down

0 comments on commit d3c0127

Please sign in to comment.