Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync with develop (20240923) #4654

Merged
merged 29 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4adb577
fix issue when copying read-only files using shutil.copy2
jfgrimm Sep 16, 2024
8cba9e9
log when ignoring permissions error
jfgrimm Sep 16, 2024
55b4c8b
python2 support
jfgrimm Sep 17, 2024
a198ad5
fix import
jfgrimm Sep 17, 2024
5d1f2ce
actually compare file contents in case of permissions error
jfgrimm Sep 17, 2024
7bf4073
Make sure the run_cmd respects sysroot when picking the shell, i.e. u…
Sep 18, 2024
73a53a8
Make sure we only check for sysroot after set_up_configuration is cal…
Sep 18, 2024
347e64c
Also add with_sysroot=False here
Sep 18, 2024
165fe79
Fix indent
Sep 18, 2024
df9827d
more explicit python version checking to ignore permissions error
jfgrimm Sep 18, 2024
202ae52
long line
jfgrimm Sep 18, 2024
4a4312e
manually copy xattrs where appropriate
jfgrimm Sep 18, 2024
150d7c3
typo
jfgrimm Sep 18, 2024
f7c3cb1
be a bit more careful with sysroot in run_cmd, add logging + add dedi…
boegel Sep 18, 2024
3b7295f
Merge pull request #4646 from casparvl/exec_command_with_sysroot
boegel Sep 18, 2024
a3b26f6
implement test for copy_file to copy read-only file with extended att…
boegel Sep 18, 2024
23b5b78
fix condition in copy_file when hitting PermissionError when copying …
boegel Sep 18, 2024
747eddd
Merge pull request #1 from boegel/read-only-copy2
jfgrimm Sep 18, 2024
bf0af10
Merge pull request #4642 from jfgrimm/read-only-copy2
boegel Sep 19, 2024
5e7531e
set $LMOD_TERSE_DECORATIONS to 'no' to avoid additional info in outpu…
boegel Sep 21, 2024
9ff6dac
Merge pull request #4648 from boegel/LMOD_TERSE_DECORATIONS
bedroge Sep 21, 2024
c2c280e
prepare release notes for EasyBuild v4.9.4 + bump version to 4.9.4
boegel Sep 21, 2024
594136c
Merge pull request #4649 from boegel/eb494
jfgrimm Sep 22, 2024
71cbb00
Merge pull request #4650 from easybuilders/4.9.x
boegel Sep 22, 2024
a3bdfef
bump version to 4.9.5dev
boegel Sep 22, 2024
4b7124f
Update RELEASE_NOTES
verdurin Sep 22, 2024
36db0c7
Merge pull request #4651 from boegel/develop
verdurin Sep 22, 2024
966fba0
Merge branch 'develop' into 5.0.x
boegel Sep 23, 2024
58f39ef
relax error pattern used in test_run_shell_cmd_delete_cwd so test als…
boegel Sep 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading