diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 1aba3187d2..9ecc492326 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -2409,37 +2409,71 @@ def _sanity_check_step_common(self, custom_paths, custom_commands): SANITY_CHECK_PATHS_DIRS: ("(non-empty) directory", lambda dp: os.path.isdir(dp) and os.listdir(dp)), } - # prepare sanity check paths - paths = self.cfg['sanity_check_paths'] - if not paths: + enhance_sanity_check = self.cfg['enhance_sanity_check'] + ec_commands = self.cfg['sanity_check_commands'] + ec_paths = self.cfg['sanity_check_paths'] + + # if enhance_sanity_check is not enabled, only sanity_check_paths specified in the easyconfig file are used, + # the ones provided by the easyblock (via custom_paths) are ignored + if ec_paths and not enhance_sanity_check: + paths = ec_paths + self.log.info("Using (only) sanity check paths specified by easyconfig file: %s", paths) + else: + # if no sanity_check_paths are specified in easyconfig, + # we fall back to the ones provided by the easyblock via custom_paths if custom_paths: paths = custom_paths - self.log.info("Using customized sanity check paths: %s" % paths) + self.log.info("Using customized sanity check paths: %s", paths) + # if custom_paths is empty, we fall back to a generic set of paths: + # non-empty bin/ + /lib or /lib64 directories else: paths = {} for key in path_keys_and_check: paths.setdefault(key, []) paths.update({SANITY_CHECK_PATHS_DIRS: ['bin', ('lib', 'lib64')]}) - self.log.info("Using default sanity check paths: %s" % paths) + self.log.info("Using default sanity check paths: %s", paths) + + # if enhance_sanity_check is enabled *and* sanity_check_paths are specified in the easyconfig, + # those paths are used to enhance the paths provided by the easyblock + if enhance_sanity_check and ec_paths: + for key in ec_paths: + val = ec_paths[key] + if isinstance(val, list): + paths[key] = paths.get(key, []) + val + else: + error_pattern = "Incorrect value type in sanity_check_paths, should be a list: " + error_pattern += "%s (type: %s)" % (val, type(val)) + raise EasyBuildError(error_pattern) + self.log.info("Enhanced sanity check paths after taking into account easyconfig file: %s", paths) + + sorted_keys = sorted(paths.keys()) + known_keys = sorted(path_keys_and_check.keys()) + + # verify sanity_check_paths value: only known keys, correct value types, at least one non-empty value + only_list_values = all(isinstance(x, list) for x in paths.values()) + only_empty_lists = all(not x for x in paths.values()) + if sorted_keys != known_keys or not only_list_values or only_empty_lists: + error_msg = "Incorrect format for sanity_check_paths: should (only) have %s keys, " + error_msg += "values should be lists (at least one non-empty)." + raise EasyBuildError(error_msg % ', '.join("'%s'" % k for k in known_keys)) + + # if enhance_sanity_check is not enabled, only sanity_check_commands specified in the easyconfig file are used, + # the ones provided by the easyblock (via custom_commands) are ignored + if ec_commands and not enhance_sanity_check: + commands = ec_commands + self.log.info("Using (only) sanity check commands specified by easyconfig file: %s", commands) else: - self.log.info("Using specified sanity check paths: %s" % paths) - - ks = sorted(paths.keys()) - valnottypes = [not isinstance(x, list) for x in paths.values()] - lenvals = [len(x) for x in paths.values()] - req_keys = sorted(path_keys_and_check.keys()) - if not ks == req_keys or sum(valnottypes) > 0 or sum(lenvals) == 0: - raise EasyBuildError("Incorrect format for sanity_check_paths (should (only) have %s keys, " - "values should be lists (at least one non-empty)).", ','.join(req_keys)) - - commands = self.cfg['sanity_check_commands'] - if not commands: if custom_commands: commands = custom_commands - self.log.info("Using customised sanity check commands: %s" % commands) + self.log.info("Using customised sanity check commands: %s", commands) else: commands = [] - self.log.info("Using specified sanity check commands: %s" % commands) + + # if enhance_sanity_check is enabled, the sanity_check_commands specified in the easyconfig file + # are combined with those provided by the easyblock via custom_commands + if enhance_sanity_check and ec_commands: + commands = commands + ec_commands + self.log.info("Enhanced sanity check commands after taking into account easyconfig file: %s", commands) for i, command in enumerate(commands): # set command to default. This allows for config files with @@ -2475,9 +2509,17 @@ def _sanity_check_step_dry_run(self, custom_paths=None, custom_commands=None, ** """ paths, path_keys_and_check, commands = self._sanity_check_step_common(custom_paths, custom_commands) - for key, (typ, _) in path_keys_and_check.items(): + for key in [SANITY_CHECK_PATHS_FILES, SANITY_CHECK_PATHS_DIRS]: + (typ, _) = path_keys_and_check[key] self.dry_run_msg("Sanity check paths - %s ['%s']", typ, key) - if paths[key]: + entries = paths[key] + if entries: + # some entries may be tuple values, + # we need to convert them to strings first so we can print them sorted + for idx, entry in enumerate(entries): + if isinstance(entry, tuple): + entries[idx] = ' or '.join(entry) + for path in sorted(paths[key]): self.dry_run_msg(" * %s", str(path)) else: diff --git a/easybuild/framework/easyconfig/default.py b/easybuild/framework/easyconfig/default.py index 920e27d1bd..1fe7c705b6 100644 --- a/easybuild/framework/easyconfig/default.py +++ b/easybuild/framework/easyconfig/default.py @@ -90,6 +90,8 @@ 'easyblock': [None, "EasyBlock to use for building; if set to None, an easyblock is selected " "based on the software name", BUILD], 'easybuild_version': [None, "EasyBuild-version this spec-file was written for", BUILD], + 'enhance_sanity_check': [False, "Indicate that additional sanity check commands & paths should enhance " + "the existin sanity check, not replace it", BUILD], 'fix_perl_shebang_for': [None, "List of files for which Perl shebang should be fixed " "to '#!/usr/bin/env perl' (glob patterns supported)", BUILD], 'fix_python_shebang_for': [None, "List of files for which Python shebang should be fixed " diff --git a/easybuild/framework/easyconfig/format/one.py b/easybuild/framework/easyconfig/format/one.py index 88d043bc61..350afdbb6c 100644 --- a/easybuild/framework/easyconfig/format/one.py +++ b/easybuild/framework/easyconfig/format/one.py @@ -97,6 +97,13 @@ class FormatOneZero(EasyConfigFormatConfigObj): PYHEADER_MANDATORY = ['version', 'name', 'toolchain', 'homepage', 'description'] PYHEADER_BLACKLIST = [] + def __init__(self, *args, **kwargs): + """FormatOneZero constructor.""" + super(FormatOneZero, self).__init__(*args, **kwargs) + + self.log = fancylogger.getLogger(self.__class__.__name__, fname=False) + self.strict_sanity_check_paths_keys = True + def validate(self): """Format validation""" # minimal checks @@ -168,11 +175,14 @@ def _reformat_line(self, param_name, param_val, outer=False, addlen=0): for item_key in ordered_item_keys: if item_key in param_val: item_val = param_val[item_key] + item_comments = self._get_item_comments(param_name, item_val) + elif param_name == 'sanity_check_paths' and not self.strict_sanity_check_paths_keys: + item_val = [] + item_comments = {} + self.log.info("Using default value for '%s' in sanity_check_paths: %s", item_key, item_val) else: raise EasyBuildError("Missing mandatory key '%s' in %s.", item_key, param_name) - item_comments = self._get_item_comments(param_name, item_val) - inline_comment = item_comments.get('inline', '') item_tmpl_dict = {'inline_comment': inline_comment} @@ -317,6 +327,10 @@ def dump(self, ecfg, default_values, templ_const, templ_val, toolchain_hierarchy :param templ_val: known template values :param toolchain_hierarchy: hierarchy of toolchains for easyconfig """ + # figoure out whether we should be strict about the format of sanity_check_paths; + # if enhance_sanity_check is set, then both files/dirs keys are not strictly required... + self.strict_sanity_check_paths_keys = not ecfg['enhance_sanity_check'] + # include header comments first dump = self.comments['header'][:] diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 25e9789d14..1e015f9b85 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -53,6 +53,7 @@ from easybuild.tools.version import get_git_revision, this_is_easybuild from easybuild.tools.py2vs3 import string_type + class EasyBlockTest(EnhancedTestCase): """ Baseclass for easyblock testcases """ @@ -1928,6 +1929,73 @@ def test_time2str(self): error_pattern = "Incorrect value type provided to time2str, should be datetime.timedelta: <.* 'int'>" self.assertErrorRegex(EasyBuildError, error_pattern, time2str, 123) + def test_sanity_check_paths_verification(self): + """Test verification of sanity_check_paths w.r.t. keys & values.""" + + testdir = os.path.abspath(os.path.dirname(__file__)) + toy_ec = os.path.join(testdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') + eb = EasyBlock(EasyConfig(toy_ec)) + eb.dry_run = True + + error_pattern = r"Incorrect format for sanity_check_paths: " + error_pattern += r"should \(only\) have 'dirs', 'files' keys, " + error_pattern += r"values should be lists \(at least one non-empty\)." + + def run_sanity_check_step(sanity_check_paths, enhance_sanity_check): + """Helper function to run sanity check step, and do trivial check on generated output.""" + self.mock_stderr(True) + self.mock_stdout(True) + eb.cfg['sanity_check_paths'] = sanity_check_paths + eb.cfg['enhance_sanity_check'] = enhance_sanity_check + eb.sanity_check_step() + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) + self.assertFalse(stderr) + self.assertTrue(stdout.startswith("Sanity check paths")) + + # partial sanity_check_paths, only allowed when using enhance_sanity_check + test_cases = [ + {'dirs': ['foo']}, + {'files': ['bar']}, + {'dirs': []}, + {'files': []}, + {'files': [], 'dirs': []}, + ] + for test_case in test_cases: + # without enhanced sanity check, these are all invalid sanity_check_paths values + self.assertErrorRegex(EasyBuildError, error_pattern, run_sanity_check_step, test_case, False) + + # if enhance_sanity_check is enabled, these are acceptable sanity_check_step values + run_sanity_check_step(test_case, True) + + # some inputs are always invalid, regardless of enhance_sanity_check, due to wrong keys/values + test_cases = [ + {'foo': ['bar']}, + {'files': ['foo'], 'dirs': [], 'libs': ['libfoo.a']}, + {'files': ['foo'], 'libs': ['libfoo.a']}, + {'dirs': [], 'libs': ['libfoo.a']}, + ] + for test_case in test_cases: + self.assertErrorRegex(EasyBuildError, error_pattern, run_sanity_check_step, test_case, False) + self.assertErrorRegex(EasyBuildError, error_pattern, run_sanity_check_step, test_case, True) + + # non-list values yield different errors with/without enhance_sanity_check + error_pattern_bis = r"Incorrect value type in sanity_check_paths, should be a list: .*" + test_cases = [ + {'files': 123, 'dirs': []}, + {'files': [], 'dirs': 123}, + {'files': 'foo', 'dirs': []}, + {'files': [], 'dirs': 'foo'}, + ] + for test_case in test_cases: + self.assertErrorRegex(EasyBuildError, error_pattern, run_sanity_check_step, test_case, False) + self.assertErrorRegex(EasyBuildError, error_pattern_bis, run_sanity_check_step, test_case, True) + + # empty sanity_check_paths is always OK, since then the fallback to default bin + lib/lib64 kicks in + run_sanity_check_step({}, False) + run_sanity_check_step({}, True) + def suite(): """ return all the tests in this file """ diff --git a/test/framework/sandbox/easybuild/easyblocks/t/toy.py b/test/framework/sandbox/easybuild/easyblocks/t/toy.py index 79f3d3d8fc..11b2dff542 100644 --- a/test/framework/sandbox/easybuild/easyblocks/t/toy.py +++ b/test/framework/sandbox/easybuild/easyblocks/t/toy.py @@ -46,7 +46,7 @@ class EB_toy(ExtensionEasyBlock): @staticmethod def extra_options(extra_vars=None): - """Custom easyconfig parameters for toytoy.""" + """Custom easyconfig parameters for toy.""" if extra_vars is None: extra_vars = {} diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 1570504205..04a916a339 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -53,7 +53,7 @@ from easybuild.tools.filetools import adjust_permissions, mkdir, read_file, remove_dir, remove_file, which, write_file from easybuild.tools.module_generator import ModuleGeneratorTcl from easybuild.tools.modules import Lmod -from easybuild.tools.py2vs3 import string_type +from easybuild.tools.py2vs3 import reload, string_type from easybuild.tools.run import run_cmd from easybuild.tools.version import VERSION as EASYBUILD_VERSION @@ -73,7 +73,31 @@ def setUp(self): def tearDown(self): """Cleanup.""" + + # kick out any paths for included easyblocks from sys.path, + # to avoid infected any other tests + for path in sys.path[:]: + if '/included-easyblocks' in path: + sys.path.remove(path) + + # reload toy easyblock (and generic toy_extension easyblock that imports it) after cleaning up sys.path, + # to avoid trouble in other tests due to included toy easyblock that is cached somewhere + # (despite the cleanup in sys.modules); + # important for tests that include a customised copy of the toy easyblock + # (like test_toy_build_enhanced_sanity_check) + import easybuild.easyblocks.toy + reload(easybuild.easyblocks.toy) + import easybuild.easyblocks.toytoy + reload(easybuild.easyblocks.toytoy) + import easybuild.easyblocks.generic.toy_extension + reload(easybuild.easyblocks.generic.toy_extension) + + del sys.modules['easybuild.easyblocks.toy'] + del sys.modules['easybuild.easyblocks.toytoy'] + del sys.modules['easybuild.easyblocks.generic.toy_extension'] + super(ToyBuildTest, self).tearDown() + # remove logs if os.path.exists(self.dummylogfn): os.remove(self.dummylogfn) @@ -1887,6 +1911,169 @@ def test_sanity_check_paths_lib64(self): write_file(test_ec, ectxt) self.test_toy_build(ec_file=test_ec, raise_error=True) + def test_toy_build_enhanced_sanity_check(self): + """Test enhancing of sanity check.""" + + # if toy easyblock was imported, get rid of corresponding entry in sys.modules, + # to avoid that it messes up the use of --include-easyblocks=toy.py below... + if 'easybuild.easyblocks.toy' in sys.modules: + del sys.modules['easybuild.easyblocks.toy'] + + test_dir = os.path.join(os.path.abspath(os.path.dirname(__file__))) + toy_ec = os.path.join(test_dir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb') + toy_ec_txt = read_file(toy_ec) + + test_ec = os.path.join(self.test_prefix, 'test.eb') + + # get rid of custom sanity check paths in test easyconfig + regex = re.compile(r'^sanity_check_paths\s*=\s*{[^}]+}', re.M) + test_ec_txt = regex.sub('', toy_ec_txt) + write_file(test_ec, test_ec_txt) + + self.assertFalse('sanity_check_' in test_ec_txt) + + # create custom easyblock for toy that has a custom sanity_check_step + toy_easyblock = os.path.join(test_dir, 'sandbox', 'easybuild', 'easyblocks', 't', 'toy.py') + + toy_easyblock_txt = read_file(toy_easyblock) + + toy_custom_sanity_check_step = '\n'.join([ + '', + " def sanity_check_step(self):", + " paths = {", + " 'files': ['bin/toy'],", + " 'dirs': [],", + " }", + " cmds = ['toy']", + " return super(EB_toy, self).sanity_check_step(custom_paths=paths, custom_commands=cmds)", + ]) + test_toy_easyblock = os.path.join(self.test_prefix, 'toy.py') + write_file(test_toy_easyblock, toy_easyblock_txt + toy_custom_sanity_check_step) + + eb_args = [ + '--extended-dry-run', + '--include-easyblocks=%s' % test_toy_easyblock, + ] + + # by default, sanity check commands & paths specified by easyblock are used + self.mock_stdout(True) + self.test_toy_build(ec_file=test_ec, extra_args=eb_args, verify=False, testing=False, raise_error=True) + stdout = self.get_stdout() + self.mock_stdout(False) + + pattern_lines = [ + r"Sanity check paths - file.*", + r"\s*\* bin/toy", + r"Sanity check paths - \(non-empty\) directory.*", + r"\s*\(none\)", + r"Sanity check commands", + r"\s*\* toy", + r'', + ] + regex = re.compile(r'\n'.join(pattern_lines), re.M) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + + # we need to manually wipe the entry for the included toy easyblock, + # to avoid trouble with subsequent EasyBuild sessions in this test + del sys.modules['easybuild.easyblocks.toy'] + + # easyconfig specifies custom sanity_check_paths & sanity_check_commands, + # the ones defined by the easyblock are skipped by default + test_ec_txt = test_ec_txt + '\n'.join([ + '', + "sanity_check_paths = {", + " 'files': ['README'],", + " 'dirs': ['bin/']", + "}", + "sanity_check_commands = ['ls %(installdir)s']", + ]) + write_file(test_ec, test_ec_txt) + + self.mock_stdout(True) + self.test_toy_build(ec_file=test_ec, extra_args=eb_args, verify=False, testing=False, raise_error=True) + stdout = self.get_stdout() + self.mock_stdout(False) + + pattern_lines = [ + r"Sanity check paths - file.*", + r"\s*\* README", + r"Sanity check paths - \(non-empty\) directory.*", + r"\s*\* bin/", + r"Sanity check commands", + r"\s*\* ls .*/software/toy/0.0", + r'', + ] + regex = re.compile(r'\n'.join(pattern_lines), re.M) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + + del sys.modules['easybuild.easyblocks.toy'] + + # if enhance_sanity_check is enabled, then sanity check paths/commands specified in easyconfigs + # are used in addition to those defined in easyblock + test_ec_txt = test_ec_txt + '\nenhance_sanity_check = True' + write_file(test_ec, test_ec_txt) + + self.mock_stdout(True) + self.test_toy_build(ec_file=test_ec, extra_args=eb_args, verify=False, testing=False, raise_error=True) + stdout = self.get_stdout() + self.mock_stdout(False) + + # now 'bin/toy' file and 'toy' command should also be part of sanity check + pattern_lines = [ + r"Sanity check paths - file.*", + r"\s*\* README", + r"\s*\* bin/toy", + r"Sanity check paths - \(non-empty\) directory.*", + r"\s*\* bin/", + r"Sanity check commands", + r"\s*\* ls .*/software/toy/0.0", + r"\s*\* toy", + r'', + ] + regex = re.compile(r'\n'.join(pattern_lines), re.M) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + + del sys.modules['easybuild.easyblocks.toy'] + + # sanity_check_paths with only one key is allowed if enhance_sanity_check is enabled; + test_ec_txt = test_ec_txt + "\nsanity_check_paths = {'files': ['README']}" + write_file(test_ec, test_ec_txt) + + # we need to do a non-dry run here, to ensure the code we want to test is triggered + # (EasyConfig.dump called by 'reproduce_build' function from 'build_and_install_one') + eb_args = [ + '--include-easyblocks=%s' % test_toy_easyblock, + '--trace', + ] + + self.mock_stdout(True) + self.test_toy_build(ec_file=test_ec, extra_args=eb_args, verify=False, testing=False, raise_error=True) + stdout = self.get_stdout() + self.mock_stdout(False) + + pattern_lines = [ + r"^== sanity checking\.\.\.", + r" >> file 'bin/toy' found: OK", + ] + regex = re.compile(r'\n'.join(pattern_lines), re.M) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + + # no directories are checked in sanity check now, only files (since dirs is an empty list) + regex = re.compile(r"directory .* found:", re.M) + self.assertFalse(regex.search(stdout), "Pattern '%s' should be not found in: %s" % (regex.pattern, stdout)) + + del sys.modules['easybuild.easyblocks.toy'] + + # if enhance_sanity_check is disabled, both files/dirs keys are strictly required in sanity_check_paths + test_ec_txt = test_ec_txt + '\nenhance_sanity_check = False' + write_file(test_ec, test_ec_txt) + + error_pattern = " Missing mandatory key 'dirs' in sanity_check_paths." + self.assertErrorRegex(EasyBuildError, error_pattern, self.test_toy_build, ec_file=test_ec, + extra_args=eb_args, raise_error=True, verbose=False) + + del sys.modules['easybuild.easyblocks.toy'] + def test_toy_dumped_easyconfig(self): """ Test dumping of file in eb_filerepo in both .eb and .yeb format """ filename = 'toy-0.0'