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

rename run function to run_shell_cmd #4335

Merged
merged 2 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 9 additions & 9 deletions easybuild/tools/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
from easybuild.tools.config import ERROR, GENERIC_EASYBLOCK_PKG, IGNORE, WARN, build_option, install_path
from easybuild.tools.output import PROGRESS_BAR_DOWNLOAD_ONE, start_progress_bar, stop_progress_bar, update_progress_bar
from easybuild.tools.hooks import load_source
from easybuild.tools.run import run
from easybuild.tools.run import run_shell_cmd
from easybuild.tools.utilities import natural_keys, nub, remove_unwanted_chars, trace_msg

try:
Expand Down Expand Up @@ -470,7 +470,7 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced
if extra_options:
cmd = f"{cmd} {extra_options}"

run(cmd, in_dry_run=forced, hidden=not trace)
run_shell_cmd(cmd, in_dry_run=forced, hidden=not trace)

# note: find_base_dir also changes into the base dir!
base_dir = find_base_dir()
Expand Down Expand Up @@ -1619,7 +1619,7 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git=False
backup_option = '-b ' if build_option('backup_patched_files') else ''
patch_cmd = f"patch {backup_option} -p{level} -i {abs_patch_file}"

res = run(patch_cmd, fail_on_error=False, hidden=True, work_dir=abs_dest)
res = run_shell_cmd(patch_cmd, fail_on_error=False, hidden=True, work_dir=abs_dest)

if res.exit_code:
msg = f"Couldn't apply patch file {patch_file}. "
Expand Down Expand Up @@ -2165,7 +2165,7 @@ def move_logs(src_logfile, target_logfile):
_log.info(f"Moved log file {src_logfile} to {new_log_path}")

if zip_log_cmd:
run(f"{zip_log_cmd} {new_log_path}")
run_shell_cmd(f"{zip_log_cmd} {new_log_path}")
_log.info(f"Zipped log {new_log_path} using '{zip_log_cmd}'")

except (IOError, OSError) as err:
Expand Down Expand Up @@ -2638,7 +2638,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config):

tmpdir = tempfile.mkdtemp()

run(' '.join(clone_cmd), hidden=True, verbose_dry_run=True, work_dir=tmpdir)
run_shell_cmd(' '.join(clone_cmd), hidden=True, verbose_dry_run=True, work_dir=tmpdir)

# If the clone is done into a specified name, change repo_name
if clone_into:
Expand All @@ -2651,14 +2651,14 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
checkout_cmd.extend(['&&', 'git', 'submodule', 'update', '--init', '--recursive'])

work_dir = os.path.join(tmpdir, repo_name) if repo_name else tmpdir
run(' '.join(checkout_cmd), work_dir=work_dir, hidden=True, verbose_dry_run=True)
run_shell_cmd(' '.join(checkout_cmd), work_dir=work_dir, hidden=True, verbose_dry_run=True)

elif not build_option('extended_dry_run'):
# If we wanted to get a tag make sure we actually got a tag and not a branch with the same name
# This doesn't make sense in dry-run mode as we don't have anything to check
cmd = "git describe --exact-match --tags HEAD"
work_dir = os.path.join(tmpdir, repo_name) if repo_name else tmpdir
res = run(cmd, fail_on_error=False, work_dir=work_dir, hidden=True, verbose_dry_run=True)
res = run_shell_cmd(cmd, fail_on_error=False, work_dir=work_dir, hidden=True, verbose_dry_run=True)

if res.exit_code != 0 or tag not in res.output.splitlines():
msg = f"Tag {tag} was not downloaded in the first try due to {url}/{repo_name} containing a branch"
Expand All @@ -2679,14 +2679,14 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
if recursive:
cmds.append("git submodule update --init --recursive")
for cmd in cmds:
run(cmd, work_dir=work_dir, hidden=True, verbose_dry_run=True)
run_shell_cmd(cmd, work_dir=work_dir, hidden=True, verbose_dry_run=True)

