Merge pull request #4327 from boegel/run_filetools

implement support in '`run`' function for running command in different working directory + switch to `run` function in filetools

easybuild/tools/filetools.py

@@ -62,11 +62,11 @@
 import urllib.request as std_urllib
 
 from easybuild.base import fancylogger
-from easybuild.tools import 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 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.run import run
 from easybuild.tools.utilities import natural_keys, nub, remove_unwanted_chars, trace_msg
 
 try:
@@ -445,32 +445,32 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced
     """
 
     if not os.path.isfile(fn) and not build_option('extended_dry_run'):
-        raise EasyBuildError("Can't extract file %s: no such file", fn)
+        raise EasyBuildError(f"Can't extract file {fn}: no such file")
 
     mkdir(dest, parents=True)
 
     # use absolute pathnames from now on
     abs_dest = os.path.abspath(dest)
 
     # change working directory
-    _log.debug("Unpacking %s in directory %s", fn, abs_dest)
+    _log.debug(f"Unpacking {fn} in directory {abs_dest}")
     cwd = change_dir(abs_dest)
 
     if cmd:
         # complete command template with filename
         cmd = cmd % fn
-        _log.debug("Using specified command to unpack %s: %s", fn, cmd)
+        _log.debug("Using specified command to unpack {fn}: {cmd}")
     else:
         cmd = extract_cmd(fn, overwrite=overwrite)
-        _log.debug("Using command derived from file extension to unpack %s: %s", fn, cmd)
+        _log.debug("Using command derived from file extension to unpack {fn}: {cmd}")
 
     if not cmd:
-        raise EasyBuildError("Can't extract file %s with unknown filetype", fn)
+        raise EasyBuildError("Can't extract file {fn} with unknown filetype")
 
     if extra_options:
-        cmd = "%s %s" % (cmd, extra_options)
+        cmd = f"{cmd} {extra_options}"
 
-    run.run_cmd(cmd, simple=True, force_in_dry_run=forced, trace=trace)
+    run(cmd, in_dry_run=forced, hidden=not trace)
 
     # note: find_base_dir also changes into the base dir!
     base_dir = find_base_dir()
@@ -479,7 +479,7 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced
     # change back to where we came from (unless that was a non-existing directory)
     if not change_into_dir:
         if cwd is None:
-            raise EasyBuildError("Can't change back to non-existing directory after extracting %s in %s", fn, dest)
+            raise EasyBuildError(f"Can't change back to non-existing directory after extracting {fn} in {dest}")
         else:
             change_dir(cwd)
 
@@ -1547,27 +1547,27 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git=False
     if build_option('extended_dry_run'):
         # skip checking of files in dry run mode
         patch_filename = os.path.basename(patch_file)
-        dry_run_msg("* applying patch file %s" % patch_filename, silent=build_option('silent'))
+        dry_run_msg(f"* applying patch file {patch_filename}", silent=build_option('silent'))
 
     elif not os.path.isfile(patch_file):
-        raise EasyBuildError("Can't find patch %s: no such file", patch_file)
+        raise EasyBuildError(f"Can't find patch {patch_file}: no such file")
 
     elif fn and not os.path.isfile(fn):
-        raise EasyBuildError("Can't patch file %s: no such file", fn)
+        raise EasyBuildError(f"Can't patch file {fn}: no such file")
 
     # copy missing files
     if copy:
         if build_option('extended_dry_run'):
-            dry_run_msg("  %s copied to %s" % (patch_file, dest), silent=build_option('silent'))
+            dry_run_msg(f"  {patch_file} copied to {dest}", silent=build_option('silent'))
         else:
             copy_file(patch_file, dest)
-            _log.debug("Copied patch %s to dir %s" % (patch_file, dest))
+            _log.debug(f"Copied patch {patch_file} to dir {dest}")
 
         # early exit, work is done after copying
         return True
 
     elif not os.path.isdir(dest):
-        raise EasyBuildError("Can't patch directory %s: no such directory", dest)
+        raise EasyBuildError(f"Can't patch directory {dest}: no such directory")
 
     # use absolute paths
     abs_patch_file = os.path.abspath(patch_file)
@@ -1582,14 +1582,14 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git=False
         patch_subextension = os.path.splitext(patch_stem)[1]
         if patch_subextension == ".patch":
             workdir = tempfile.mkdtemp(prefix='eb-patch-')
-            _log.debug("Extracting the patch to: %s", workdir)
+            _log.debug(f"Extracting the patch to: {workdir}")
             # extracting the patch
             extracted_dir = extract_file(abs_patch_file, workdir, change_into_dir=False)
             abs_patch_file = os.path.join(extracted_dir, patch_stem)
 
     if use_git:
         verbose = '--verbose ' if build_option('debug') else ''
-        patch_cmd = "git apply %s%s" % (verbose, abs_patch_file)
+        patch_cmd = f"git apply {verbose}{abs_patch_file}"
     else:
         if level is None and build_option('extended_dry_run'):
             level = '<derived>'
@@ -1602,27 +1602,30 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git=False
             patched_files = det_patched_files(path=abs_patch_file)
 
             if not patched_files:
-                raise EasyBuildError("Can't guess patchlevel from patch %s: no testfile line found in patch",
-                                     abs_patch_file)
+                msg = f"Can't guess patchlevel from patch {abs_patch_file}: no testfile line found in patch"
+                raise EasyBuildError(msg)
 
             level = guess_patch_level(patched_files, abs_dest)
 
             if level is None:  # level can also be 0 (zero), so don't use "not level"
                 # no match
-                raise EasyBuildError("Can't determine patch level for patch %s from directory %s", patch_file, abs_dest)
+                raise EasyBuildError(f"Can't determine patch level for patch {patch_file} from directory {abs_dest}")
             else:
-                _log.debug("Guessed patch level %d for patch %s" % (level, patch_file))
+                _log.debug(f"Guessed patch level {level} for patch {patch_file}")
 
         else:
-            _log.debug("Using specified patch level %d for patch %s" % (level, patch_file))
+            _log.debug(f"Using specified patch level {level} for patch {patch_file}")
 
         backup_option = '-b ' if build_option('backup_patched_files') else ''
-        patch_cmd = "patch " + backup_option + "-p%s -i %s" % (level, abs_patch_file)
+        patch_cmd = f"patch {backup_option} -p{level} -i {abs_patch_file}"
 
-    out, ec = run.run_cmd(patch_cmd, simple=False, path=abs_dest, log_ok=False, trace=False)
+    res = run(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}. "
+        msg += f"Process exited with code {res.exit_code}: {res.output}"
+        raise EasyBuildError(msg)
 
-    if ec:
-        raise EasyBuildError("Couldn't apply patch file %s. Process exited with code %s: %s", patch_file, ec, out)
     return True
 
 
@@ -2148,7 +2151,7 @@ def move_logs(src_logfile, target_logfile):
     try:
 
         # there may be multiple log files, due to log rotation
-        app_logs = glob.glob('%s*' % src_logfile)
+        app_logs = glob.glob(f'{src_logfile}*')
         for app_log in app_logs:
             # retain possible suffix
             new_log_path = target_logfile + app_log[src_logfile_len:]
@@ -2159,11 +2162,11 @@ def move_logs(src_logfile, target_logfile):
 
             # move log to target path
             move_file(app_log, new_log_path)
-            _log.info("Moved log file %s to %s" % (src_logfile, new_log_path))
+            _log.info(f"Moved log file {src_logfile} to {new_log_path}")
 
             if zip_log_cmd:
-                run.run_cmd("%s %s" % (zip_log_cmd, new_log_path))
-                _log.info("Zipped log %s using '%s'", new_log_path, zip_log_cmd)
+                run(f"{zip_log_cmd} {new_log_path}")
+                _log.info(f"Zipped log {new_log_path} using '{zip_log_cmd}'")
 
     except (IOError, OSError) as err:
         raise EasyBuildError("Failed to move log file(s) %s* to new log file %s*: %s",
@@ -2253,21 +2256,6 @@ def decode_class_name(name):
         return decode_string(name)
 
 
-def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True, log_output=False, path=None):
-    """NO LONGER SUPPORTED: use run_cmd from easybuild.tools.run instead"""
-    _log.nosupport("run_cmd was moved from easybuild.tools.filetools to easybuild.tools.run", '2.0')
-
-
-def run_cmd_qa(cmd, qa, no_qa=None, log_ok=True, log_all=False, simple=False, regexp=True, std_qa=None, path=None):
-    """NO LONGER SUPPORTED: use run_cmd_qa from easybuild.tools.run instead"""
-    _log.nosupport("run_cmd_qa was moved from easybuild.tools.filetools to easybuild.tools.run", '2.0')
-
-
-def parse_log_for_error(txt, regExp=None, stdout=True, msg=None):
-    """NO LONGER SUPPORTED: use parse_log_for_error from easybuild.tools.run instead"""
-    _log.nosupport("parse_log_for_error was moved from easybuild.tools.filetools to easybuild.tools.run", '2.0')
-
-
 def det_size(path):
     """
     Determine total size of given filepath (in bytes).
