From 7230321f7f09c66c4afd43fe85c6985d69dc513c Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 20 Feb 2020 12:16:54 +0800 Subject: [PATCH 01/14] add support for --include-easyblocks-from-pr --- easybuild/main.py | 9 +++++++-- easybuild/tools/github.py | 31 ++++++++++++++++++++++++------- easybuild/tools/include.py | 5 ++++- easybuild/tools/options.py | 6 ++++-- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/easybuild/main.py b/easybuild/main.py index 69c47a7293..cf9779d7cc 100644 --- a/easybuild/main.py +++ b/easybuild/main.py @@ -57,12 +57,13 @@ from easybuild.tools.containers.common import containerize from easybuild.tools.docs import list_software from easybuild.tools.filetools import adjust_permissions, cleanup, copy_file, copy_files, read_file, write_file -from easybuild.tools.github import check_github, close_pr, new_branch_github, find_easybuild_easyconfig +from easybuild.tools.github import check_github, close_pr, new_branch_github, fetch_easyblocks_from_pr +from easybuild.tools.github import find_easybuild_easyconfig from easybuild.tools.github import install_github_token, list_prs, new_pr, new_pr_from_branch, merge_pr from easybuild.tools.github import sync_branch_with_develop, sync_pr_with_develop, update_branch, update_pr from easybuild.tools.hooks import START, END, load_hooks, run_hook from easybuild.tools.modules import modules_tool -from easybuild.tools.options import set_up_configuration, use_color +from easybuild.tools.options import include_easyblocks, set_up_configuration, use_color from easybuild.tools.robot import check_conflicts, dry_run, missing_deps, resolve_dependencies, search_easyconfigs from easybuild.tools.package.utilities import check_pkg_support from easybuild.tools.parallelbuild import submit_jobs @@ -199,6 +200,10 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): eb_go, cfg_settings = set_up_configuration(args=args, logfile=logfile, testing=testing) options, orig_paths = eb_go.options, eb_go.args + if options.include_easyblocks_from_pr: + included_easyblocks = fetch_easyblocks_from_pr(options.include_easyblocks_from_pr) + include_easyblocks(options.tmpdir, included_easyblocks) + global _log (build_specs, _log, logfile, robot_path, search_query, eb_tmpdir, try_to_generate, tweaked_ecs_paths) = cfg_settings diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 44b1c6d450..2846673e91 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -85,6 +85,7 @@ GITHUB_API_URL = 'https://api.github.com' GITHUB_DIR_TYPE = u'dir' GITHUB_EB_MAIN = 'easybuilders' +GITHUB_EASYBLOCKS_REPO = 'easybuild-easyblocks' GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs' GITHUB_DEVELOP_BRANCH = 'develop' GITHUB_FILE_TYPE = u'file' @@ -369,8 +370,18 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_ return extracted_path +def fetch_easyblocks_from_pr(pr, path=None, github_user=None): + """Fetch patched easyconfig files for a particular PR.""" + return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYBLOCKS_REPO) + + def fetch_easyconfigs_from_pr(pr, path=None, github_user=None): """Fetch patched easyconfig files for a particular PR.""" + return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYCONFIGS_REPO) + + +def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): + """Fetch patched easyconfig files for a particular PR.""" if github_user is None: github_user = build_option('github_user') @@ -384,9 +395,15 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None): mkdir(path, parents=True) github_account = build_option('pr_target_account') - github_repo = GITHUB_EASYCONFIGS_REPO - _log.debug("Fetching easyconfigs from %s/%s PR #%s into %s", github_account, github_repo, pr, path) + if github_repo is None: + github_repo = GITHUB_EASYCONFIGS_REPO + elif github_repo not in [GITHUB_EASYBLOCKS_REPO, GITHUB_EASYCONFIGS_REPO]: + raise EasyBuildError("Don't know how to fetch files from repo %s", github_repo) + + easyfiles = 'easyconfigs' if github_repo == GITHUB_EASYCONFIGS_REPO else 'easyblocks' + + _log.debug("Fetching %s from %s/%s PR #%s into %s", easyfiles, github_account, github_repo, pr, path) pr_data, _ = fetch_pr_data(pr, github_account, github_repo, github_user) pr_merged = pr_data['merged'] @@ -429,7 +446,7 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None): if final_path is None: if pr_closed: - print_warning("Using easyconfigs from closed PR #%s" % pr) + print_warning("Using %s from closed PR #%s" % (easyfiles, pr)) # obtain most recent version of patched files for patched_file in patched_files: @@ -444,21 +461,21 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None): # symlink directories into expected place if they're not there yet if final_path != path: - dirpath = os.path.join(final_path, 'easybuild', 'easyconfigs') + dirpath = os.path.join(final_path, 'easybuild', easyfiles) for eb_dir in os.listdir(dirpath): symlink(os.path.join(dirpath, eb_dir), os.path.join(path, os.path.basename(eb_dir))) # sanity check: make sure all patched files are downloaded - ec_files = [] + files = [] for patched_file in [f for f in patched_files if not f.startswith('test/')]: fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:]) full_path = os.path.join(path, fn) if os.path.exists(full_path): - ec_files.append(full_path) + files.append(full_path) else: raise EasyBuildError("Couldn't find path to patched file %s", full_path) - return ec_files + return files def create_gist(txt, fn, descr=None, github_user=None, github_token=None): diff --git a/easybuild/tools/include.py b/easybuild/tools/include.py index 90b9715280..e71ed7a161 100644 --- a/easybuild/tools/include.py +++ b/easybuild/tools/include.py @@ -154,7 +154,10 @@ def include_easyblocks(tmpdir, paths): easyblocks_dir = os.path.join(easyblocks_path, 'easybuild', 'easyblocks') - allpaths = [p for p in expand_glob_paths(paths) if os.path.basename(p) != '__init__.py'] + allpaths = [p for p in expand_glob_paths(paths) + if os.path.basename(p).endswith('.py') and + os.path.basename(p) != '__init__.py'] + for easyblock_module in allpaths: filename = os.path.basename(easyblock_module) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index d149ee3d79..9ed4406db0 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -79,7 +79,7 @@ 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 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS -from easybuild.tools.github import fetch_github_token +from easybuild.tools.github import fetch_easyblocks_from_pr, fetch_github_token from easybuild.tools.hooks import KNOWN_HOOKS from easybuild.tools.include import include_easyblocks, include_module_naming_schemes, include_toolchains from easybuild.tools.job.backend import avail_job_backends @@ -592,6 +592,8 @@ def github_options(self): 'git-working-dirs-path': ("Path to Git working directories for EasyBuild repositories", str, 'store', None), 'github-user': ("GitHub username", str, 'store', None), 'github-org': ("GitHub organization", str, 'store', None), + 'include-easyblocks-from-pr': ("Include easyblocks from specified PR", int, 'store', None, + {'metavar': 'PR#'}), 'install-github-token': ("Install GitHub token (requires --github-user)", None, 'store_true', False), 'close-pr': ("Close pull request", int, 'store', None, {'metavar': 'PR#'}), 'close-pr-msg': ("Custom close message for pull request closed with --close-pr; ", str, 'store', None), @@ -922,7 +924,7 @@ def _postprocess_checks(self): """Check whether (combination of) configuration options make sense.""" # fail early if required dependencies for functionality requiring using GitHub API are not available: - if self.options.from_pr or self.options.upload_test_report: + if self.options.from_pr or self.options.include_easyblocks_from_pr or self.options.upload_test_report: if not HAVE_GITHUB_API: raise EasyBuildError("Required support for using GitHub API is not available (see warnings)") From eeb1adfd9ce6a52cd0ec746e9f5e0ab8e9053a92 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 20 Feb 2020 14:47:32 +0800 Subject: [PATCH 02/14] remove unnecessary import --- easybuild/tools/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 9ed4406db0..e8ed232161 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -79,7 +79,7 @@ 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 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS -from easybuild.tools.github import fetch_easyblocks_from_pr, fetch_github_token +from easybuild.tools.github import fetch_github_token from easybuild.tools.hooks import KNOWN_HOOKS from easybuild.tools.include import include_easyblocks, include_module_naming_schemes, include_toolchains from easybuild.tools.job.backend import avail_job_backends From 12f1e570e7d5046a8b538dc0c4a522f1cd3ac6e8 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 20 Feb 2020 15:12:26 +0800 Subject: [PATCH 03/14] add test for fetch_easyblocks_from_pr --- test/framework/github.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/framework/github.py b/test/framework/github.py index 4b4c68c31c..251d69464f 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -245,6 +245,33 @@ def test_close_pr(self): for pattern in patterns: self.assertTrue(pattern in stdout, "Pattern '%s' found in: %s" % (pattern, stdout)) + def test_fetch_easyblocks_from_pr(self): + """Test fetch_easyblocks_from_pr function.""" + if self.skip_github_tests: + print("Skipping test_fetch_easyblocks_from_pr, no GitHub token available?") + return + + init_config(build_options={ + 'pr_target_account': gh.GITHUB_EB_MAIN, + }) + + # PR with new easyblock plus non-easyblock file + all_ebs_pr1964 = ['lammps.py'] + + # PR with changed easyblock + all_ebs_pr1967 = ['siesta.py'] + + # PR with more than one easyblock + all_ebs_pr1949 = ['configuremake.py', 'rpackage.py'] + + for pr, all_ebs in [(1964, all_ebs_pr1964), (1967, all_ebs_pr_1967), (1949, all_ebs_pr_1949)]: + try: + tmpdir = os.path.join(self.test_prefix, 'pr%s' % pr) + eb_files = gh.fetch_easyblocks_from_pr(pr, path=tmpdir, github_user=GITHUB_TEST_ACCOUNT) + self.assertEqual(sorted(all_ebs), sorted([os.path.basename(f) for f in eb_files])) + except URLError as err: + print("Ignoring URLError '%s' in test_fetch_easyconfigs_from_pr" % err) + def test_fetch_easyconfigs_from_pr(self): """Test fetch_easyconfigs_from_pr function.""" if self.skip_github_tests: From 1ba8e56100a421f6fcd7b3aaf11cb12853fc596e Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 20 Feb 2020 15:14:46 +0800 Subject: [PATCH 04/14] fix typos in fetch_easyblocks_from_pr --- test/framework/github.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/framework/github.py b/test/framework/github.py index 251d69464f..22714da9fa 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -264,13 +264,13 @@ def test_fetch_easyblocks_from_pr(self): # PR with more than one easyblock all_ebs_pr1949 = ['configuremake.py', 'rpackage.py'] - for pr, all_ebs in [(1964, all_ebs_pr1964), (1967, all_ebs_pr_1967), (1949, all_ebs_pr_1949)]: + for pr, all_ebs in [(1964, all_ebs_pr1964), (1967, all_ebs_pr1967), (1949, all_ebs_pr1949)]: try: tmpdir = os.path.join(self.test_prefix, 'pr%s' % pr) eb_files = gh.fetch_easyblocks_from_pr(pr, path=tmpdir, github_user=GITHUB_TEST_ACCOUNT) self.assertEqual(sorted(all_ebs), sorted([os.path.basename(f) for f in eb_files])) except URLError as err: - print("Ignoring URLError '%s' in test_fetch_easyconfigs_from_pr" % err) + print("Ignoring URLError '%s' in test_fetch_easyblocks_from_pr" % err) def test_fetch_easyconfigs_from_pr(self): """Test fetch_easyconfigs_from_pr function.""" From 51d36baea53ea9294c1a35a54d53105fcaba7481 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 20 Feb 2020 15:39:30 +0800 Subject: [PATCH 05/14] fix test_fetch_easyblocks_from_pr --- test/framework/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/github.py b/test/framework/github.py index 22714da9fa..b64b98cea2 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -256,7 +256,7 @@ def test_fetch_easyblocks_from_pr(self): }) # PR with new easyblock plus non-easyblock file - all_ebs_pr1964 = ['lammps.py'] + all_ebs_pr1964 = ['.gitignore', 'lammps.py'] # PR with changed easyblock all_ebs_pr1967 = ['siesta.py'] From 1f995afd87cafb641711a97828d05ba2732352b5 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 9 Mar 2020 17:06:30 +0800 Subject: [PATCH 06/14] move handling of options.include_easyblocks_from_pr to options.set_up_configuration --- easybuild/main.py | 9 ++------- easybuild/tools/options.py | 7 ++++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/easybuild/main.py b/easybuild/main.py index cf9779d7cc..69c47a7293 100644 --- a/easybuild/main.py +++ b/easybuild/main.py @@ -57,13 +57,12 @@ from easybuild.tools.containers.common import containerize from easybuild.tools.docs import list_software from easybuild.tools.filetools import adjust_permissions, cleanup, copy_file, copy_files, read_file, write_file -from easybuild.tools.github import check_github, close_pr, new_branch_github, fetch_easyblocks_from_pr -from easybuild.tools.github import find_easybuild_easyconfig +from easybuild.tools.github import check_github, close_pr, new_branch_github, find_easybuild_easyconfig from easybuild.tools.github import install_github_token, list_prs, new_pr, new_pr_from_branch, merge_pr from easybuild.tools.github import sync_branch_with_develop, sync_pr_with_develop, update_branch, update_pr from easybuild.tools.hooks import START, END, load_hooks, run_hook from easybuild.tools.modules import modules_tool -from easybuild.tools.options import include_easyblocks, set_up_configuration, use_color +from easybuild.tools.options import set_up_configuration, use_color from easybuild.tools.robot import check_conflicts, dry_run, missing_deps, resolve_dependencies, search_easyconfigs from easybuild.tools.package.utilities import check_pkg_support from easybuild.tools.parallelbuild import submit_jobs @@ -200,10 +199,6 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): eb_go, cfg_settings = set_up_configuration(args=args, logfile=logfile, testing=testing) options, orig_paths = eb_go.options, eb_go.args - if options.include_easyblocks_from_pr: - included_easyblocks = fetch_easyblocks_from_pr(options.include_easyblocks_from_pr) - include_easyblocks(options.tmpdir, included_easyblocks) - global _log (build_specs, _log, logfile, robot_path, search_query, eb_tmpdir, try_to_generate, tweaked_ecs_paths) = cfg_settings diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index e8ed232161..7d622fbb1a 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -79,7 +79,7 @@ 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 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS -from easybuild.tools.github import fetch_github_token +from easybuild.tools.github import fetch_easyblocks_from_pr, fetch_github_token from easybuild.tools.hooks import KNOWN_HOOKS from easybuild.tools.include import include_easyblocks, include_module_naming_schemes, include_toolchains from easybuild.tools.job.backend import avail_job_backends @@ -1395,6 +1395,11 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): init(options, config_options_dict) 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: + included_easyblocks = fetch_easyblocks_from_pr(eb_go.options.include_easyblocks_from_pr) + include_easyblocks(eb_go.options.tmpdir, included_easyblocks) + check_python_version() # move directory containing fake vsc namespace into temporary directory used for this session From b1368d6c1ee136c3cbe37c2fd68b3d794836d63d Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 9 Mar 2020 17:07:33 +0800 Subject: [PATCH 07/14] use tempfile.mkdtemp for included easyblocks --- easybuild/tools/include.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/include.py b/easybuild/tools/include.py index e71ed7a161..2e85d99e20 100644 --- a/easybuild/tools/include.py +++ b/easybuild/tools/include.py @@ -31,6 +31,7 @@ import os import re import sys +import tempfile from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError @@ -147,7 +148,7 @@ def is_software_specific_easyblock(module): def include_easyblocks(tmpdir, paths): """Include generic and software-specific easyblocks found in specified locations.""" - easyblocks_path = os.path.join(tmpdir, 'included-easyblocks') + easyblocks_path = tempfile.mkdtemp(dir=tmpdir, prefix='included-easyblocks-') set_up_eb_package(easyblocks_path, 'easybuild.easyblocks', subpkgs=['generic'], pkg_init_body=EASYBLOCKS_PKG_INIT_BODY) From a5254e22ad290d530f797ac28594ddb2df37deae Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 9 Mar 2020 17:09:51 +0800 Subject: [PATCH 08/14] make fetch_files_from_pr also work for easyblocks also when PR is already merged --- easybuild/tools/github.py | 21 +++++++++++++-------- test/framework/github.py | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 2846673e91..e8baa141cb 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -381,7 +381,7 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None): def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): - """Fetch patched easyconfig files for a particular PR.""" + """Fetch patched files for a particular PR.""" if github_user is None: github_user = build_option('github_user') @@ -398,10 +398,15 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): if github_repo is None: github_repo = GITHUB_EASYCONFIGS_REPO - elif github_repo not in [GITHUB_EASYBLOCKS_REPO, GITHUB_EASYCONFIGS_REPO]: + + if github_repo == GITHUB_EASYCONFIGS_REPO: + easyfiles = 'easyconfigs' + elif github_repo == GITHUB_EASYBLOCKS_REPO: + easyfiles = 'easyblocks' + else: raise EasyBuildError("Don't know how to fetch files from repo %s", github_repo) - easyfiles = 'easyconfigs' if github_repo == GITHUB_EASYCONFIGS_REPO else 'easyblocks' + subdir = os.path.join('easybuild', easyfiles) _log.debug("Fetching %s from %s/%s PR #%s into %s", easyfiles, github_account, github_repo, pr, path) pr_data, _ = fetch_pr_data(pr, github_account, github_repo, github_user) @@ -449,9 +454,9 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): print_warning("Using %s from closed PR #%s" % (easyfiles, pr)) # obtain most recent version of patched files - for patched_file in patched_files: + for patched_file in [f for f in patched_files if subdir in f]: # path to patch file, incl. subdir it is in - fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:]) + fn = patched_file.split(subdir)[1].strip(os.path.sep) sha = pr_data['head']['sha'] full_url = URL_SEPARATOR.join([GITHUB_RAW, github_account, github_repo, sha, patched_file]) _log.info("Downloading %s from %s", fn, full_url) @@ -461,14 +466,14 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): # symlink directories into expected place if they're not there yet if final_path != path: - dirpath = os.path.join(final_path, 'easybuild', easyfiles) + dirpath = os.path.join(final_path, subdir) for eb_dir in os.listdir(dirpath): symlink(os.path.join(dirpath, eb_dir), os.path.join(path, os.path.basename(eb_dir))) # sanity check: make sure all patched files are downloaded files = [] - for patched_file in [f for f in patched_files if not f.startswith('test/')]: - fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:]) + for patched_file in [f for f in patched_files if subdir in f]: + fn = patched_file.split(easyfiles)[1].strip(os.path.sep) full_path = os.path.join(path, fn) if os.path.exists(full_path): files.append(full_path) diff --git a/test/framework/github.py b/test/framework/github.py index b64b98cea2..22714da9fa 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -256,7 +256,7 @@ def test_fetch_easyblocks_from_pr(self): }) # PR with new easyblock plus non-easyblock file - all_ebs_pr1964 = ['.gitignore', 'lammps.py'] + all_ebs_pr1964 = ['lammps.py'] # PR with changed easyblock all_ebs_pr1967 = ['siesta.py'] From 1a5d0b013c3aadf01805ca157fd26cf04221f848 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 9 Mar 2020 18:00:19 +0800 Subject: [PATCH 09/14] fix tests after use of mkdtemp for included easyblocks --- test/framework/options.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index bcb3dcbe09..810291e694 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2401,7 +2401,7 @@ def test_xxx_include_easyblocks(self): self.eb_main(args, logfile=dummylogfn, raise_error=True) logtxt = read_file(self.logfile) - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks', 'foo.py') + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks', 'foo.py') foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % path_pattern, re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) @@ -2444,7 +2444,7 @@ def test_xxx_include_generic_easyblocks(self): self.eb_main(args, logfile=dummylogfn, raise_error=True) logtxt = read_file(self.logfile) - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks', + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks', 'generic', 'foobar.py') foo_regex = re.compile(r"^\|-- FooBar \(easybuild.easyblocks.generic.foobar @ %s\)" % path_pattern, re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) @@ -2482,7 +2482,7 @@ def test_xxx_include_generic_easyblocks(self): logtxt = read_file(self.logfile) mod_pattern = 'easybuild.easyblocks.generic.generictest' - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks', + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks', 'generic', 'generictest.py') foo_regex = re.compile(r"^\|-- GenericTest \(%s @ %s\)" % (mod_pattern, path_pattern), re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) From 476b96f6f2cdc1246bdf251472cccaf67dba4dfb Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 9 Mar 2020 18:23:13 +0800 Subject: [PATCH 10/14] appease the hound --- test/framework/options.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/framework/options.py b/test/framework/options.py index 810291e694..71e22e7200 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2401,7 +2401,8 @@ def test_xxx_include_easyblocks(self): self.eb_main(args, logfile=dummylogfn, raise_error=True) logtxt = read_file(self.logfile) - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks', 'foo.py') + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', + 'easybuild', 'easyblocks', 'foo.py') foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % path_pattern, re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) From d61b52adc03edf75984d165fbc5f773fcdbc5c2f Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Wed, 11 Mar 2020 14:27:42 +0800 Subject: [PATCH 11/14] make --list-easyblocks aware of --include-easyblocks-from-pr and test using both --include-easyblocks options --- easybuild/tools/options.py | 29 ++++++++++++++++++++----- test/framework/options.py | 43 ++++++++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 7d622fbb1a..19b1c4d9ed 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1042,8 +1042,8 @@ def _postprocess_list_avail(self): if self.options.avail_easyconfig_licenses: msg += avail_easyconfig_licenses(self.options.output_format) - # dump available easyblocks - if self.options.list_easyblocks: + # dump available easyblocks (unless including easyblocks from pr, in which case it will be done later) + if self.options.list_easyblocks and not self.options.include_easyblocks_from_pr: msg += list_easyblocks(self.options.list_easyblocks, self.options.output_format) # dump known toolchains @@ -1087,7 +1087,8 @@ def _postprocess_list_avail(self): print(msg) # cleanup tmpdir and exit - cleanup_and_exit(self.tmpdir) + if not self.options.include_easyblocks_from_pr: + cleanup_and_exit(self.tmpdir) def avail_repositories(self): """Show list of known repository types.""" @@ -1397,8 +1398,26 @@ 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: - included_easyblocks = fetch_easyblocks_from_pr(eb_go.options.include_easyblocks_from_pr) - include_easyblocks(eb_go.options.tmpdir, included_easyblocks) + easyblocks_from_pr = fetch_easyblocks_from_pr(eb_go.options.include_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]) + included_from_file = set([os.path.basename(eb) for eb in eb_go.options.include_easyblocks]) + 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)) + + include_easyblocks(eb_go.options.tmpdir, easyblocks_from_pr) + + if eb_go.options.list_easyblocks: + msg = list_easyblocks(eb_go.options.list_easyblocks, eb_go.options.output_format) + if eb_go.options.unittest_file: + log.info(msg) + else: + print(msg) + cleanup_and_exit(tmpdir) check_python_version() diff --git a/test/framework/options.py b/test/framework/options.py index 71e22e7200..31d302dc75 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2351,7 +2351,7 @@ def generate_cmd_line(ebopts): # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... def test_xxx_include_easyblocks(self): - """Test --include-easyblocks.""" + """Test --include-easyblocks*.""" orig_local_sys_path = sys.path[:] fd, dummylogfn = tempfile.mkstemp(prefix='easybuild-dummy', suffix='.log') @@ -2393,25 +2393,56 @@ def test_xxx_include_easyblocks(self): # clear log write_file(self.logfile, '') + # include both extra EB_foo easyblock and an easyblock from a PR args = [ '--include-easyblocks=%s/*.py' % self.test_prefix, + '--include-easyblocks-from-pr=1915', '--list-easyblocks=detailed', '--unittest-file=%s' % self.logfile, ] self.eb_main(args, logfile=dummylogfn, raise_error=True) logtxt = read_file(self.logfile) - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', - 'easybuild', 'easyblocks', 'foo.py') - foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % path_pattern, re.M) + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks') + + foo_pattern = os.path.join(path_pattern, 'foo.py') + foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % foo_pattern, re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) - # easyblock is found via get_easyblock_class + 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)) + + # easyblocks are found via get_easyblock_class klass = get_easyblock_class('EB_foo') self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) - # 'undo' import of foo easyblock + klass = get_easyblock_class('CMakeMake') + self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) + + # 'undo' import of foo and cmakemake easyblock del sys.modules['easybuild.easyblocks.foo'] + del sys.modules['easybuild.easyblocks.generic.cmakemake'] + + # include extra test cmakemake easyblock + cmm_txt = '\n'.join([ + 'from easybuild.framework.easyblock import EasyBlock', + 'class CMakeMake(EasyBlock):', + ' pass', + '' + ]) + write_file(os.path.join(self.test_prefix, 'cmakemake.py'), cmm_txt) + + # including the same easyblock twice should fail + args = [ + '--include-easyblocks=%s/cmakemake.py' % self.test_prefix, + '--include-easyblocks-from-pr=1915', + '--list-easyblocks=detailed', + '--unittest-file=%s' % self.logfile, + ] + self.assertErrorRegex(EasyBuildError, + "Multiple inclusion of cmakemake.py, check your --include-easyblocks options", + self.eb_main, args, raise_error=True) # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... From 998c5da04cc9a2b7af5c79be9dc3757e39454c57 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Wed, 11 Mar 2020 18:31:39 +0800 Subject: [PATCH 12/14] use different paths when fetching both easyconfigs or easyblocks from PRs --- easybuild/tools/github.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index d197736dc6..0dd8eced55 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -385,8 +385,15 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): if github_user is None: github_user = build_option('github_user') + + if github_repo is None: + github_repo = GITHUB_EASYCONFIGS_REPO + if path is None: - path = build_option('pr_path') + if github_repo == GITHUB_EASYCONFIGS_REPO: + path = build_option('pr_path') + elif github_repo == GITHUB_EASYBLOCKS_REPO: + path = 'ebs_pr%s' % pr if path is None: path = tempfile.mkdtemp() @@ -396,9 +403,6 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): github_account = build_option('pr_target_account') - if github_repo is None: - github_repo = GITHUB_EASYCONFIGS_REPO - if github_repo == GITHUB_EASYCONFIGS_REPO: easyfiles = 'easyconfigs' elif github_repo == GITHUB_EASYBLOCKS_REPO: From 255fcec28991bf73ba2ec5553a15bf36f93f2e47 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Thu, 12 Mar 2020 11:48:36 +0800 Subject: [PATCH 13/14] add separate test for include_easyblocks_from_pr --- test/framework/options.py | 150 ++++++++++++++++++++++++++++---------- 1 file changed, 113 insertions(+), 37 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index 41ba6ce92c..1d981eb577 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2354,7 +2354,7 @@ def generate_cmd_line(ebopts): # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... def test_xxx_include_easyblocks(self): - """Test --include-easyblocks*.""" + """Test --include-easyblocks.""" orig_local_sys_path = sys.path[:] fd, dummylogfn = tempfile.mkstemp(prefix='easybuild-dummy', suffix='.log') @@ -2396,56 +2396,25 @@ def test_xxx_include_easyblocks(self): # clear log write_file(self.logfile, '') - # include both extra EB_foo easyblock and an easyblock from a PR args = [ '--include-easyblocks=%s/*.py' % self.test_prefix, - '--include-easyblocks-from-pr=1915', '--list-easyblocks=detailed', '--unittest-file=%s' % self.logfile, ] self.eb_main(args, logfile=dummylogfn, raise_error=True) logtxt = read_file(self.logfile) - path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks') - - foo_pattern = os.path.join(path_pattern, 'foo.py') - foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % foo_pattern, re.M) + path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks', + 'foo.py') + foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % path_pattern, re.M) self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt)) - 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)) - - # easyblocks are found via get_easyblock_class + # easyblock is found via get_easyblock_class klass = get_easyblock_class('EB_foo') self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) - klass = get_easyblock_class('CMakeMake') - self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) - - # 'undo' import of foo and cmakemake easyblock + # 'undo' import of foo easyblock del sys.modules['easybuild.easyblocks.foo'] - del sys.modules['easybuild.easyblocks.generic.cmakemake'] - - # include extra test cmakemake easyblock - cmm_txt = '\n'.join([ - 'from easybuild.framework.easyblock import EasyBlock', - 'class CMakeMake(EasyBlock):', - ' pass', - '' - ]) - write_file(os.path.join(self.test_prefix, 'cmakemake.py'), cmm_txt) - - # including the same easyblock twice should fail - args = [ - '--include-easyblocks=%s/cmakemake.py' % self.test_prefix, - '--include-easyblocks-from-pr=1915', - '--list-easyblocks=detailed', - '--unittest-file=%s' % self.logfile, - ] - self.assertErrorRegex(EasyBuildError, - "Multiple inclusion of cmakemake.py, check your --include-easyblocks options", - self.eb_main, args, raise_error=True) # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... @@ -2528,6 +2497,113 @@ def test_xxx_include_generic_easyblocks(self): # 'undo' import of foo easyblock del sys.modules['easybuild.easyblocks.generic.generictest'] + # must be run after test for --list-easyblocks, hence the '_xxx_' + # cleaning up the imported easyblocks is quite difficult... + def test_xxx_include_easyblocks_from_pr(self): + """Test --include-easyblocks-from-pr.""" + if self.github_token is None: + print("Skipping test_preview_pr, no GitHub token available?") + return + + orig_local_sys_path = sys.path[:] + fd, dummylogfn = tempfile.mkstemp(prefix='easybuild-dummy', suffix='.log') + os.close(fd) + + # clear log + write_file(self.logfile, '') + + # include extra test easyblock + foo_txt = '\n'.join([ + 'from easybuild.framework.easyblock import EasyBlock', + 'class EB_foo(EasyBlock):', + ' pass', + '' + ]) + write_file(os.path.join(self.test_prefix, 'foo.py'), foo_txt) + + args = [ + '--include-easyblocks=%s/*.py' % self.test_prefix, # this shouldn't interfere + '--include-easyblocks-from-pr=1915', # a PR for CMakeMake easyblock + '--list-easyblocks=detailed', + '--unittest-file=%s' % self.logfile, + '--github-user=%s' % GITHUB_TEST_ACCOUNT, + ] + 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, 'foo.py')) + sys.path = orig_local_sys_path + import easybuild.easyblocks + reload(easybuild.easyblocks) + import easybuild.easyblocks.generic + reload(easybuild.easyblocks.generic) + + # include test cmakemake easyblock + cmm_txt = '\n'.join([ + 'from easybuild.framework.easyblock import EasyBlock', + 'class CMakeMake(EasyBlock):', + ' pass', + '' + ]) + write_file(os.path.join(self.test_prefix, 'cmakemake.py'), cmm_txt) + + # including the same easyblock twice should fail + args = [ + '--include-easyblocks=%s/cmakemake.py' % self.test_prefix, + '--include-easyblocks-from-pr=1915', + '--list-easyblocks=detailed', + '--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) + + os.remove(os.path.join(self.test_prefix, 'cmakemake.py')) + + # clear log + write_file(self.logfile, '') + + args = [ + '--from-pr=9979', # PR for CMake easyconfig + '--include-easyblocks-from-pr=1936', # PR for EB_CMake easyblock + '--unittest-file=%s' % self.logfile, + '--github-user=%s' % GITHUB_TEST_ACCOUNT, + '--extended-dry-run', + ] + self.eb_main(args, logfile=dummylogfn, raise_error=True) + logtxt = read_file(self.logfile) + + # easyconfig from pr is found + ec_pattern = os.path.join(self.test_prefix, '.*', 'files_pr9979', 'c', 'CMake', + 'CMake-3.16.4-GCCcore-9.2.0.eb') + ec_regex = re.compile(r"Parsing easyconfig file %s" % ec_pattern, re.M) + self.assertTrue(ec_regex.search(logtxt), "Pattern '%s' found in: %s" % (ec_regex.pattern, logtxt)) + + # easyblock included from pr is found + eb_regex = re.compile(r"Successfully obtained EB_CMake class instance from easybuild.easyblocks.cmake", re.M) + self.assertTrue(eb_regex.search(logtxt), "Pattern '%s' found in: %s" % (eb_regex.pattern, logtxt)) + + # easyblock is found via get_easyblock_class + klass = get_easyblock_class('EB_CMake') + self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass) + + # 'undo' import of easyblocks + del sys.modules['easybuild.easyblocks.cmake'] + def mk_eb_test_cmd(self, args): """Construct test command for 'eb' with given options.""" From 76ef5f80991f89af17dd312f3cae23913baeda05 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 4 Apr 2020 17:35:16 +0200 Subject: [PATCH 14/14] use subdir in temp dir for easyblocks downloaded from PR --- easybuild/tools/github.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 0dd8eced55..400d226a0c 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -393,7 +393,9 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None): if github_repo == GITHUB_EASYCONFIGS_REPO: path = build_option('pr_path') elif github_repo == GITHUB_EASYBLOCKS_REPO: - path = 'ebs_pr%s' % pr + path = os.path.join(tempfile.gettempdir(), 'ebs_pr%s' % pr) + else: + raise EasyBuildError("Unknown repo: %s" % github_repo) if path is None: path = tempfile.mkdtemp()