# create an archive and delete the git repo directory
if keep_git_dir:
tar_cmd = ['tar', 'cfvz', targetpath, repo_name]
else:
tar_cmd = ['tar', 'cfvz', targetpath, '--exclude', '.git', repo_name]
run(' '.join(tar_cmd), work_dir=tmpdir, hidden=True, verbose_dry_run=True)
run_shell_cmd(' '.join(tar_cmd), work_dir=tmpdir, hidden=True, verbose_dry_run=True)

# cleanup (repo_name dir does not exist in dry run mode)
remove(tmpdir)
Expand Down
11 changes: 6 additions & 5 deletions easybuild/tools/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
from easybuild.tools.environment import ORIG_OS_ENVIRON, restore_env, setvar, unset_env_vars
from easybuild.tools.filetools import convert_name, mkdir, normalize_path, path_matches, read_file, which, write_file
from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX
from easybuild.tools.run import run
from easybuild.tools.run import run_shell_cmd
from easybuild.tools.utilities import get_subclasses, nub

# software root/version environment variable name prefixes
Expand Down Expand Up @@ -312,7 +312,7 @@ def check_module_function(self, allow_mismatch=False, regex=None):
output, exit_code = None, 1
else:
cmd = "type module"
res = run(cmd, fail_on_error=False, in_dry_run=False, hidden=True)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=False, hidden=True)
output, exit_code = res.output, res.exit_code

if regex is None:
Expand Down Expand Up @@ -824,8 +824,8 @@ def run_module(self, *args, **kwargs):
cmd_list = self.compose_cmd_list(args)
cmd = ' '.join(cmd_list)
# note: module commands are always run in dry mode, and are kept hidden in trace and dry run output
res = run(cmd_list, env=environ, fail_on_error=False, shell=False, split_stderr=True,
hidden=True, in_dry_run=True)
res = run_shell_cmd(cmd_list, env=environ, fail_on_error=False, shell=False, split_stderr=True,
hidden=True, in_dry_run=True)

# stdout will contain python code (to change environment etc)
# stderr will contain text (just like the normal module command)
Expand Down Expand Up @@ -1426,7 +1426,8 @@ def update(self):
cmd = ' '.join(cmd_list)
self.log.debug("Running command '%s'...", cmd)

res = run(cmd_list, env=os.environ, fail_on_error=False, shell=False, split_stderr=True, hidden=True)
res = run_shell_cmd(cmd_list, env=os.environ, fail_on_error=False, shell=False, split_stderr=True,
hidden=True)
stdout, stderr = res.output, res.stderr

if stderr:
Expand Down
8 changes: 4 additions & 4 deletions easybuild/tools/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ def cache_aware_func(cmd, *args, **kwargs):