@@ -2592,7 +2580,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
     """
     # sanity check on git_config value being passed
     if not isinstance(git_config, dict):
-        raise EasyBuildError("Found unexpected type of value for 'git_config' argument: %s" % type(git_config))
+        raise EasyBuildError("Found unexpected type of value for 'git_config' argument: {type(git_config)}")
 
     # Making a copy to avoid modifying the object with pops
     git_config = git_config.copy()
@@ -2606,7 +2594,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
 
     # input validation of git_config dict
     if git_config:
-        raise EasyBuildError("Found one or more unexpected keys in 'git_config' specification: %s", git_config)
+        raise EasyBuildError("Found one or more unexpected keys in 'git_config' specification: {git_config}")
 
     if not repo_name:
         raise EasyBuildError("repo_name not specified in git_config parameter")
@@ -2643,14 +2631,14 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
         # checkout is done separately below for specific commits
         clone_cmd.append('--no-checkout')
 
-    clone_cmd.append('%s/%s.git' % (url, repo_name))
+    clone_cmd.append(f'{url}/{repo_name}.git')
 
     if clone_into:
-        clone_cmd.append('%s' % clone_into)
+        clone_cmd.append(clone_into)
 
     tmpdir = tempfile.mkdtemp()
-    cwd = change_dir(tmpdir)
-    run.run_cmd(' '.join(clone_cmd), log_all=True, simple=True, regexp=False, trace=False)
+
+    run(' '.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:
@@ -2662,43 +2650,45 @@ def get_source_tarball_from_git(filename, targetdir, git_config):
         if recursive:
             checkout_cmd.extend(['&&', 'git', 'submodule', 'update', '--init', '--recursive'])
 
-        run.run_cmd(' '.join(checkout_cmd), log_all=True, simple=True, regexp=False, path=repo_name, trace=False)
+        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)
 
     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'
-        # Note: Disable logging to also disable the error handling in run_cmd
-        (out, ec) = run.run_cmd(cmd, log_ok=False, log_all=False, regexp=False, path=repo_name, trace=False)
-        if ec != 0 or tag not in out.splitlines():
-            print_warning('Tag %s was not downloaded in the first try due to %s/%s containing a branch'
-                          ' with the same name. You might want to alert the maintainers of %s about that issue.',
-                          tag, url, repo_name, repo_name)
+        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)
+
+        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"
+            msg += f" with the same name. You might want to alert the maintainers of {repo_name} about that issue."
+            print_warning(msg)
+
             cmds = []
 
             if not keep_git_dir:
                 # make the repo unshallow first;
                 # this is equivalent with 'git fetch -unshallow' in Git 1.8.3+
                 # (first fetch seems to do nothing, unclear why)
-                cmds.append('git fetch --depth=2147483647 && git fetch --depth=2147483647')
+                cmds.append("git fetch --depth=2147483647 && git fetch --depth=2147483647")
 
-            cmds.append('git checkout refs/tags/' + tag)
+            cmds.append(f"git checkout refs/tags/{tag}")
             # Clean all untracked files, e.g. from left-over submodules
-            cmds.append('git clean --force -d -x')
+            cmds.append("git clean --force -d -x")
             if recursive:
-                cmds.append('git submodule update --init --recursive')
+                cmds.append("git submodule update --init --recursive")
             for cmd in cmds:
-                run.run_cmd(cmd, log_all=True, simple=True, regexp=False, path=repo_name, trace=False)
+                run(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.run_cmd(' '.join(tar_cmd), log_all=True, simple=True, regexp=False, trace=False)
+    run(' '.join(tar_cmd), work_dir=tmpdir, hidden=True, verbose_dry_run=True)
 
     # cleanup (repo_name dir does not exist in dry run mode)
-    change_dir(cwd)
     remove(tmpdir)
 
     return targetpath

easybuild/tools/run.py

@@ -108,7 +108,7 @@ 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, work_dir=None, shell=True,
+        hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, shell=True,
         output_file=False, stream_output=False, asynchronous=False,
         qa_patterns=None, qa_wait_patterns=None):
     """
