From 971ac5db8861649c43210bc8a8bf29736b8a844a Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 30 Apr 2020 16:38:46 +0800 Subject: [PATCH 1/7] expand glob paths when checking for multiple inclusion of easyblocks --- easybuild/tools/options.py | 6 ++++-- test/framework/options.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 2a09600f78..e0905b0a1c 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -75,7 +75,8 @@ from easybuild.tools.docs import avail_toolchain_opts, avail_easyconfig_params, avail_easyconfig_templates from easybuild.tools.docs import list_easyblocks, list_toolchains from easybuild.tools.environment import restore_env, unset_env_vars -from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, install_fake_vsc, move_file, which +from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, expand_glob_paths, install_fake_vsc +from easybuild.tools.filetools import move_file, which from easybuild.tools.github import GITHUB_EB_MAIN, GITHUB_EASYCONFIGS_REPO from easybuild.tools.github import GITHUB_PR_DIRECTION_DESC, GITHUB_PR_ORDER_CREATED, GITHUB_PR_STATE_OPEN from easybuild.tools.github import GITHUB_PR_STATES, GITHUB_PR_ORDERS, GITHUB_PR_DIRECTIONS @@ -1432,7 +1433,8 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): if eb_go.options.include_easyblocks: # make sure we're not including the same easyblock twice included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr]) - included_from_file = set([os.path.basename(eb) for eb in eb_go.options.include_easyblocks]) + included_paths = expand_glob_paths(eb_go.options.include_easyblocks) + included_from_file = set([os.path.basename(eb) for eb in included_paths]) included_twice = included_from_pr & included_from_file if included_twice: raise EasyBuildError("Multiple inclusion of %s, check your --include-easyblocks options", diff --git a/test/framework/options.py b/test/framework/options.py index 354cdfad16..2968b7d27c 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2734,7 +2734,7 @@ def test_xxx_include_easyblocks_from_pr(self): # including the same easyblock twice should fail args = [ - '--include-easyblocks=%s/cmakemake.py' % self.test_prefix, + '--include-easyblocks=%s/*.py' % self.test_prefix, '--include-easyblocks-from-pr=1915', '--list-easyblocks=detailed', '--unittest-file=%s' % self.logfile, From 7a7b8a116b2b83711e9e1198fe6fb859f9bd5cc9 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 27 Aug 2020 22:41:32 +0800 Subject: [PATCH 2/7] restore sys.modules at the end of verify_imports --- easybuild/tools/include.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/easybuild/tools/include.py b/easybuild/tools/include.py index 67ded09f53..5214607f38 100644 --- a/easybuild/tools/include.py +++ b/easybuild/tools/include.py @@ -125,6 +125,8 @@ def set_up_eb_package(parent_path, eb_pkg_name, subpkgs=None, pkg_init_body=None def verify_imports(pymods, pypkg, from_path): """Verify that import of specified modules from specified package and expected location works.""" + saved_modules = sys.modules.copy() + for pymod in pymods: pymod_spec = '%s.%s' % (pypkg, pymod) try: @@ -140,6 +142,10 @@ def verify_imports(pymods, pypkg, from_path): _log.debug("Import of %s from %s verified", pymod_spec, from_path) + # restore sys.modules to its original state (not only verified modules but also their dependencies) + sys.modules.clear() + sys.modules.update(saved_modules) + def is_software_specific_easyblock(module): """Determine whether Python module at specified location is a software-specific easyblock.""" From 60f71a350bf92b8d40f394702fde97772a2a6883 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 27 Aug 2020 22:43:59 +0800 Subject: [PATCH 3/7] allow including the same easyblock from a file and from a PR, warn that the one from the PR will be used --- easybuild/tools/options.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 10d9ea42e6..58b66464f7 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1454,16 +1454,19 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): # done here instead of in _postprocess_include because github integration requires build_options to be initialized if eb_go.options.include_easyblocks_from_pr: easyblocks_from_pr = fetch_easyblocks_from_pr(eb_go.options.include_easyblocks_from_pr) + included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr]) if eb_go.options.include_easyblocks: - # make sure we're not including the same easyblock twice - included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr]) + # check if you are including the same easyblock twice included_paths = expand_glob_paths(eb_go.options.include_easyblocks) included_from_file = set([os.path.basename(eb) for eb in included_paths]) included_twice = included_from_pr & included_from_file if included_twice: - raise EasyBuildError("Multiple inclusion of %s, check your --include-easyblocks options", - ','.join(included_twice)) + log.info("EasyBlock(s) %s are included from multiple locations, the one from a PR will be used", + ','.join(included_twice)) + + for easyblock in included_from_pr: + log.info("== easyblock %s included from PR #%s", easyblock, eb_go.options.include_easyblocks_from_pr) include_easyblocks(eb_go.options.tmpdir, easyblocks_from_pr) From 150d91196e4d3608f1cf2817c8b7fdfd3cad8d6f Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 27 Aug 2020 22:44:41 +0800 Subject: [PATCH 4/7] update include_easyblocks tests --- test/framework/include.py | 19 +++++++++++++++---- test/framework/options.py | 25 +++++++++++++++++++++---- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/test/framework/include.py b/test/framework/include.py index 16dd73986e..e3b8866968 100644 --- a/test/framework/include.py +++ b/test/framework/include.py @@ -102,19 +102,22 @@ def test_include_easyblocks(self): myfoo_pyc_path = easybuild.easyblocks.myfoo.__file__ myfoo_real_py_path = os.path.realpath(os.path.join(os.path.dirname(myfoo_pyc_path), 'myfoo.py')) self.assertTrue(os.path.samefile(up(myfoo_real_py_path, 1), myeasyblocks)) + del sys.modules['easybuild.easyblocks.myfoo'] import easybuild.easyblocks.generic.mybar mybar_pyc_path = easybuild.easyblocks.generic.mybar.__file__ mybar_real_py_path = os.path.realpath(os.path.join(os.path.dirname(mybar_pyc_path), 'mybar.py')) self.assertTrue(os.path.samefile(up(mybar_real_py_path, 2), myeasyblocks)) + del sys.modules['easybuild.easyblocks.generic.mybar'] # existing (test) easyblocks are unaffected import easybuild.easyblocks.foofoo foofoo_path = os.path.dirname(os.path.dirname(easybuild.easyblocks.foofoo.__file__)) self.assertTrue(os.path.samefile(foofoo_path, test_easyblocks)) + del sys.modules['easybuild.easyblocks.foofoo'] def test_include_easyblocks_priority(self): - """Test whether easyblocks included via include_easyblocks() get prioroity over others.""" + """Test whether easyblocks included via include_easyblocks() get priority over others.""" test_easyblocks = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'sandbox', 'easybuild', 'easyblocks') # make sure that test 'foo' easyblock is there @@ -138,15 +141,23 @@ def test_include_easyblocks_priority(self): " pass", ]) write_file(os.path.join(myeasyblocks, 'foo.py'), foo_easyblock_txt) + + # check that the sandboxed easyblock is imported before include_easyblocks is run + foo_pyc_path = easybuild.easyblocks.foo.__file__ + foo_real_py_path = os.path.realpath(os.path.join(os.path.dirname(foo_pyc_path), 'foo.py')) + self.assertTrue(os.path.samefile(os.path.dirname(os.path.dirname(foo_pyc_path)), test_easyblocks)) + self.assertFalse(os.path.samefile(foo_real_py_path, os.path.join(myeasyblocks, 'foo.py'))) + include_easyblocks(self.test_prefix, [os.path.join(myeasyblocks, 'foo.py')]) + # check that the included easyblock is imported after include_easyblocks is run foo_pyc_path = easybuild.easyblocks.foo.__file__ foo_real_py_path = os.path.realpath(os.path.join(os.path.dirname(foo_pyc_path), 'foo.py')) - self.assertFalse(os.path.samefile(os.path.dirname(foo_pyc_path), test_easyblocks)) + self.assertFalse(os.path.samefile(os.path.dirname(os.path.dirname(foo_pyc_path)), test_easyblocks)) self.assertTrue(os.path.samefile(foo_real_py_path, os.path.join(myeasyblocks, 'foo.py'))) - # 'undo' import of foo easyblock - del sys.modules['easybuild.easyblocks.foo'] + # check that the included easyblock is not loaded + self.assertFalse('easybuild.easyblocks.foo' in sys.modules) def test_include_mns(self): """Test include_module_naming_schemes().""" diff --git a/test/framework/options.py b/test/framework/options.py index 0848bad278..2d7863e607 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2917,7 +2917,7 @@ def test_xxx_include_easyblocks_from_pr(self): ]) write_file(os.path.join(self.test_prefix, 'cmakemake.py'), cmm_txt) - # including the same easyblock twice should fail + # including the same easyblock twice should work and give priority to the one from the PR args = [ '--include-easyblocks=%s/*.py' % self.test_prefix, '--include-easyblocks-from-pr=1915', @@ -2925,11 +2925,28 @@ def test_xxx_include_easyblocks_from_pr(self): '--unittest-file=%s' % self.logfile, '--github-user=%s' % GITHUB_TEST_ACCOUNT, ] - self.assertErrorRegex(EasyBuildError, - "Multiple inclusion of cmakemake.py, check your --include-easyblocks options", - self.eb_main, args, raise_error=True) + self.eb_main(args, logfile=dummylogfn, raise_error=True) + logtxt = read_file(self.logfile) + + # easyblock included from pr is found + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks') + cmm_pattern = os.path.join(path_pattern, 'generic', 'cmakemake.py') + cmm_regex = re.compile(r"\|-- CMakeMake \(easybuild.easyblocks.generic.cmakemake @ %s\)" % cmm_pattern, re.M) + self.assertTrue(cmm_regex.search(logtxt), "Pattern '%s' found in: %s" % (cmm_regex.pattern, logtxt)) + + # easyblock is found via get_easyblock_class + klass = get_easyblock_class('CMakeMake') + self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) + # 'undo' import of easyblocks + del sys.modules['easybuild.easyblocks.foo'] + del sys.modules['easybuild.easyblocks.generic.cmakemake'] os.remove(os.path.join(self.test_prefix, 'cmakemake.py')) + sys.path = orig_local_sys_path + import easybuild.easyblocks + reload(easybuild.easyblocks) + import easybuild.easyblocks.generic + reload(easybuild.easyblocks.generic) # clear log write_file(self.logfile, '') From 37abe78ebbd5f1d4612b4eb348d7d70784434aad Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Wed, 2 Sep 2020 11:28:22 +0800 Subject: [PATCH 5/7] make messages and warnings more explicit --- easybuild/tools/options.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 58b66464f7..7a389f9899 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -58,7 +58,7 @@ from easybuild.toolchains.compiler.systemcompiler import TC_CONSTANT_SYSTEM from easybuild.tools import build_log, run # build_log should always stay there, to ensure EasyBuildLog from easybuild.tools.build_log import DEVEL_LOG_LEVEL, EasyBuildError -from easybuild.tools.build_log import init_logging, log_start, print_warning, raise_easybuilderror +from easybuild.tools.build_log import init_logging, log_start, print_msg, print_warning, raise_easybuilderror from easybuild.tools.config import CONT_IMAGE_FORMATS, CONT_TYPES, DEFAULT_CONT_TYPE, DEFAULT_ALLOW_LOADED_MODULES from easybuild.tools.config import DEFAULT_BRANCH, DEFAULT_FORCE_DOWNLOAD, DEFAULT_INDEX_MAX_AGE from easybuild.tools.config import DEFAULT_JOB_BACKEND, DEFAULT_LOGFILE_FORMAT, DEFAULT_MAX_FAIL_RATIO_PERMS @@ -1452,8 +1452,9 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): init_build_options(build_options=build_options, cmdline_options=options) # done here instead of in _postprocess_include because github integration requires build_options to be initialized - if eb_go.options.include_easyblocks_from_pr: - easyblocks_from_pr = fetch_easyblocks_from_pr(eb_go.options.include_easyblocks_from_pr) + pr_easyblocks = eb_go.options.include_easyblocks_from_pr + if pr_easyblocks: + easyblocks_from_pr = fetch_easyblocks_from_pr(pr_easyblocks) included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr]) if eb_go.options.include_easyblocks: @@ -1462,11 +1463,11 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): included_from_file = set([os.path.basename(eb) for eb in included_paths]) included_twice = included_from_pr & included_from_file if included_twice: - log.info("EasyBlock(s) %s are included from multiple locations, the one from a PR will be used", - ','.join(included_twice)) + print_warning("EasyBlock(s) %s included from multiple locations, the one from a PR will be used" % + ','.join(included_twice)) for easyblock in included_from_pr: - log.info("== easyblock %s included from PR #%s", easyblock, eb_go.options.include_easyblocks_from_pr) + print_msg("easyblock %s included from PR #%s" % (easyblock, pr_easyblocks), log=log) include_easyblocks(eb_go.options.tmpdir, easyblocks_from_pr) From 92c97dcec2d9628b3efdf2c77f0ed4b8dc06025e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 2 Sep 2020 08:28:52 +0200 Subject: [PATCH 6/7] mention PR id in warning message when easyblocks are included from multiple locations --- easybuild/tools/options.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 7a389f9899..f56c226930 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1463,8 +1463,9 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): included_from_file = set([os.path.basename(eb) for eb in included_paths]) included_twice = included_from_pr & included_from_file if included_twice: - print_warning("EasyBlock(s) %s included from multiple locations, the one from a PR will be used" % - ','.join(included_twice)) + warning_msg = "One or more easyblocks included from multiple locations: %s " % ', '.join(included_twice) + warning_msg += "(the one(s) from PR #%s will be used)" % pr_easyblocks + print_warning(warning_msg) for easyblock in included_from_pr: print_msg("easyblock %s included from PR #%s" % (easyblock, pr_easyblocks), log=log) From 292b7ba527e5c617a0a432a9031e188204a3be08 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 2 Sep 2020 08:29:15 +0200 Subject: [PATCH 7/7] catch & check output produced by --include-easyblocks-from-pr in test --- test/framework/options.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/framework/options.py b/test/framework/options.py index 2d7863e607..285626b796 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2885,9 +2885,17 @@ def test_xxx_include_easyblocks_from_pr(self): '--unittest-file=%s' % self.logfile, '--github-user=%s' % GITHUB_TEST_ACCOUNT, ] + self.mock_stderr(True) + self.mock_stdout(True) self.eb_main(args, logfile=dummylogfn, raise_error=True) + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) logtxt = read_file(self.logfile) + self.assertFalse(stderr) + self.assertEqual(stdout, "== easyblock cmakemake.py included from PR #1915\n") + # easyblock included from pr is found path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks') cmm_pattern = os.path.join(path_pattern, 'generic', 'cmakemake.py') @@ -2925,9 +2933,19 @@ def test_xxx_include_easyblocks_from_pr(self): '--unittest-file=%s' % self.logfile, '--github-user=%s' % GITHUB_TEST_ACCOUNT, ] + self.mock_stderr(True) + self.mock_stdout(True) self.eb_main(args, logfile=dummylogfn, raise_error=True) + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) logtxt = read_file(self.logfile) + expected = "WARNING: One or more easyblocks included from multiple locations: " + expected += "cmakemake.py (the one(s) from PR #1915 will be used)" + self.assertEqual(stderr.strip(), expected) + self.assertEqual(stdout, "== easyblock cmakemake.py included from PR #1915\n") + # easyblock included from pr is found path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks') cmm_pattern = os.path.join(path_pattern, 'generic', 'cmakemake.py') @@ -2958,9 +2976,17 @@ def test_xxx_include_easyblocks_from_pr(self): '--github-user=%s' % GITHUB_TEST_ACCOUNT, '--extended-dry-run', ] + self.mock_stderr(True) + self.mock_stdout(True) self.eb_main(args, logfile=dummylogfn, raise_error=True) + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) logtxt = read_file(self.logfile) + self.assertFalse(stderr) + self.assertEqual(stdout, "== easyblock cmake.py included from PR #1936\n") + # easyconfig from pr is found ec_pattern = os.path.join(self.test_prefix, '.*', 'files_pr10487', 'c', 'CMake', 'CMake-3.16.4-GCCcore-9.3.0.eb')