diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index f42d3ca4c4..744e51ce3d 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1911,7 +1911,9 @@ def extract_step(self): """ for src in self.src: self.log.info("Unpacking source %s" % src['name']) - srcdir = extract_file(src['path'], self.builddir, cmd=src['cmd'], extra_options=self.cfg['unpack_options']) + srcdir = extract_file(src['path'], self.builddir, cmd=src['cmd'], + extra_options=self.cfg['unpack_options'], change_into_dir=False) + change_dir(srcdir) if srcdir: self.src[self.src.index(src)]['finalpath'] = srcdir else: diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 35b1bf4407..277a59fb45 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -103,7 +103,9 @@ def run(self, unpack_src=False): # unpack file if desired if unpack_src: targetdir = os.path.join(self.master.builddir, remove_unwanted_chars(self.name)) - self.ext_dir = extract_file("%s" % self.src, targetdir, extra_options=self.unpack_options) + self.ext_dir = extract_file(self.src, targetdir, extra_options=self.unpack_options, + change_into_dir=False) + change_dir(self.ext_dir) if self.start_dir and os.path.isdir(self.start_dir): self.log.debug("Using start_dir: %s", self.start_dir) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index b7149cc553..97002607a7 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -371,7 +371,7 @@ def change_dir(path): return cwd -def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced=False): +def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced=False, change_into_dir=None): """ Extract file at given path to specified directory :param fn: path to file to extract @@ -380,8 +380,16 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced :param extra_options: extra options to pass to extract command :param overwrite: overwrite existing unpacked file :param forced: force extraction in (extended) dry run mode + :param change_into_dir: change into resulting directory; + None (current default) implies True, but this is deprecated, + this named argument should be set to False or True explicitely + (in a future major release, default will be changed to False) :return: path to directory (in case of success) """ + if change_into_dir is None: + _log.deprecated("extract_file function was called without specifying value for change_into_dir", '5.0') + change_into_dir = True + if not os.path.isfile(fn) and not build_option('extended_dry_run'): raise EasyBuildError("Can't extract file %s: no such file", fn) @@ -391,8 +399,8 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced abs_dest = os.path.abspath(dest) # change working directory - _log.debug("Unpacking %s in directory %s.", fn, abs_dest) - change_dir(abs_dest) + _log.debug("Unpacking %s in directory %s", fn, abs_dest) + cwd = change_dir(abs_dest) if not cmd: cmd = extract_cmd(fn, overwrite=overwrite) @@ -407,7 +415,18 @@ def extract_file(fn, dest, cmd=None, extra_options=None, overwrite=False, forced run.run_cmd(cmd, simple=True, force_in_dry_run=forced) - return find_base_dir() + # note: find_base_dir also changes into the base dir! + base_dir = find_base_dir() + + # if changing into obtained directory is not desired, + # 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) + else: + change_dir(cwd) + + return base_dir def which(cmd, retain_all=False, check_perms=True, log_ok=True, log_error=True): @@ -1203,7 +1222,8 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git_am=Fa workdir = tempfile.mkdtemp(prefix='eb-patch-') _log.debug("Extracting the patch to: %s", workdir) # extracting the patch - apatch_dir = extract_file(apatch, workdir) + apatch_dir = extract_file(apatch, workdir, change_into_dir=False) + change_dir(apatch_dir) apatch = os.path.join(apatch_dir, apatch_name) if level is None and build_option('extended_dry_run'): diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 9eb9219dd8..28173c13e9 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -50,7 +50,7 @@ from easybuild.framework.easyconfig.parser import EasyConfigParser from easybuild.tools.build_log import EasyBuildError, print_msg, print_warning from easybuild.tools.config import build_option -from easybuild.tools.filetools import apply_patch, copy_dir, copy_easyblocks, copy_framework_files +from easybuild.tools.filetools import apply_patch, change_dir, copy_dir, copy_easyblocks, copy_framework_files from easybuild.tools.filetools import det_patched_files, download_file, extract_file from easybuild.tools.filetools import get_easyblock_class_name, mkdir, read_file, symlink, which, write_file from easybuild.tools.py2vs3 import HTTPError, URLError, ascii_letters, urlopen @@ -360,7 +360,9 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_ download_file(base_name, url, target_path, forced=True) _log.debug("%s downloaded to %s, extracting now" % (base_name, path)) - extracted_path = os.path.join(extract_file(target_path, path, forced=True), extracted_dir_name) + base_dir = extract_file(target_path, path, forced=True, change_into_dir=False) + change_dir(base_dir) + extracted_path = os.path.join(base_dir, extracted_dir_name) # check if extracted_path exists if not os.path.isdir(extracted_path): diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 1e015f9b85..8139e326ae 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -46,7 +46,7 @@ from easybuild.tools import config from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import get_module_syntax -from easybuild.tools.filetools import copy_dir, copy_file, mkdir, read_file, remove_file, write_file +from easybuild.tools.filetools import change_dir, copy_dir, copy_file, mkdir, read_file, remove_file, write_file from easybuild.tools.module_generator import module_generator from easybuild.tools.modules import reset_module_caches from easybuild.tools.utilities import time2str @@ -1568,8 +1568,13 @@ def test_guess_start_dir(self): test_easyconfigs = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs', 'test_ecs') ec = process_easyconfig(os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0.eb'))[0] + cwd = os.getcwd() + self.assertTrue(os.path.exists(cwd)) + def check_start_dir(expected_start_dir): """Check start dir.""" + # make sure we're in an existing directory at the start + change_dir(cwd) eb = EasyBlock(ec['ec']) eb.silent = True eb.cfg['stop'] = 'patch' diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 166f13c8c2..cf5d7df25c 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1360,7 +1360,8 @@ def test_apply_patch(self): """ Test apply_patch """ testdir = os.path.dirname(os.path.abspath(__file__)) tmpdir = self.test_prefix - path = ft.extract_file(os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz'), tmpdir) + toy_tar_gz = os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz') + path = ft.extract_file(toy_tar_gz, tmpdir, change_into_dir=False) toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch' toy_patch = os.path.join(testdir, 'sandbox', 'sources', 'toy', toy_patch_fn) @@ -1653,19 +1654,24 @@ def test_change_dir(self): def test_extract_file(self): """Test extract_file""" + cwd = os.getcwd() + testdir = os.path.dirname(os.path.abspath(__file__)) toy_tarball = os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz') self.assertFalse(os.path.exists(os.path.join(self.test_prefix, 'toy-0.0', 'toy.source'))) - path = ft.extract_file(toy_tarball, self.test_prefix) + path = ft.extract_file(toy_tarball, self.test_prefix, change_into_dir=False) self.assertTrue(os.path.exists(os.path.join(self.test_prefix, 'toy-0.0', 'toy.source'))) self.assertTrue(os.path.samefile(path, self.test_prefix)) + # still in same directory as before if change_into_dir is set to False + self.assertTrue(os.path.samefile(os.getcwd(), cwd)) shutil.rmtree(os.path.join(path, 'toy-0.0')) toy_tarball_renamed = os.path.join(self.test_prefix, 'toy_tarball') shutil.copyfile(toy_tarball, toy_tarball_renamed) - path = ft.extract_file(toy_tarball_renamed, self.test_prefix, cmd="tar xfvz %s") + path = ft.extract_file(toy_tarball_renamed, self.test_prefix, cmd="tar xfvz %s", change_into_dir=False) + self.assertTrue(os.path.samefile(os.getcwd(), cwd)) self.assertTrue(os.path.exists(os.path.join(self.test_prefix, 'toy-0.0', 'toy.source'))) self.assertTrue(os.path.samefile(path, self.test_prefix)) shutil.rmtree(os.path.join(path, 'toy-0.0')) @@ -1678,17 +1684,56 @@ def test_extract_file(self): init_config(build_options=build_options) self.mock_stdout(True) - path = ft.extract_file(toy_tarball, self.test_prefix) + path = ft.extract_file(toy_tarball, self.test_prefix, change_into_dir=False) txt = self.get_stdout() self.mock_stdout(False) + self.assertTrue(os.path.samefile(os.getcwd(), cwd)) self.assertTrue(os.path.samefile(path, self.test_prefix)) self.assertFalse(os.path.exists(os.path.join(self.test_prefix, 'toy-0.0'))) self.assertTrue(re.search('running command "tar xzf .*/toy-0.0.tar.gz"', txt)) - path = ft.extract_file(toy_tarball, self.test_prefix, forced=True) + path = ft.extract_file(toy_tarball, self.test_prefix, forced=True, change_into_dir=False) self.assertTrue(os.path.exists(os.path.join(self.test_prefix, 'toy-0.0', 'toy.source'))) self.assertTrue(os.path.samefile(path, self.test_prefix)) + self.assertTrue(os.path.samefile(os.getcwd(), cwd)) + + build_options['extended_dry_run'] = False + init_config(build_options=build_options) + + ft.remove_dir(os.path.join(self.test_prefix, 'toy-0.0')) + + # a deprecation warning is printed (which is an error in this context) + # if the 'change_into_dir' named argument was left unspecified + error_pattern = "extract_file function was called without specifying value for change_into_dir" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.extract_file, toy_tarball, self.test_prefix) + self.allow_deprecated_behaviour() + + # make sure we're not in self.test_prefix now (checks below assumes so) + self.assertFalse(os.path.samefile(os.getcwd(), self.test_prefix)) + + # by default, extract_file changes to directory in which source file was unpacked + self.mock_stderr(True) + path = ft.extract_file(toy_tarball, self.test_prefix) + stderr = self.get_stderr().strip() + self.mock_stderr(False) + self.assertTrue(os.path.samefile(path, self.test_prefix)) + self.assertTrue(os.path.samefile(os.getcwd(), self.test_prefix)) + regex = re.compile("^WARNING: .*extract_file function was called without specifying value for change_into_dir") + self.assertTrue(regex.search(stderr), "Pattern '%s' found in: %s" % (regex.pattern, stderr)) + + ft.change_dir(cwd) + self.assertFalse(os.path.samefile(os.getcwd(), self.test_prefix)) + + # no deprecation warning when change_into_dir is set to True + self.mock_stderr(True) + path = ft.extract_file(toy_tarball, self.test_prefix, change_into_dir=True) + stderr = self.get_stderr().strip() + self.mock_stderr(False) + + self.assertTrue(os.path.samefile(path, self.test_prefix)) + self.assertTrue(os.path.samefile(os.getcwd(), self.test_prefix)) + self.assertFalse(stderr) def test_remove(self): """Test remove_file, remove_dir and join remove functions."""