@run_cache
def run(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, shell=True,
output_file=False, stream_output=False, asynchronous=False, with_hooks=True,
qa_patterns=None, qa_wait_patterns=None):
def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, shell=True,
output_file=False, stream_output=False, asynchronous=False, with_hooks=True,
qa_patterns=None, qa_wait_patterns=None):
"""
Run specified (interactive) shell command, and capture output + exit code.

Expand Down
38 changes: 19 additions & 19 deletions easybuild/tools/systemtools.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
from collections import OrderedDict
from ctypes.util import find_library
from socket import gethostname
from easybuild.tools.run import subprocess_popen_text

# pkg_resources is provided by the setuptools Python package,
# which we really want to keep as an *optional* dependency
Expand All @@ -65,7 +64,7 @@
from easybuild.tools.build_log import EasyBuildError, print_warning
from easybuild.tools.config import IGNORE
from easybuild.tools.filetools import is_readable, read_file, which
from easybuild.tools.run import run
from easybuild.tools.run import run_shell_cmd, subprocess_popen_text


_log = fancylogger.getLogger('systemtools', fname=False)
Expand Down Expand Up @@ -274,7 +273,7 @@ def get_avail_core_count():
core_cnt = int(sum(sched_getaffinity()))
else:
# BSD-type systems
res = run('sysctl -n hw.ncpu', in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd('sysctl -n hw.ncpu', in_dry_run=True, hidden=True, with_hooks=False)
try:
if int(res.output) > 0:
core_cnt = int(res.output)
Expand Down Expand Up @@ -311,7 +310,7 @@ 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)
res = run(cmd, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, in_dry_run=True, hidden=True, with_hooks=False)
if res.exit_code == 0:
memtotal = int(res.output.strip()) // (1024**2)

Expand Down Expand Up @@ -393,14 +392,14 @@ def get_cpu_vendor():

elif os_type == DARWIN:
cmd = "sysctl -n machdep.cpu.vendor"
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
out = res.output.strip()
if res.exit_code == 0 and out in VENDOR_IDS:
vendor = VENDOR_IDS[out]
_log.debug("Determined CPU vendor on DARWIN as being '%s' via cmd '%s" % (vendor, cmd))
else:
cmd = "sysctl -n machdep.cpu.brand_string"
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
out = res.output.strip().split(' ')[0]
if res.exit_code == 0 and out in CPU_VENDORS:
vendor = out
Expand Down Expand Up @@ -503,7 +502,7 @@ def get_cpu_model():

elif os_type == DARWIN:
cmd = "sysctl -n machdep.cpu.brand_string"
res = run(cmd, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, in_dry_run=True, hidden=True, with_hooks=False)
if res.exit_code == 0:
model = res.output.strip()
_log.debug("Determined CPU model on Darwin using cmd '%s': %s" % (cmd, model))
Expand Down Expand Up @@ -548,7 +547,7 @@ 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)
res = run(cmd, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, in_dry_run=True, hidden=True, with_hooks=False)
out = res.output.strip()
cpu_freq = None
if res.exit_code == 0 and out:
Expand Down Expand Up @@ -596,7 +595,7 @@ def get_cpu_features():
for feature_set in ['extfeatures', 'features', 'leaf7_features']:
cmd = "sysctl -n machdep.cpu.%s" % feature_set
_log.debug("Trying to determine CPU features on Darwin via cmd '%s'", cmd)
res = run(cmd, in_dry_run=True, hidden=True, fail_on_error=False, with_hooks=False)
res = run_shell_cmd(cmd, in_dry_run=True, hidden=True, fail_on_error=False, with_hooks=False)
if res.exit_code == 0:
cpu_feat.extend(res.output.strip().lower().split())

Expand All @@ -623,7 +622,7 @@ 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)
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
if res.exit_code == 0:
for line in res.output.strip().split('\n'):
nvidia_gpu_info = gpu_info.setdefault('NVIDIA', {})
Expand All @@ -641,13 +640,13 @@ def get_gpu_info():
try:
cmd = "rocm-smi --showdriverversion --csv"
_log.debug("Trying to determine AMD GPU driver on Linux via cmd '%s'", cmd)
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
if res.exit_code == 0:
amd_driver = res.output.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)
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
if res.exit_code == 0:
for line in res.output.strip().split('\n')[1:]:
amd_card_series = line.split(',')[1]
Expand Down Expand Up @@ -866,7 +865,7 @@ def check_os_dependency(dep):
pkg_cmd_flag.get(pkg_cmd),
dep,
])
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True)
found = res.exit_code == 0
if found:
break
Expand All @@ -878,7 +877,7 @@ def check_os_dependency(dep):
# try locate if it's available
if not found and which('locate'):
cmd = 'locate -c --regexp "/%s$"' % dep
res = run(cmd, fail_on_error=False, in_dry_run=True, hidden=True)
res = run_shell_cmd(cmd, fail_on_error=False, in_dry_run=True, hidden=True)
try:
found = (res.exit_code == 0 and int(res.output.strip()) > 0)
except ValueError:
Expand All @@ -893,7 +892,8 @@ def get_tool_version(tool, version_option='--version', ignore_ec=False):
Get output of running version option for specific command line tool.
Output is returned as a single-line string (newlines are replaced by '; ').
"""
res = run(' '.join([tool, version_option]), fail_on_error=False, in_dry_run=True, hidden=True, with_hooks=False)
res = run_shell_cmd(' '.join([tool, version_option]), fail_on_error=False, in_dry_run=True,
hidden=True, with_hooks=False)
if not ignore_ec and res.exit_code:
_log.warning("Failed to determine version of %s using '%s %s': %s" % (tool, tool, version_option, res.output))
return UNKNOWN
Expand All @@ -905,7 +905,7 @@ def get_gcc_version():
"""
Process `gcc --version` and return the GCC version.
"""
res = run('gcc --version', fail_on_error=False, in_dry_run=True, hidden=True)
res = run_shell_cmd('gcc --version', fail_on_error=False, in_dry_run=True, hidden=True)
gcc_ver = None
if res.exit_code:
_log.warning("Failed to determine the version of GCC: %s", res.output)
Expand Down Expand Up @@ -961,7 +961,7 @@ def get_linked_libs_raw(path):
or None for other types of files.
"""

res = run("file %s" % path, fail_on_error=False, hidden=True)
res = run_shell_cmd("file %s" % path, fail_on_error=False, hidden=True)
if res.exit_code:
fail_msg = "Failed to run 'file %s': %s" % (path, res.output)
_log.warning(fail_msg)
Expand Down Expand Up @@ -996,7 +996,7 @@ def get_linked_libs_raw(path):
# take into account that 'ldd' may fail for strange reasons,
# like printing 'not a dynamic executable' when not enough memory is available
# (see also https://bugzilla.redhat.com/show_bug.cgi?id=1817111)
res = run(linked_libs_cmd, fail_on_error=False, hidden=True)
res = run_shell_cmd(linked_libs_cmd, fail_on_error=False, hidden=True)
if res.exit_code == 0:
linked_libs_out = res.output
else:
Expand Down Expand Up @@ -1178,7 +1178,7 @@ def get_default_parallelism():
# No cache -> Calculate value from current system values
par = get_avail_core_count()
# determine max user processes via ulimit -u
res = run("ulimit -u", in_dry_run=True, hidden=True)
res = run_shell_cmd("ulimit -u", in_dry_run=True, hidden=True)
try:
if res.output.startswith("unlimited"):
maxuserproc = 2 ** 32 - 1
Expand Down
10 changes: 5 additions & 5 deletions test/framework/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from easybuild.tools.options import set_up_configuration
from easybuild.tools.filetools import mkdir
from easybuild.tools.modules import modules_tool
from easybuild.tools.run import run, run_cmd
from easybuild.tools.run import run_shell_cmd, run_cmd


class EasyBuildLibTest(TestCase):
Expand Down Expand Up @@ -85,18 +85,18 @@ def test_run_cmd(self):
self.assertEqual(ec, 0)
self.assertEqual(out, 'hello\n')

def test_run(self):
"""Test use of run function in the context of using EasyBuild framework as a library."""
def test_run_shell_cmd(self):
"""Test use of run_shell_cmd function in the context of using EasyBuild framework as a library."""

error_pattern = r"Undefined build option: .*"
error_pattern += r" Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
self.assertErrorRegex(EasyBuildError, error_pattern, run, "echo hello")
self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, "echo hello")

self.configure()

# runworks fine if set_up_configuration was called first
self.mock_stdout(True)
res = run("echo hello")
res = run_shell_cmd("echo hello")
self.mock_stdout(False)
self.assertEqual(res.exit_code, 0)
self.assertEqual(res.output, 'hello\n')
Expand Down
4 changes: 2 additions & 2 deletions test/framework/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
from easybuild.tools.modules import EnvironmentModules, EnvironmentModulesC, EnvironmentModulesTcl, Lmod, NoModulesTool
from easybuild.tools.modules import curr_module_paths, get_software_libdir, get_software_root, get_software_version
from easybuild.tools.modules import invalidate_module_caches_for, modules_tool, reset_module_caches
from easybuild.tools.run import run
from easybuild.tools.run import run_shell_cmd


# number of modules included for testing purposes
Expand Down Expand Up @@ -1332,7 +1332,7 @@ def test_module_use_bash(self):
self.assertIn(modules_dir, modulepath)

with self.mocked_stdout_stderr():
res = run("bash -c 'echo MODULEPATH: $MODULEPATH'")
res = run_shell_cmd("bash -c 'echo MODULEPATH: $MODULEPATH'")
self.assertEqual(res.output.strip(), f"MODULEPATH: {modulepath}")
self.assertIn(modules_dir, res.output)

Expand Down