@@ -120,6 +120,7 @@ def run(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
     :param env: environment to use to run command (if None, inherit current process environment)
     :param hidden: do not show command in terminal output (when using --trace, or with --extended-dry-run / -x)
     :param in_dry_run: also run command in dry run mode
+    :param verbose_dry_run: show that command is run in dry run mode (overrules 'hidden')
     :param work_dir: working directory to run command in (current working directory if None)
     :param shell: execute command through bash shell (enabled by default)
     :param output_file: collect command output in temporary output file
@@ -135,7 +136,7 @@ def run(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
     """
 
     # temporarily raise a NotImplementedError until all options are implemented
-    if any((work_dir, stream_output, asynchronous)):
+    if any((stream_output, asynchronous)):
         raise NotImplementedError
 
     if qa_patterns or qa_wait_patterns:
@@ -162,7 +163,7 @@ def run(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
 
     # early exit in 'dry run' mode, after printing the command that would be run (unless 'hidden' is enabled)
     if not in_dry_run and build_option('extended_dry_run'):
-        if not hidden:
+        if not hidden or verbose_dry_run:
             silent = build_option('silent')
             msg = f"  running command \"{cmd_str}\"\n"
             msg += f"  (in {work_dir})"
@@ -187,7 +188,7 @@ def run(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
 
     _log.info(f"Running command '{cmd_str}' in {work_dir}")
     proc = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=stderr, check=fail_on_error,
-                          env=env, input=stdin, shell=shell, executable=executable)
+                          cwd=work_dir, env=env, input=stdin, shell=shell, executable=executable)
 
     # return output as a regular string rather than a byte sequence (and non-UTF-8 characters get stripped out)
     output = proc.stdout.decode('utf-8', 'ignore')

test/framework/filetools.py

@@ -2831,7 +2831,7 @@ def run_check():
             r'  running command "git clone --no-checkout %(git_repo)s"',
             r"  \(in /.*\)",
             r'  running command "git checkout 8456f86 && git submodule update --init --recursive"',
-            r"  \(in testrepository\)",
+            r"  \(in /.*/testrepository\)",
             r'  running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"',
             r"  \(in /.*\)",
         ]) % git_repo
@@ -2842,7 +2842,7 @@ def run_check():
             r'  running command "git clone --no-checkout %(git_repo)s"',
             r"  \(in /.*\)",
             r'  running command "git checkout 8456f86"',
-            r"  \(in testrepository\)",
+            r"  \(in /.*/testrepository\)",
             r'  running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"',
             r"  \(in /.*\)",
         ]) % git_repo