From 9413335872b5c1c10caaa1d25e94627bc2321cf3 Mon Sep 17 00:00:00 2001 From: Caylo Date: Tue, 26 Jul 2016 11:46:54 +0200 Subject: [PATCH 01/30] WIP --- easybuild/framework/easyblock.py | 2 +- easybuild/tools/github.py | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index dbd2acd512..b7536285a1 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -668,7 +668,7 @@ def obtain_file(self, filename, extension=False, urls=None): raise EasyBuildError("Couldn't find file %s anywhere, and downloading it didn't work either... " "Paths attempted (in order): %s ", filename, ', '.join(failedpaths)) - # + # a # GETTER/SETTER UTILITY FUNCTIONS # @property diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index a599b77a2b..1669c57821 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -81,6 +81,7 @@ GITHUB_API_URL = 'https://api.github.com' GITHUB_DIR_TYPE = u'dir' GITHUB_EB_MAIN = 'hpcugent' +GITHUB_EASYBLOCKS_REPO = 'easybuild-easyblocks' GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs' GITHUB_FILE_TYPE = u'file' GITHUB_MAX_PER_PAGE = 100 @@ -584,9 +585,11 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') git_repo = init_repo(git_working_dir, pr_target_repo) - if pr_target_repo != GITHUB_EASYCONFIGS_REPO: + if pr_target_repo != GITHUB_EASYCONFIGS_REPO and pr_target_repo != GITHUB_EASYBLOCKS_REPO: raise EasyBuildError("Don't know how to create/update a pull request to the %s repository", pr_target_repo) + easyblocks = pr_target_repo == GITHUB_EASYBLOCKS_REPO + if start_branch is None: start_branch = build_option('pr_target_branch') @@ -596,7 +599,10 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco _log.debug("git status: %s", git_repo.git.status()) # copy files to right place - file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) + if easyblocks: + copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + else: + file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) # checkout target branch if pr_branch is None: @@ -659,6 +665,12 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco return file_info, git_repo, pr_branch, diff_stat +def copy_easyblocks(paths, targetdir): + # circular dependencies ugh + from easybuild.framework.easyblock import EasyBlock + print "new pr for easyblock" + + @only_if_module_is_available('git', pkgname='GitPython') def new_pr(paths, title=None, descr=None, commit_msg=None): """Open new pull request using specified files.""" @@ -1059,3 +1071,4 @@ def validate_github_token(token, github_user): _log.info("GitHub token can be used for authenticated GitHub access, validation passed") return sanity_check and token_test + From 874b53a4df018352307fc2258eeb0934fe7362b5 Mon Sep 17 00:00:00 2001 From: Caylo Date: Tue, 26 Jul 2016 15:53:19 +0200 Subject: [PATCH 02/30] --new-pr for easyblocks repo --- easybuild/framework/easyblock.py | 2 +- easybuild/tools/github.py | 106 +++++++++++++++++++++++-------- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index b7536285a1..dbd2acd512 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -668,7 +668,7 @@ def obtain_file(self, filename, extension=False, urls=None): raise EasyBuildError("Couldn't find file %s anywhere, and downloading it didn't work either... " "Paths attempted (in order): %s ", filename, ', '.join(failedpaths)) - # a + # # GETTER/SETTER UTILITY FUNCTIONS # @property diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 1669c57821..709d39f592 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -31,6 +31,8 @@ """ import base64 import getpass +import imp +import inspect import os import random import re @@ -47,8 +49,8 @@ from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs from easybuild.tools.build_log import EasyBuildError, print_msg from easybuild.tools.config import build_option -from easybuild.tools.filetools import det_patched_files, download_file, extract_file, mkdir, read_file -from easybuild.tools.filetools import which, write_file +from easybuild.tools.filetools import det_patched_files, decode_class_name, download_file, extract_file, mkdir +from easybuild.tools.filetools import read_file, which, write_file from easybuild.tools.systemtools import UNKNOWN, get_tool_version from easybuild.tools.utilities import only_if_module_is_available @@ -77,6 +79,8 @@ _log.warning("Failed to import 'git' Python module: %s", err) +EB_PREFIX = 'EB_' +GENERIC_EB = 'generic' GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' GITHUB_DIR_TYPE = u'dir' @@ -91,6 +95,7 @@ HTTP_STATUS_OK = 200 HTTP_STATUS_CREATED = 201 KEYRING_GITHUB_TOKEN = 'github_token' +PYTHON_EXTENSION = 'py' URL_SEPARATOR = '/' @@ -600,13 +605,14 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco # copy files to right place if easyblocks: - copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + name_version, file_info = copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + else: file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) + name_version = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.') # checkout target branch if pr_branch is None: - name_version = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.') pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), name_version) # create branch to commit to and push; @@ -665,10 +671,49 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco return file_info, git_repo, pr_branch, diff_stat -def copy_easyblocks(paths, targetdir): - # circular dependencies ugh - from easybuild.framework.easyblock import EasyBlock - print "new pr for easyblock" +def copy_easyblocks(paths, target_dir): + file_info = { + 'paths_in_repo': [], + 'new': [], + 'ebs' : [], + } + + subdir = os.path.join('easybuild', 'easyblocks') + if os.path.exists(os.path.join(target_dir, subdir)): + for path in paths: + fn = os.path.basename(path).split('.')[0] + + mod = imp.load_source(fn, path) + clsmembers = inspect.getmembers(mod, inspect.isclass) + classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] + + if len(classnames) > 1: + raise EasyBuildError("Invalid EB file") + + cn = classnames[0] + eb_name = decode_class_name(cn).lower() # TODO not fully right yet. - to _ (and others??) + if cn.startswith(EB_PREFIX): + # regular eb file + letter = fn.lower()[0] + target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) + else: + # generic + target_path = os.path.join(subdir, GENERIC_EB, "%s.%s" % (eb_name.lower(), PYTHON_EXTENSION)) + + full_target_path = os.path.join(target_dir, target_path) + file_info['paths_in_repo'].append(full_target_path) + file_info['ebs'].append(eb_name) + try: + file_info['new'].append(not os.path.exists(full_target_path)) + + mkdir(os.path.dirname(full_target_path), parents=True) + shutil.copy2(path, full_target_path) + _log.info("%s copied to %s", path, full_target_path) + + except OSError as err: + raise EasyBuildError("Failed to copy %s to %s: %s", path, target_path, err) + + return eb_name, file_info @only_if_module_is_available('git', pkgname='GitPython') @@ -698,24 +743,36 @@ def new_pr(paths, title=None, descr=None, commit_msg=None): commit_msg=commit_msg) # only use most common toolchain(s) in toolchain label of PR title - toolchains = ['%(name)s/%(version)s' % ec['toolchain'] for ec in file_info['ecs']] - toolchains_counted = sorted([(toolchains.count(tc), tc) for tc in nub(toolchains)]) - toolchain_label = ','.join([tc for (cnt, tc) in toolchains_counted if cnt == toolchains_counted[-1][0]]) - - # only use most common module class(es) in moduleclass label of PR title - classes = [ec['moduleclass'] for ec in file_info['ecs']] - classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) - class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) - if title is None: - # mention software name/version in PR title (only first 3) - names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] - if len(names_and_versions) <= 3: - main_title = ', '.join(names_and_versions) - else: - main_title = ', '.join(names_and_versions[:3] + ['...']) + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + + toolchains = ['%(name)s/%(version)s' % ec['toolchain'] for ec in file_info['ecs']] + toolchains_counted = sorted([(toolchains.count(tc), tc) for tc in nub(toolchains)]) + toolchain_label = ','.join([tc for (cnt, tc) in toolchains_counted if cnt == toolchains_counted[-1][0]]) + + # only use most common module class(es) in moduleclass label of PR title + classes = [ec['moduleclass'] for ec in file_info['ecs']] + classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) + class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) + + if title is None: + # mention software name/version in PR title (only first 3) + names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] + if len(names_and_versions) <= 3: + main_title = ', '.join(names_and_versions) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) + + title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) + + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO: + names = file_info['ebs'] + if len(names) <= 3: + main_title = ', '.join(names) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) + title = "EasyBlock for %s" % main_title - title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) full_descr = "(created using `eb --new-pr`)\n" if descr is not None: @@ -1071,4 +1128,3 @@ def validate_github_token(token, github_user): _log.info("GitHub token can be used for authenticated GitHub access, validation passed") return sanity_check and token_test - From a484f9ed8961646ec5017007b34503d2dd928977 Mon Sep 17 00:00:00 2001 From: Caylo Date: Wed, 27 Jul 2016 10:07:33 +0200 Subject: [PATCH 03/30] small fix --- easybuild/tools/github.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 709d39f592..170ea78ba4 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -605,7 +605,8 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco # copy files to right place if easyblocks: - name_version, file_info = copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + file_info = copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + name_version = file_info['ebs'][0] else: file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) @@ -713,7 +714,7 @@ def copy_easyblocks(paths, target_dir): except OSError as err: raise EasyBuildError("Failed to copy %s to %s: %s", path, target_path, err) - return eb_name, file_info + return file_info @only_if_module_is_available('git', pkgname='GitPython') @@ -755,15 +756,14 @@ def new_pr(paths, title=None, descr=None, commit_msg=None): classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) - if title is None: - # mention software name/version in PR title (only first 3) - names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] - if len(names_and_versions) <= 3: - main_title = ', '.join(names_and_versions) - else: - main_title = ', '.join(names_and_versions[:3] + ['...']) + # mention software name/version in PR title (only first 3) + names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] + if len(names_and_versions) <= 3: + main_title = ', '.join(names_and_versions) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) - title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) + title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) elif pr_target_repo == GITHUB_EASYBLOCKS_REPO: names = file_info['ebs'] From b889c8ca782ae7ad8b58216e9f7ae0b948a8c4cf Mon Sep 17 00:00:00 2001 From: Caylo Date: Tue, 26 Jul 2016 13:08:11 +0200 Subject: [PATCH 04/30] --new-pr for framework repo --- easybuild/tools/github.py | 98 ++++++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index a599b77a2b..0d361086d6 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -82,6 +82,7 @@ GITHUB_DIR_TYPE = u'dir' GITHUB_EB_MAIN = 'hpcugent' GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs' +GITHUB_FRAMEWORK_REPO = 'easybuild-framework' GITHUB_FILE_TYPE = u'file' GITHUB_MAX_PER_PAGE = 100 GITHUB_MERGEABLE_STATE_CLEAN = 'clean' @@ -584,9 +585,11 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') git_repo = init_repo(git_working_dir, pr_target_repo) - if pr_target_repo != GITHUB_EASYCONFIGS_REPO: + if not pr_target_repo in [GITHUB_EASYCONFIGS_REPO, GITHUB_FRAMEWORK_REPO,]: raise EasyBuildError("Don't know how to create/update a pull request to the %s repository", pr_target_repo) + framework = pr_target_repo == GITHUB_FRAMEWORK_REPO + if start_branch is None: start_branch = build_option('pr_target_branch') @@ -596,11 +599,16 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco _log.debug("git status: %s", git_repo.git.status()) # copy files to right place - file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) + if framework: + file_info = copy_framework_files(paths, os.path.join(git_working_dir, pr_target_repo)) + name_version = file_info['files'][0] + + else: + file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) + name_version = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.') # checkout target branch if pr_branch is None: - name_version = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.') pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), name_version) # create branch to commit to and push; @@ -659,6 +667,50 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco return file_info, git_repo, pr_branch, diff_stat +def copy_framework_files(paths, target_dir): + file_info = { + 'paths_in_repo': [], + 'new': [], + 'files' : [], + } + + + dirs = [x[0] for x in os.walk(target_dir)] + paths = [os.path.abspath(path) for path in paths] + + target_path = None + for path in paths: + fn = os.path.basename(path) + dirnames = os.path.dirname(path).split(os.path.sep) + + if 'easybuild-framework' in dirnames: + ind = dirnames.index('easybuild-framework') + 1 + parent_dir = os.path.join(*dirnames[ind:]) + + if os.path.exists(os.path.join(target_dir, parent_dir)): + target_path = os.path.join(target_dir, parent_dir) + + if target_path is None: + raise EasyBuildError("Couldn't find parent folder of updated file: %s" % path) + + full_target_path = os.path.join(target_path, os.path.basename(path)) + + file_info['paths_in_repo'].append(full_target_path) + file_info['files'].append(os.path.basename(path).split('.')[0]) + + try: + file_info['new'].append(not os.path.exists(full_target_path)) + + mkdir(os.path.dirname(full_target_path), parents=True) + shutil.copy2(path, full_target_path) + _log.info("%s copied to %s", path, full_target_path) + + except OSError as err: + raise EasyBuildError("Failed to copy %s to %s: %s", path, full_target_path, err) + + return file_info + + @only_if_module_is_available('git', pkgname='GitPython') def new_pr(paths, title=None, descr=None, commit_msg=None): """Open new pull request using specified files.""" @@ -686,24 +738,34 @@ def new_pr(paths, title=None, descr=None, commit_msg=None): commit_msg=commit_msg) # only use most common toolchain(s) in toolchain label of PR title - toolchains = ['%(name)s/%(version)s' % ec['toolchain'] for ec in file_info['ecs']] - toolchains_counted = sorted([(toolchains.count(tc), tc) for tc in nub(toolchains)]) - toolchain_label = ','.join([tc for (cnt, tc) in toolchains_counted if cnt == toolchains_counted[-1][0]]) + if title is None: + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: - # only use most common module class(es) in moduleclass label of PR title - classes = [ec['moduleclass'] for ec in file_info['ecs']] - classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) - class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) + toolchains = ['%(name)s/%(version)s' % ec['toolchain'] for ec in file_info['ecs']] + toolchains_counted = sorted([(toolchains.count(tc), tc) for tc in nub(toolchains)]) + toolchain_label = ','.join([tc for (cnt, tc) in toolchains_counted if cnt == toolchains_counted[-1][0]]) - if title is None: - # mention software name/version in PR title (only first 3) - names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] - if len(names_and_versions) <= 3: - main_title = ', '.join(names_and_versions) - else: - main_title = ', '.join(names_and_versions[:3] + ['...']) + # only use most common module class(es) in moduleclass label of PR title + classes = [ec['moduleclass'] for ec in file_info['ecs']] + classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) + class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) - title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) + # mention software name/version in PR title (only first 3) + names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']] + if len(names_and_versions) <= 3: + main_title = ', '.join(names_and_versions) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) + + title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) + + elif pr_target_repo == GITHUB_FRAMEWORK_REPO: + names = file_info['files'] + if len(names) <= 3: + main_title = ', '.join(names) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) + title = "Changes to %s" % main_title full_descr = "(created using `eb --new-pr`)\n" if descr is not None: From 6c79086dd61435b24a2618978a0dcc840c2ab8f2 Mon Sep 17 00:00:00 2001 From: Caylo Date: Wed, 10 Aug 2016 16:45:39 +0200 Subject: [PATCH 05/30] fix remark --- easybuild/tools/github.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 170ea78ba4..c78718bde1 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -49,6 +49,7 @@ from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs from easybuild.tools.build_log import EasyBuildError, print_msg from easybuild.tools.config import build_option +from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX from easybuild.tools.filetools import det_patched_files, decode_class_name, download_file, extract_file, mkdir from easybuild.tools.filetools import read_file, which, write_file from easybuild.tools.systemtools import UNKNOWN, get_tool_version @@ -79,7 +80,6 @@ _log.warning("Failed to import 'git' Python module: %s", err) -EB_PREFIX = 'EB_' GENERIC_EB = 'generic' GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' @@ -693,7 +693,7 @@ def copy_easyblocks(paths, target_dir): cn = classnames[0] eb_name = decode_class_name(cn).lower() # TODO not fully right yet. - to _ (and others??) - if cn.startswith(EB_PREFIX): + if cn.startswith(EASYBLOCK_CLASS_PREFIX): # regular eb file letter = fn.lower()[0] target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) From a046c9e8b50914c2f30c1f45441c08755d171cce Mon Sep 17 00:00:00 2001 From: Caylo Date: Fri, 12 Aug 2016 12:49:57 +0200 Subject: [PATCH 06/30] fix variable name --- easybuild/tools/github.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index ee03ecd537..941e50f12a 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -645,16 +645,16 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco # copy easyconfig files to right place target_dir = os.path.join(git_working_dir, pr_target_repo) - print_msg("copying easyconfigs to %s..." % target_dir) + print_msg("copying files to %s..." % target_dir) if easyblocks: - file_info = copy_easyblocks(paths, os.path.join(git_working_dir, pr_target_repo)) + file_info = copy_easyblocks(ec_paths, os.path.join(git_working_dir, pr_target_repo)) elif framework: - file_info = copy_framework_files(paths, os.path.join(git_working_dir, pr_target_repo)) + file_info = copy_framework_files(ec_paths, os.path.join(git_working_dir, pr_target_repo)) else: - file_info = copy_easyconfigs(paths, os.path.join(git_working_dir, pr_target_repo)) + file_info = copy_easyconfigs(ec_paths, os.path.join(git_working_dir, pr_target_repo)) # figure out to which software name patches relate, and copy them to the right place if patch_paths: From 36bfbfe60e07b3347e705577dbba827aab63d6e3 Mon Sep 17 00:00:00 2001 From: Caylo Date: Fri, 12 Aug 2016 12:57:35 +0200 Subject: [PATCH 07/30] copy functions --- easybuild/tools/github.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 941e50f12a..f0caf2d197 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -628,9 +628,6 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco if pr_target_repo not in [GITHUB_EASYCONFIGS_REPO, GITHUB_EASYBLOCKS_REPO, GITHUB_FRAMEWORK_REPO,]: raise EasyBuildError("Don't know how to create/update a pull request to the %s repository", pr_target_repo) - easyblocks = pr_target_repo == GITHUB_EASYBLOCKS_REPO - framework = pr_target_repo == GITHUB_FRAMEWORK_REPO - if start_branch is None: start_branch = build_option('pr_target_branch') @@ -646,15 +643,7 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco # copy easyconfig files to right place target_dir = os.path.join(git_working_dir, pr_target_repo) print_msg("copying files to %s..." % target_dir) - - if easyblocks: - file_info = copy_easyblocks(ec_paths, os.path.join(git_working_dir, pr_target_repo)) - - elif framework: - file_info = copy_framework_files(ec_paths, os.path.join(git_working_dir, pr_target_repo)) - - else: - file_info = copy_easyconfigs(ec_paths, os.path.join(git_working_dir, pr_target_repo)) + file_info = COPY_FUNCTIONS[pr_target_repo](ec_paths, os.path.join(git_working_dir, pr_target_repo)) # figure out to which software name patches relate, and copy them to the right place if patch_paths: @@ -1319,3 +1308,10 @@ def validate_github_token(token, github_user): _log.info("GitHub token can be used for authenticated GitHub access, validation passed") return sanity_check and token_test + +# copy fucntions for --new-pr +COPY_FUNCTIONS = { + GITHUB_EASYCONFIGS_REPO: copy_easyconfigs, + GITHUB_EASYBLOCKS_REPO: copy_easyblocks, + GITHUB_FRAMEWORK_REPO: copy_framework_files, +} From a0c7e38e07cf3b2a9533321ed769de671aa18429 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Wed, 5 Feb 2020 16:27:37 +0800 Subject: [PATCH 08/30] update test_new_branch_github to also test easyblocks and framework --- easybuild/framework/easyconfig/tools.py | 6 +- easybuild/tools/github.py | 95 +++++++++++++++---------- test/framework/options.py | 51 ++++++++++++- test/framework/sandbox/a_test.py | 3 + 4 files changed, 114 insertions(+), 41 deletions(-) create mode 100644 test/framework/sandbox/a_test.py diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 7d717d6258..b9c79a8136 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -604,17 +604,21 @@ def dump_env_script(easyconfigs): def categorize_files_by_type(paths): """ - Splits list of filepaths into a 3 separate lists: easyconfigs, files to delete and patch files + Splits list of filepaths into a 4 separate lists: easyconfigs, files to delete, patch files and + files with extension .py """ res = { 'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], + 'py_files': [], } for path in paths: if path.startswith(':'): res['files_to_delete'].append(path[1:]) + elif path.endswith('.py'): + res['py_files'].append(path) # file must exist in order to check whether it's a patch file elif os.path.isfile(path) and is_patch_file(path): res['patch_files'].append(path) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 1d1999c998..4937ef95e7 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -681,8 +681,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ # we need files to create the PR with non_existing_paths = [] ec_paths = [] - if paths['easyconfigs']: - for path in paths['easyconfigs']: + if paths['easyconfigs'] or paths['py_files']: + for path in paths['easyconfigs'] + paths['py_files']: if not os.path.exists(path): non_existing_paths.append(path) else: @@ -696,6 +696,15 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ pr_target_repo = build_option('pr_target_repo') + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + if paths['py_files']: + raise EasyBuildError("You are submitting files with .py extension, " + "did you forget to specify --pr-target-repo?") + else: + if paths['easyconfigs'] or paths['patch_files']: + raise EasyBuildError("You are submitting easyconfigs and/or patches, " + "shouldn\'t this PR target the easyconfigs repo?") + # initialize repository git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') git_repo = init_repo(git_working_dir, pr_target_repo) @@ -731,15 +740,17 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ # figure out commit message to use if commit_msg: cnt = len(file_info['paths_in_repo']) - _log.debug("Using specified commit message for all %d new/modified easyconfigs at once: %s", cnt, commit_msg) - elif all(file_info['new']) and not paths['files_to_delete']: + _log.debug("Using specified commit message for all %d new/modified files at once: %s", cnt, commit_msg) + elif pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths['files_to_delete']: # automagically derive meaningful commit message if all easyconfig files are new commit_msg = "adding easyconfigs: %s" % ', '.join(os.path.basename(p) for p in file_info['paths_in_repo']) if paths['patch_files']: commit_msg += " and patches: %s" % ', '.join(os.path.basename(p) for p in paths['patch_files']) + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and all(file_info['new']): + commit_msg = "adding easyblocks: %s" % ', '.join(os.path.basename(p) for p in file_info['paths_in_repo']) else: raise EasyBuildError("A meaningful commit message must be specified via --pr-commit-msg when " - "modifying/deleting easyconfigs") + "modifying/deleting files or targeting the framework repo.") # figure out to which software name patches relate, and copy them to the right place if paths['patch_files']: @@ -996,7 +1007,10 @@ def copy_easyblocks(paths, target_dir): mod = imp.load_source(fn, path) clsmembers = inspect.getmembers(mod, inspect.isclass) - classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] + if clsmembers: + classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] + else: + raise EasyBuildError("Invalid easyblock file") if len(classnames) > 1: raise EasyBuildError("Invalid easyblock file") @@ -1014,7 +1028,7 @@ def copy_easyblocks(paths, target_dir): full_target_path = os.path.join(target_dir, target_path) file_info['paths_in_repo'].append(full_target_path) file_info['new'].append(not os.path.exists(full_target_path)) - copy_file(path, full_target_path) + copy_file(path, full_target_path, force_in_dry_run=True) else: raise EasyBuildError("Subdir easyblocks not found") @@ -1358,11 +1372,10 @@ def new_branch_github(paths, ecs, commit_msg=None): """ Create new branch on GitHub using specified files - :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches) + :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches, files with .py extension) :param ecs: list of parsed easyconfigs, incl. for dependencies (if robot is enabled) :param commit_msg: commit message to use """ - branch_name = build_option('pr_branch_name') if commit_msg is None: commit_msg = build_option('pr_commit_msg') @@ -1473,14 +1486,14 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_metadata=None): file_info = det_file_info(ec_paths, target_dir) - # label easyconfigs for new software and/or new easyconfigs for existing software labels = [] - if any(file_info['new_folder']): - labels.append('new') - if any(file_info['new_file_in_existing_folder']): - labels.append('update') - if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + # label easyconfigs for new software and/or new easyconfigs for existing software + if any(file_info['new_folder']): + labels.append('new') + if any(file_info['new_file_in_existing_folder']): + labels.append('update') + # only use most common toolchain(s) in toolchain label of PR title toolchains = ['%(name)s/%(version)s' % ec['toolchain'] for ec in file_info['ecs']] toolchains_counted = sorted([(toolchains.count(tc), tc) for tc in nub(toolchains)]) @@ -1490,33 +1503,39 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_metadata=None): classes = [ec['moduleclass'] for ec in file_info['ecs']] classes_counted = sorted([(classes.count(c), c) for c in nub(classes)]) class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]]) + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO: + if any(file_info['new']): + labels.append('new') if title is None: + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + if file_info['ecs'] and all(file_info['new']) and not deleted_paths: + # mention software name/version in PR title (only first 3) + names_and_versions = nub(["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']]) + if len(names_and_versions) <= 3: + main_title = ', '.join(names_and_versions) + else: + main_title = ', '.join(names_and_versions[:3] + ['...']) + + title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) + + # if Python is listed as a dependency, then mention Python version(s) in PR title + pyver = [] + for ec in file_info['ecs']: + # iterate over all dependencies (incl. build dependencies & multi-deps) + for dep in ec.dependencies(): + if dep['name'] == 'Python': + # check whether Python is listed as a multi-dep if it's marked as a build dependency + if dep['build_only'] and 'Python' not in ec['multi_deps']: + continue + else: + pyver.append(dep['version']) + if pyver: + title += " w/ Python %s" % ' + '.join(sorted(nub(pyver))) - if file_info['ecs'] and all(file_info['new']) and not deleted_paths: - # mention software name/version in PR title (only first 3) - names_and_versions = nub(["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']]) - if len(names_and_versions) <= 3: - main_title = ', '.join(names_and_versions) else: - main_title = ', '.join(names_and_versions[:3] + ['...']) - - title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title) - - # if Python is listed as a dependency, then mention Python version(s) in PR title - pyver = [] - for ec in file_info['ecs']: - # iterate over all dependencies (incl. build dependencies & multi-deps) - for dep in ec.dependencies(): - if dep['name'] == 'Python': - # check whether Python is listed as a multi-dep if it's marked as a build dependency - if dep['build_only'] and 'Python' not in ec['multi_deps']: - continue - else: - pyver.append(dep['version']) - if pyver: - title += " w/ Python %s" % ' + '.join(sorted(nub(pyver))) - + raise EasyBuildError("Don't know how to make a PR title for this PR. " + "Please include a title (use --pr-title)") else: raise EasyBuildError("Don't know how to make a PR title for this PR. " "Please include a title (use --pr-title)") diff --git a/test/framework/options.py b/test/framework/options.py index bcb3dcbe09..1b42289140 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2940,6 +2940,8 @@ def test_new_branch_github(self): return topdir = os.path.dirname(os.path.abspath(__file__)) + + # test easyconfigs test_ecs = os.path.join(topdir, 'easyconfigs', 'test_ecs') toy_ec = os.path.join(test_ecs, 't', 'toy', 'toy-0.0.eb') @@ -2954,11 +2956,56 @@ def test_new_branch_github(self): remote = 'git@github.com:%s/easybuild-easyconfigs.git' % GITHUB_TEST_ACCOUNT regexs = [ r"^== fetching branch 'develop' from https://github.com/easybuilders/easybuild-easyconfigs.git\.\.\.", - r"^== copying easyconfigs to .*/easybuild-easyconfigs\.\.\.", + r"^== copying files to .*/easybuild-easyconfigs\.\.\.", + r"^== pushing branch '.*' to remote '.*' \(%s\) \[DRY RUN\]" % remote, + ] + self._assert_regexs(regexs, txt) + + # test easyblocks + test_ebs = os.path.join(topdir, 'sandbox', 'easybuild', 'easyblocks') + toy_eb = os.path.join(test_ebs, 't', 'toy.py') + + args = [ + '--new-branch-github', + '--pr-target-repo=easybuild-easyblocks', + '--github-user=%s' % GITHUB_TEST_ACCOUNT, + toy_eb, + '--pr-title="add easyblock for toy"', + '-D', + ] + txt, _ = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False) + + remote = 'git@github.com:%s/easybuild-easyblocks.git' % GITHUB_TEST_ACCOUNT + regexs = [ + r"^== fetching branch 'develop' from https://github.com/easybuilders/easybuild-easyblocks.git\.\.\.", + r"^== copying files to .*/easybuild-easyblocks\.\.\.", r"^== pushing branch '.*' to remote '.*' \(%s\) \[DRY RUN\]" % remote, ] self._assert_regexs(regexs, txt) + # test framework + test_ebs = os.path.join(topdir, 'sandbox') + toy_py = os.path.join(test_ebs, 'a_test.py') + + args = [ + '--new-branch-github', + '--pr-target-repo=easybuild-framework', + '--github-user=%s' % GITHUB_TEST_ACCOUNT, + toy_py, + '--pr-commit-msg="a test"', + '-D', + ] + txt, _ = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False) + + remote = 'git@github.com:%s/easybuild-framework.git' % GITHUB_TEST_ACCOUNT + regexs = [ + r"^== fetching branch 'develop' from https://github.com/easybuilders/easybuild-framework.git\.\.\.", + r"^== copying files to .*/easybuild-framework\.\.\.", + r"^== pushing branch '.*' to remote '.*' \(%s\) \[DRY RUN\]" % remote, + ] + self._assert_regexs(regexs, txt) + + def test_new_pr_from_branch(self): """Test --new-pr-from-branch.""" if self.github_token is None: @@ -3019,7 +3066,7 @@ def test_update_branch_github(self): full_repo = 'boegel/easybuild-easyconfigs' regexs = [ r"^== fetching branch 'develop' from https://github.com/%s.git\.\.\." % full_repo, - r"^== copying easyconfigs to .*/git-working-dir.*/easybuild-easyconfigs...", + r"^== copying files to .*/git-working-dir.*/easybuild-easyconfigs...", r"^== pushing branch 'develop' to remote '.*' \(git@github.com:%s.git\) \[DRY RUN\]" % full_repo, r"^Overview of changes:\n.*/easyconfigs/t/toy/toy-0.0.eb \| 32", r"== pushed updated branch 'develop' to boegel/easybuild-easyconfigs \[DRY RUN\]", diff --git a/test/framework/sandbox/a_test.py b/test/framework/sandbox/a_test.py new file mode 100644 index 0000000000..6d8a26f090 --- /dev/null +++ b/test/framework/sandbox/a_test.py @@ -0,0 +1,3 @@ +""" +Used for test_new_branch_github +""" From 489e801ae799b8d6f1dce97dff3c8e4868eacf07 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Wed, 5 Feb 2020 17:05:45 +0800 Subject: [PATCH 09/30] update test_categorize_files_by_type --- test/framework/easyconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index f956900f59..d5fae16ece 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2713,7 +2713,7 @@ def test_hidden_toolchain(self): def test_categorize_files_by_type(self): """Test categorize_files_by_type""" - self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': []}, categorize_files_by_type([])) + self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], 'py_files': []}, categorize_files_by_type([])) test_ecs_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs',) toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch' From 16fcc926b3ab94e819bb0045cd22265801b8785b Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Fri, 21 Feb 2020 13:02:30 +0800 Subject: [PATCH 10/30] appease the hound --- easybuild/tools/github.py | 4 +--- test/framework/easyconfig.py | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 4937ef95e7..9a90171988 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -54,7 +54,7 @@ from easybuild.tools.config import build_option from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX from easybuild.tools.filetools import apply_patch, copy_dir, copy_file, det_patched_files, decode_class_name -from easybuild.tools.filetools import download_file, extract_file, is_patch_file, mkdir, read_file, symlink +from easybuild.tools.filetools import download_file, extract_file, mkdir, read_file, symlink from easybuild.tools.filetools import which, write_file from easybuild.tools.py2vs3 import HTTPError, URLError, ascii_letters, urlopen from easybuild.tools.systemtools import UNKNOWN, get_tool_version @@ -1043,12 +1043,10 @@ def copy_framework_files(paths, target_dir): 'new': [], } - dirs = [x[0] for x in os.walk(target_dir)] paths = [os.path.abspath(path) for path in paths] target_path = None for path in paths: - fn = os.path.basename(path) dirnames = os.path.dirname(path).split(os.path.sep) if 'easybuild-framework' in dirnames: diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index d5fae16ece..e9e39768ca 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2713,7 +2713,8 @@ def test_hidden_toolchain(self): def test_categorize_files_by_type(self): """Test categorize_files_by_type""" - self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], 'py_files': []}, categorize_files_by_type([])) + self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], 'py_files': []}, + categorize_files_by_type([])) test_ecs_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs',) toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch' From 8e67a1cc58154061bf51cc65a034878f4073e452 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Fri, 21 Feb 2020 18:08:42 +0800 Subject: [PATCH 11/30] flesh out method to determine easyblock class --- easybuild/tools/github.py | 59 ++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 9a90171988..c37fb316f0 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -48,7 +48,7 @@ from easybuild.base import fancylogger from easybuild.framework.easyconfig.easyconfig import EASYCONFIGS_ARCHIVE_DIR from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs, copy_patch_files, det_file_info -from easybuild.framework.easyconfig.easyconfig import process_easyconfig +from easybuild.framework.easyconfig.easyconfig import is_generic_easyblock, process_easyconfig 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 @@ -58,7 +58,7 @@ from easybuild.tools.filetools import which, write_file from easybuild.tools.py2vs3 import HTTPError, URLError, ascii_letters, urlopen from easybuild.tools.systemtools import UNKNOWN, get_tool_version -from easybuild.tools.utilities import nub, only_if_module_is_available +from easybuild.tools.utilities import nub, only_if_module_is_available, remove_unwanted_chars _log = fancylogger.getLogger('github', fname=False) @@ -698,8 +698,14 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if pr_target_repo == GITHUB_EASYCONFIGS_REPO: if paths['py_files']: - raise EasyBuildError("You are submitting files with .py extension, " - "did you forget to specify --pr-target-repo?") + if any([get_easyblock_class(path) for path in paths['py_files']]): + # this is not enough, we would need to change build_option('pr_target_repo') + pr_target_repo = GITHUB_EASYBLOCKS_REPO + raise EasyBuildError("You are submitting easyblock files, " + "did you forget to specify --pr-target-repo=easybuild-easyblocks?") + else: + raise EasyBuildError("You are submitting python files that are not easyblocks, " + "did you forget to specify --pr-target-repo=easybuild-framework?") else: if paths['easyconfigs'] or paths['patch_files']: raise EasyBuildError("You are submitting easyconfigs and/or patches, " @@ -993,6 +999,26 @@ def find_software_name_for_patch(patch_name, ec_dirs): return soft_name +def get_easyblock_class(path): + """Get easyblock class from file""" + fn = os.path.basename(path).split('.')[0] + mod = imp.load_source(fn, path) + clsmembers = inspect.getmembers(mod, inspect.isclass) + is_easyblock = False + for cn in clsmembers: + if cn[0] == 'EasyBlock' or cn[0].startswith(EASYBLOCK_CLASS_PREFIX): + is_easyblock = True + break + if is_easyblock: + classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] + if len(classnames) > 1: + return None + else: + return classnames[0] + else: + return None + + def copy_easyblocks(paths, target_dir): """ Find right location for easyblock file and copy it there""" file_info = { @@ -1003,27 +1029,16 @@ def copy_easyblocks(paths, target_dir): subdir = os.path.join('easybuild', 'easyblocks') if os.path.exists(os.path.join(target_dir, subdir)): for path in paths: - fn = os.path.basename(path).split('.')[0] - - mod = imp.load_source(fn, path) - clsmembers = inspect.getmembers(mod, inspect.isclass) - if clsmembers: - classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] - else: - raise EasyBuildError("Invalid easyblock file") - - if len(classnames) > 1: + cn = get_easyblock_class(path) + if not cn: raise EasyBuildError("Invalid easyblock file") - cn = classnames[0] - eb_name = decode_class_name(cn).lower() # TODO not fully right yet. - to _ (and others??) - if cn.startswith(EASYBLOCK_CLASS_PREFIX): - # regular eb file - letter = fn.lower()[0] - target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) - else: - # generic + eb_name = remove_unwanted_chars(decode_class_name(cn).replace('-', '_')).lower() + if is_generic_easyblock(cn): target_path = os.path.join(subdir, GENERIC_EB, "%s.%s" % (eb_name.lower(), PYTHON_EXTENSION)) + else: + letter = os.path.basename(path).lower()[0] + target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) full_target_path = os.path.join(target_dir, target_path) file_info['paths_in_repo'].append(full_target_path) From c0c61e68407851a1e0f62f7282cc7f407d029e8a Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 24 Feb 2020 16:02:47 +0800 Subject: [PATCH 12/30] improve detection of easyblock class --- easybuild/tools/github.py | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index c37fb316f0..3a4264a92c 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -52,7 +52,6 @@ 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 EASYBLOCK_CLASS_PREFIX from easybuild.tools.filetools import apply_patch, copy_dir, copy_file, det_patched_files, decode_class_name from easybuild.tools.filetools import download_file, extract_file, mkdir, read_file, symlink from easybuild.tools.filetools import which, write_file @@ -698,7 +697,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if pr_target_repo == GITHUB_EASYCONFIGS_REPO: if paths['py_files']: - if any([get_easyblock_class(path) for path in paths['py_files']]): + if any([get_easyblock_class_name(path) for path in paths['py_files']]): # this is not enough, we would need to change build_option('pr_target_repo') pr_target_repo = GITHUB_EASYBLOCKS_REPO raise EasyBuildError("You are submitting easyblock files, " @@ -999,24 +998,17 @@ def find_software_name_for_patch(patch_name, ec_dirs): return soft_name -def get_easyblock_class(path): - """Get easyblock class from file""" +def get_easyblock_class_name(path): + """Make sure file is an easyblock and get easyblock class name""" fn = os.path.basename(path).split('.')[0] mod = imp.load_source(fn, path) clsmembers = inspect.getmembers(mod, inspect.isclass) - is_easyblock = False - for cn in clsmembers: - if cn[0] == 'EasyBlock' or cn[0].startswith(EASYBLOCK_CLASS_PREFIX): - is_easyblock = True - break - if is_easyblock: - classnames = [cl[1].__name__ for cl in clsmembers if cl[1].__module__ == mod.__name__] - if len(classnames) > 1: - return None - else: - return classnames[0] - else: - return None + for cn, co in clsmembers: + if co.__module__ == mod.__name__: + ancestors = inspect.getmro(co) + if ancestors[-2].__name__ == 'EasyBlock': + return cn + return None def copy_easyblocks(paths, target_dir): @@ -1029,7 +1021,7 @@ def copy_easyblocks(paths, target_dir): subdir = os.path.join('easybuild', 'easyblocks') if os.path.exists(os.path.join(target_dir, subdir)): for path in paths: - cn = get_easyblock_class(path) + cn = get_easyblock_class_name(path) if not cn: raise EasyBuildError("Invalid easyblock file") From 68eabad1c51ea7d839a03fdd48ae23131681b114 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Mon, 24 Feb 2020 18:09:28 +0800 Subject: [PATCH 13/30] detect pr-target-repo when using --new-pr for easyblocks --- easybuild/tools/github.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 3a4264a92c..307b741b56 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -698,10 +698,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if pr_target_repo == GITHUB_EASYCONFIGS_REPO: if paths['py_files']: if any([get_easyblock_class_name(path) for path in paths['py_files']]): - # this is not enough, we would need to change build_option('pr_target_repo') pr_target_repo = GITHUB_EASYBLOCKS_REPO - raise EasyBuildError("You are submitting easyblock files, " - "did you forget to specify --pr-target-repo=easybuild-easyblocks?") else: raise EasyBuildError("You are submitting python files that are not easyblocks, " "did you forget to specify --pr-target-repo=easybuild-framework?") @@ -839,7 +836,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ push_branch_to_github(git_repo, target_account, pr_target_repo, pr_branch) - return file_info, deleted_paths, git_repo, pr_branch, diff_stat + return file_info, deleted_paths, git_repo, pr_branch, diff_stat, pr_target_repo def create_remote(git_repo, account, repo, https=False): @@ -1014,6 +1011,7 @@ def get_easyblock_class_name(path): def copy_easyblocks(paths, target_dir): """ Find right location for easyblock file and copy it there""" file_info = { + 'eb_names': [], 'paths_in_repo': [], 'new': [], } @@ -1033,6 +1031,7 @@ def copy_easyblocks(paths, target_dir): target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) full_target_path = os.path.join(target_dir, target_path) + file_info['eb_names'].append(eb_name) file_info['paths_in_repo'].append(full_target_path) file_info['new'].append(not os.path.exists(full_target_path)) copy_file(path, full_target_path, force_in_dry_run=True) @@ -1392,14 +1391,15 @@ def new_branch_github(paths, ecs, commit_msg=None): @only_if_module_is_available('git', pkgname='GitPython') -def new_pr_from_branch(branch_name, title=None, descr=None, pr_metadata=None): +def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, pr_metadata=None): """ Create new pull request from specified branch on GitHub. """ pr_target_account = build_option('pr_target_account') pr_target_branch = build_option('pr_target_branch') - pr_target_repo = build_option('pr_target_repo') + if pr_target_repo is None: + pr_target_repo = build_option('pr_target_repo') # fetch GitHub token (required to perform actions on GitHub) github_user = build_option('github_user') @@ -1537,13 +1537,14 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_metadata=None): pyver.append(dep['version']) if pyver: title += " w/ Python %s" % ' + '.join(sorted(nub(pyver))) + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO: + if file_info['eb_names'] and all(file_info['new']) and not deleted_paths: + plural = 's' if len(file_info['eb_names']) > 1 else '' + title = "new easyblock%s for %s" % (plural, (', '.join(file_info['eb_names']))) - else: - raise EasyBuildError("Don't know how to make a PR title for this PR. " - "Please include a title (use --pr-title)") - else: - raise EasyBuildError("Don't know how to make a PR title for this PR. " - "Please include a title (use --pr-title)") + if title is None: + raise EasyBuildError("Don't know how to make a PR title for this PR. " + "Please include a title (use --pr-title)") full_descr = "(created using `eb --new-pr`)\n" if descr is not None: @@ -1617,9 +1618,10 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): # create new branch in GitHub res = new_branch_github(paths, ecs, commit_msg=commit_msg) - file_info, deleted_paths, _, branch_name, diff_stat = res + file_info, deleted_paths, _, branch_name, diff_stat, pr_target_repo = res - new_pr_from_branch(branch_name, title=title, descr=descr, pr_metadata=(file_info, deleted_paths, diff_stat)) + new_pr_from_branch(branch_name, title=title, descr=descr, pr_target_repo=pr_target_repo, + pr_metadata=(file_info, deleted_paths, diff_stat)) def det_account_branch_for_pr(pr_id, github_user=None): From 484fb7767b7506df9172b969c7a127f8f7d85867 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Tue, 25 Feb 2020 13:38:18 +0800 Subject: [PATCH 14/30] remove duplicate definition of pr_target_repo in new_pr_from_branch --- easybuild/tools/github.py | 1 - 1 file changed, 1 deletion(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 307b741b56..08db5bca09 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -1554,7 +1554,6 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, pr_target_branch = build_option('pr_target_branch') dry_run = build_option('dry_run') or build_option('extended_dry_run') - pr_target_repo = build_option('pr_target_repo') msg = '\n'.join([ '', "Opening pull request%s" % ('', " [DRY RUN]")[dry_run], From 94b0cae3d3407b8a838656bdb698a302aed26f61 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Tue, 25 Feb 2020 13:53:51 +0800 Subject: [PATCH 15/30] also detect pr-target-repo when using --update-pr for easyblocks --- easybuild/tools/github.py | 55 +++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 08db5bca09..178f22a320 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -693,19 +693,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if not any(paths.values()): raise EasyBuildError("No paths specified") - pr_target_repo = build_option('pr_target_repo') - - if pr_target_repo == GITHUB_EASYCONFIGS_REPO: - if paths['py_files']: - if any([get_easyblock_class_name(path) for path in paths['py_files']]): - pr_target_repo = GITHUB_EASYBLOCKS_REPO - else: - raise EasyBuildError("You are submitting python files that are not easyblocks, " - "did you forget to specify --pr-target-repo=easybuild-framework?") - else: - if paths['easyconfigs'] or paths['patch_files']: - raise EasyBuildError("You are submitting easyconfigs and/or patches, " - "shouldn\'t this PR target the easyconfigs repo?") + pr_target_repo = det_pr_target_repo(paths) # initialize repository git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') @@ -1623,7 +1611,7 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): pr_metadata=(file_info, deleted_paths, diff_stat)) -def det_account_branch_for_pr(pr_id, github_user=None): +def det_account_branch_for_pr(pr_id, github_user=None, pr_target_repo=None): """Determine account & branch corresponding to pull request with specified id.""" if github_user is None: @@ -1633,7 +1621,8 @@ def det_account_branch_for_pr(pr_id, github_user=None): raise EasyBuildError("GitHub username (--github-user) must be specified!") pr_target_account = build_option('pr_target_account') - pr_target_repo = build_option('pr_target_repo') + if pr_target_repo is None: + pr_target_repo = build_option('pr_target_repo') pr_data, _ = fetch_pr_data(pr_id, pr_target_account, pr_target_repo, github_user) @@ -1646,6 +1635,29 @@ def det_account_branch_for_pr(pr_id, github_user=None): return account, branch +def det_pr_target_repo(paths): + """Determine pr_target_repo from cagetorized list of files + + :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches) + """ + + pr_target_repo = build_option('pr_target_repo') + + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + if paths['py_files']: + if any([get_easyblock_class_name(path) for path in paths['py_files']]): + pr_target_repo = GITHUB_EASYBLOCKS_REPO + else: + raise EasyBuildError("You are submitting python files that are not easyblocks, " + "did you forget to specify --pr-target-repo=easybuild-framework?") + else: + if paths['easyconfigs'] or paths['patch_files']: + raise EasyBuildError("You are submitting easyconfigs and/or patches, " + "shouldn\'t this PR target the easyconfigs repo?") + + return pr_target_repo + + @only_if_module_is_available('git', pkgname='GitPython') def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None): """ @@ -1665,12 +1677,13 @@ def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None) if github_account is None: github_account = build_option('github_user') or build_option('github_org') - _, _, _, _, diff_stat = _easyconfigs_pr_common(paths, ecs, start_branch=branch_name, pr_branch=branch_name, - start_account=github_account, commit_msg=commit_msg) + _, _, _, _, diff_stat, pr_target_repo = _easyconfigs_pr_common(paths, ecs, start_branch=branch_name, + pr_branch=branch_name, start_account=github_account, + commit_msg=commit_msg) print_msg("Overview of changes:\n%s\n" % diff_stat, log=_log, prefix=False) - full_repo = '%s/%s' % (github_account, build_option('pr_target_repo')) + full_repo = '%s/%s' % (github_account, pr_target_repo) msg = "pushed updated branch '%s' to %s" % (branch_name, full_repo) if build_option('dry_run') or build_option('extended_dry_run'): msg += " [DRY RUN]" @@ -1688,11 +1701,13 @@ def update_pr(pr_id, paths, ecs, commit_msg=None): :param commit_msg: commit message to use """ - github_account, branch_name = det_account_branch_for_pr(pr_id) + pr_target_repo = det_pr_target_repo(paths) + + github_account, branch_name = det_account_branch_for_pr(pr_id, pr_target_repo=pr_target_repo) update_branch(branch_name, paths, ecs, github_account=github_account, commit_msg=commit_msg) - full_repo = '%s/%s' % (build_option('pr_target_account'), build_option('pr_target_repo')) + full_repo = '%s/%s' % (build_option('pr_target_account'), pr_target_repo) msg = "updated https://github.com/%s/pull/%s" % (full_repo, pr_id) if build_option('dry_run') or build_option('extended_dry_run'): msg += " [DRY RUN]" From 0907cfbfd4e2b0aad0fc1affc90bb9fca98af268 Mon Sep 17 00:00:00 2001 From: Miguel Dias Costa Date: Tue, 3 Mar 2020 18:17:51 +0800 Subject: [PATCH 16/30] improve error messages, use eb_name consistently --- easybuild/tools/github.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 178f22a320..5094d379c1 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -1009,14 +1009,13 @@ def copy_easyblocks(paths, target_dir): for path in paths: cn = get_easyblock_class_name(path) if not cn: - raise EasyBuildError("Invalid easyblock file") + raise EasyBuildError("Could not determine easyblock class from file %s" % path) eb_name = remove_unwanted_chars(decode_class_name(cn).replace('-', '_')).lower() if is_generic_easyblock(cn): - target_path = os.path.join(subdir, GENERIC_EB, "%s.%s" % (eb_name.lower(), PYTHON_EXTENSION)) + target_path = os.path.join(subdir, GENERIC_EB, "%s.%s" % (eb_name, PYTHON_EXTENSION)) else: - letter = os.path.basename(path).lower()[0] - target_path = os.path.join(subdir, letter, "%s.%s" % (eb_name, PYTHON_EXTENSION)) + target_path = os.path.join(subdir, eb_name[0], "%s.%s" % (eb_name, PYTHON_EXTENSION)) full_target_path = os.path.join(target_dir, target_path) file_info['eb_names'].append(eb_name) @@ -1025,7 +1024,7 @@ def copy_easyblocks(paths, target_dir): copy_file(path, full_target_path, force_in_dry_run=True) else: - raise EasyBuildError("Subdir easyblocks not found") + raise EasyBuildError("Could not find %s" % os.path.join(target_dir, subdir)) return file_info From d9eda8c7aba442ab7c759fc6c4d236db0f9b8437 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sun, 8 Mar 2020 15:04:36 +0100 Subject: [PATCH 17/30] enhance test for categorize_files_by_type to also cover py_files --- test/framework/easyconfig.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index e9e39768ca..65b5f1b309 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2716,15 +2716,23 @@ def test_categorize_files_by_type(self): self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], 'py_files': []}, categorize_files_by_type([])) - test_ecs_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs',) + test_dir = os.path.dirname(os.path.abspath(__file__)) + test_ecs_dir = os.path.join(test_dir, 'easyconfigs') toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch' toy_patch = os.path.join(os.path.dirname(test_ecs_dir), 'sandbox', 'sources', 'toy', toy_patch_fn) + + easyblocks_dir = os.path.join(test_dir, 'sandbox', 'easybuild', 'easyblocks') + configuremake = os.path.join(easyblocks_dir, 'generic', 'configuremake.py') + toy_easyblock = os.path.join(easyblocks_dir, 't', 'toy.py') + paths = [ 'bzip2-1.0.6.eb', + toy_easyblock, os.path.join(test_ecs_dir, 'test_ecs', 'g', 'gzip', 'gzip-1.4.eb'), toy_patch, 'foo', ':toy-0.0-deps.eb', + configuremake, ] res = categorize_files_by_type(paths) expected = [ @@ -2735,6 +2743,7 @@ def test_categorize_files_by_type(self): self.assertEqual(res['easyconfigs'], expected) self.assertEqual(res['files_to_delete'], ['toy-0.0-deps.eb']) self.assertEqual(res['patch_files'], [toy_patch]) + self.assertEqual(res['py_files'], [toy_easyblock, configuremake]) def test_resolve_template(self): """Test resolve_template function.""" From bdf437f8324bf6d7906e1bfcf069ee84350fbd6b Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sun, 8 Mar 2020 15:21:53 +0100 Subject: [PATCH 18/30] move constant for 'generic' to config.py, avoid constant for .py + order imports alphabetically --- easybuild/framework/easyconfig/easyconfig.py | 5 +++-- easybuild/tools/config.py | 3 +++ easybuild/tools/github.py | 15 ++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index b3e8af1cb8..650d29d5fc 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -58,7 +58,8 @@ from easybuild.framework.easyconfig.parser import EasyConfigParser, fetch_parameters_from_easyconfig from easybuild.framework.easyconfig.templates import TEMPLATE_CONSTANTS, template_constant_dict from easybuild.tools.build_log import EasyBuildError, print_warning, print_msg -from easybuild.tools.config import LOCAL_VAR_NAMING_CHECK_ERROR, LOCAL_VAR_NAMING_CHECK_LOG, LOCAL_VAR_NAMING_CHECK_WARN +from easybuild.tools.config import GENERIC_EASYBLOCK_PKG, LOCAL_VAR_NAMING_CHECK_ERROR, LOCAL_VAR_NAMING_CHECK_LOG +from easybuild.tools.config import LOCAL_VAR_NAMING_CHECK_WARN from easybuild.tools.config import Singleton, build_option, get_module_naming_scheme from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, copy_file, decode_class_name, encode_class_name from easybuild.tools.filetools import find_backup_name_candidate, find_easyconfigs, read_file, write_file @@ -1697,7 +1698,7 @@ def get_module_path(name, generic=None, decode=True): modpath = ['easybuild', 'easyblocks'] if generic: - modpath.append('generic') + modpath.append(GENERIC_EASYBLOCK_PKG) return '.'.join(modpath + [module_name]) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index ab98bcad6d..d4899a7fe3 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -111,6 +111,9 @@ FORCE_DOWNLOAD_CHOICES = [FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES] DEFAULT_FORCE_DOWNLOAD = FORCE_DOWNLOAD_SOURCES +# package name for generic easyblocks +GENERIC_EASYBLOCK_PKG = 'generic' + # general module class GENERAL_CLASS = 'all' diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 5094d379c1..520d796627 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -30,11 +30,11 @@ :author: Toon Willems (Ghent University) """ import base64 -import imp -import inspect import copy import getpass import glob +import imp +import inspect import os import random import re @@ -51,7 +51,7 @@ from easybuild.framework.easyconfig.easyconfig import is_generic_easyblock, process_easyconfig 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.config import GENERIC_EASYBLOCK_PKG, build_option from easybuild.tools.filetools import apply_patch, copy_dir, copy_file, det_patched_files, decode_class_name from easybuild.tools.filetools import download_file, extract_file, mkdir, read_file, symlink from easybuild.tools.filetools import which, write_file @@ -84,7 +84,6 @@ _log.warning("Failed to import 'git' Python module: %s", err) -GENERIC_EB = 'generic' GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' GITHUB_DIR_TYPE = u'dir' @@ -109,7 +108,6 @@ HTTP_STATUS_CREATED = 201 HTTP_STATUS_NO_CONTENT = 204 KEYRING_GITHUB_TOKEN = 'github_token' -PYTHON_EXTENSION = 'py' URL_SEPARATOR = '/' VALID_CLOSE_PR_REASONS = { @@ -1012,10 +1010,13 @@ def copy_easyblocks(paths, target_dir): raise EasyBuildError("Could not determine easyblock class from file %s" % path) eb_name = remove_unwanted_chars(decode_class_name(cn).replace('-', '_')).lower() + if is_generic_easyblock(cn): - target_path = os.path.join(subdir, GENERIC_EB, "%s.%s" % (eb_name, PYTHON_EXTENSION)) + pkgdir = GENERIC_EASYBLOCK_PKG else: - target_path = os.path.join(subdir, eb_name[0], "%s.%s" % (eb_name, PYTHON_EXTENSION)) + pkgdir = eb_name[0] + + target_path = os.path.join(subdir, pkgdir, eb_name + '.py') full_target_path = os.path.join(target_dir, target_path) file_info['eb_names'].append(eb_name) From 9569fc31b2b595d4153ef9a63c6e11111c160e0e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sun, 5 Apr 2020 18:20:09 +0200 Subject: [PATCH 19/30] change implementation of det_pr_target_repo to always honor --pr-target-repo, make it return None if guessing didn't work + use None as default for --pr-target-repo --- easybuild/tools/github.py | 60 ++++++++++++++++++++++++++------------ easybuild/tools/options.py | 3 +- test/framework/github.py | 53 +++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 20 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 8164f25c7d..61039b60fa 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -692,6 +692,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ raise EasyBuildError("No paths specified") pr_target_repo = det_pr_target_repo(paths) + if pr_target_repo is None: + raise EasyBuildError("Failed to determine target repository, please specify it via --pr-target-repo!") # initialize repository git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') @@ -1232,7 +1234,7 @@ def close_pr(pr, motivation_msg=None): raise EasyBuildError("GitHub user must be specified to use --close-pr") pr_target_account = build_option('pr_target_account') - pr_target_repo = build_option('pr_target_repo') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO pr_data, _ = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user, full=True) @@ -1305,7 +1307,7 @@ def list_prs(params, per_page=GITHUB_MAX_PER_PAGE, github_user=None): print_msg("Listing PRs with parameters: %s" % ', '.join(k + '=' + str(parameters[k]) for k in sorted(parameters))) pr_target_account = build_option('pr_target_account') - pr_target_repo = build_option('pr_target_repo') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO pr_data, _ = fetch_pr_data(None, pr_target_account, pr_target_repo, github_user, **parameters) @@ -1325,7 +1327,7 @@ def merge_pr(pr): raise EasyBuildError("GitHub user must be specified to use --merge-pr") pr_target_account = build_option('pr_target_account') - pr_target_repo = build_option('pr_target_repo') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO pr_data, pr_url = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user, full=True) @@ -1388,7 +1390,7 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, pr_target_account = build_option('pr_target_account') pr_target_branch = build_option('pr_target_branch') if pr_target_repo is None: - pr_target_repo = build_option('pr_target_repo') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO # fetch GitHub token (required to perform actions on GitHub) github_user = build_option('github_user') @@ -1623,7 +1625,7 @@ def det_account_branch_for_pr(pr_id, github_user=None, pr_target_repo=None): pr_target_account = build_option('pr_target_account') if pr_target_repo is None: - pr_target_repo = build_option('pr_target_repo') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO pr_data, _ = fetch_pr_data(pr_id, pr_target_account, pr_target_repo, github_user) @@ -1637,24 +1639,42 @@ def det_account_branch_for_pr(pr_id, github_user=None, pr_target_repo=None): def det_pr_target_repo(paths): - """Determine pr_target_repo from cagetorized list of files + """Determine target repository for pull request from given cagetorized list of files - :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches) + :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches, .py files) """ - pr_target_repo = build_option('pr_target_repo') - if pr_target_repo == GITHUB_EASYCONFIGS_REPO: - if paths['py_files']: - if any([get_easyblock_class_name(path) for path in paths['py_files']]): + # determine target repository for PR based on which files are provided + # (see categorize_files_by_type function) + if pr_target_repo is None: + + _log.info("Trying to derive target repository based on specified files...") + + easyconfigs, files_to_delete, patch_files, py_files = [paths[key] for key in sorted(paths.keys())] + + # Python files provided, and no easyconfig files or patches + if py_files and not (easyconfigs or patch_files): + + _log.info("Only Python files provided, no easyconfig files or patches...") + + # if all Python files are easyblocks, target repo should be easyblocks; + # otherwise, target repo is assumed to be framework + if all([get_easyblock_class_name(path) for path in py_files]): pr_target_repo = GITHUB_EASYBLOCKS_REPO + _log.info("All Python files are easyblocks, target repository is assumed to be %s", pr_target_repo) else: - raise EasyBuildError("You are submitting python files that are not easyblocks, " - "did you forget to specify --pr-target-repo=easybuild-framework?") - else: - if paths['easyconfigs'] or paths['patch_files']: - raise EasyBuildError("You are submitting easyconfigs and/or patches, " - "shouldn\'t this PR target the easyconfigs repo?") + pr_target_repo = GITHUB_FRAMEWORK_REPO + _log.info("Not all Python files are easyblocks, target repository is assumed to be %s", pr_target_repo) + + # if no Python files are provided, only easyconfigs & patches, or if files to delete are .eb files, + # then target repo is assumed to be easyconfigs + elif easyconfigs or patch_files or (files_to_delete and all(x.endswith('.eb') for x in files_to_delete)): + pr_target_repo = GITHUB_EASYCONFIGS_REPO + _log.info("Only easyconfig and patch files found, target repository is assumed to be %s", pr_target_repo) + + else: + _log.info("No Python files, easyconfigs or patches found, can't derive target repository...") return pr_target_repo @@ -1703,6 +1723,8 @@ def update_pr(pr_id, paths, ecs, commit_msg=None): """ pr_target_repo = det_pr_target_repo(paths) + if pr_target_repo is None: + raise EasyBuildError("Failed to determine target repository, please specify it via --pr-target-repo!") github_account, branch_name = det_account_branch_for_pr(pr_id, pr_target_repo=pr_target_repo) @@ -2163,7 +2185,7 @@ def sync_pr_with_develop(pr_id): raise EasyBuildError("GitHub user must be specified to use --sync-pr-with-develop") target_account = build_option('pr_target_account') - target_repo = build_option('pr_target_repo') + target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO pr_account, pr_branch = det_account_branch_for_pr(pr_id) @@ -2186,7 +2208,7 @@ def sync_branch_with_develop(branch_name): raise EasyBuildError("GitHub user must be specified to use --sync-branch-with-develop") target_account = build_option('pr_target_account') - target_repo = build_option('pr_target_repo') + target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO # initialize repository git_working_dir = tempfile.mkdtemp(prefix='git-working-dir') diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 513bf715e6..0532f2846b 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -612,7 +612,8 @@ def github_options(self): 'pr-descr': ("Description for new pull request created with --new-pr", str, 'store', None), 'pr-target-account': ("Target account for new PRs", str, 'store', GITHUB_EB_MAIN), 'pr-target-branch': ("Target branch for new PRs", str, 'store', DEFAULT_BRANCH), - 'pr-target-repo': ("Target repository for new/updating PRs", str, 'store', GITHUB_EASYCONFIGS_REPO), + 'pr-target-repo': ("Target repository for new/updating PRs (default: auto-detect based on provided files)", + str, 'store', None), 'pr-title': ("Title for new pull request created with --new-pr", str, 'store', None), 'preview-pr': ("Preview a new pull request", None, 'store_true', False), 'sync-branch-with-develop': ("Sync branch with current 'develop' branch", str, 'store', None), diff --git a/test/framework/github.py b/test/framework/github.py index 4b4c68c31c..5e9837d6d2 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -37,6 +37,7 @@ from unittest import TextTestRunner from easybuild.base.rest import RestClient +from easybuild.framework.easyconfig.tools import categorize_files_by_type from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import module_classes from easybuild.tools.configobj import ConfigObj @@ -666,6 +667,58 @@ def test_det_account_branch_for_pr(self): self.assertEqual(account, 'migueldiascosta') self.assertEqual(branch, 'fix_inject_checksums') + def test_det_pr_target_repo(self): + """Test det_pr_target_repo.""" + + # no files => return default target repo (None) + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([])), None) + + # easyconfigs/patches (incl. files to delete) => easyconfigs repo + # this is solely based on filenames, actual files are not opened + test_cases = [ + ['toy.eb'], + ['toy.patch'], + ['toy.eb', 'toy.patch'], + [':toy.eb'], # deleting toy.eb + ['one.eb', 'two.eb'], + ['one.eb', 'two.eb', 'toy.patch', ':todelete.eb'], + ] + for test_case in test_cases: + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(test_case)), 'easybuild-easyconfigs') + + # if only Python files are involved, result is easyblocks or framework repo; + # all Python files are easyblocks => easyblocks repo, otherwise => framework repo; + # files are opened and inspected here to discriminate between easyblocks & other Python files, so must exist! + github_py = os.path.abspath(__file__) + testdir = os.path.dirname(github_py) + + configuremake = os.path.join(testdir, 'sandbox', 'easybuild', 'easyblocks', 'generic', 'configuremake.py') + self.assertTrue(os.path.exists(configuremake)) + toy_eb = os.path.join(testdir, 'sandbox', 'easybuild', 'easyblocks', 't', 'toy.py') + self.assertTrue(os.path.exists(toy_eb)) + + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([github_py])), 'easybuild-framework') + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([configuremake])), 'easybuild-easyblocks') + py_files = [github_py, configuremake] + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(py_files)), 'easybuild-framework') + py_files[0] = toy_eb + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(py_files)), 'easybuild-easyblocks') + py_files.append(github_py) + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(py_files)), 'easybuild-framework') + + # as soon as an easyconfig file or patch files is involved => result is easybuild-easyconfigs repo + for fn in ['toy.eb', 'toy.patch']: + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(py_files + [fn])), 'easybuild-easyconfigs') + + # if --pr-target-repo is specified, we always get this value (no guessing anymore) + init_config(build_options={'pr_target_repo': 'thisisjustatest'}) + + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([])), 'thisisjustatest') + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(['toy.eb', 'toy.patch'])), 'thisisjustatest') + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type(py_files)), 'thisisjustatest') + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([configuremake])), 'thisisjustatest') + self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([toy_eb])), 'thisisjustatest') + def test_push_branch_to_github(self): """Test push_branch_to_github.""" From 653cae3d4ce40bbc9662d173a3e57614d2041c1d Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 6 Apr 2020 12:07:44 +0200 Subject: [PATCH 20/30] fix get_easyblock_class_name for easyblocks that derive from ExtensionEasyblock (+ add dedicated test) --- easybuild/tools/github.py | 2 +- test/framework/github.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 61039b60fa..cbed9f2cab 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -991,7 +991,7 @@ def get_easyblock_class_name(path): for cn, co in clsmembers: if co.__module__ == mod.__name__: ancestors = inspect.getmro(co) - if ancestors[-2].__name__ == 'EasyBlock': + if any(a.__name__ == 'EasyBlock' for a in ancestors): return cn return None diff --git a/test/framework/github.py b/test/framework/github.py index 5e9837d6d2..311ef988b9 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -748,6 +748,21 @@ def test_push_branch_to_github(self): regex = re.compile(pattern) self.assertTrue(regex.match(stdout.strip()), "Pattern '%s' doesn't match: %s" % (regex.pattern, stdout)) + def test_get_easyblock_class_name(self): + """Test for get_easyblock_class_name function.""" + + topdir = os.path.dirname(os.path.abspath(__file__)) + test_ebs = os.path.join(topdir, 'sandbox', 'easybuild', 'easyblocks') + + configuremake = os.path.join(test_ebs, 'generic', 'configuremake.py') + self.assertEqual(gh.get_easyblock_class_name(configuremake), 'ConfigureMake') + + gcc_eb = os.path.join(test_ebs, 'g', 'gcc.py') + self.assertEqual(gh.get_easyblock_class_name(gcc_eb), 'EB_GCC') + + toy_eb = os.path.join(test_ebs, 't', 'toy.py') + self.assertEqual(gh.get_easyblock_class_name(toy_eb), 'EB_toy') + def suite(): """ returns all the testcases in this module """ From 0e2f33d3c7d2d0ff83e3562b893eb02afa6c0c0a Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 6 Apr 2020 14:43:16 +0200 Subject: [PATCH 21/30] use test_module_naming_scheme.py rather than a dedicated new file a_test.py in test_new_branch_github --- test/framework/options.py | 16 ++++++++++------ test/framework/sandbox/a_test.py | 3 --- 2 files changed, 10 insertions(+), 9 deletions(-) delete mode 100644 test/framework/sandbox/a_test.py diff --git a/test/framework/options.py b/test/framework/options.py index 3ef2ed2cb9..ad5ab94ad9 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -2970,7 +2970,6 @@ def test_new_branch_github(self): args = [ '--new-branch-github', - '--pr-target-repo=easybuild-easyblocks', '--github-user=%s' % GITHUB_TEST_ACCOUNT, toy_eb, '--pr-title="add easyblock for toy"', @@ -2986,15 +2985,20 @@ def test_new_branch_github(self): ] self._assert_regexs(regexs, txt) - # test framework - test_ebs = os.path.join(topdir, 'sandbox') - toy_py = os.path.join(test_ebs, 'a_test.py') + # test framework with tweaked copy of test_module_naming_scheme.py + test_mns_py = os.path.join(topdir, 'sandbox', 'easybuild', 'tools', 'module_naming_scheme', + 'test_module_naming_scheme.py') + target_dir = os.path.join(self.test_prefix, 'easybuild-framework', 'test', 'framework', 'sandbox', + 'easybuild', 'tools', 'module_naming_scheme') + mkdir(target_dir, parents=True) + copy_file(test_mns_py, target_dir) + test_mns_py = os.path.join(target_dir, os.path.basename(test_mns_py)) + write_file(test_mns_py, '\n\n', append=True) args = [ '--new-branch-github', - '--pr-target-repo=easybuild-framework', '--github-user=%s' % GITHUB_TEST_ACCOUNT, - toy_py, + test_mns_py, '--pr-commit-msg="a test"', '-D', ] diff --git a/test/framework/sandbox/a_test.py b/test/framework/sandbox/a_test.py deleted file mode 100644 index 6d8a26f090..0000000000 --- a/test/framework/sandbox/a_test.py +++ /dev/null @@ -1,3 +0,0 @@ -""" -Used for test_new_branch_github -""" From bf4ebe1baffe8aa6954704ee065b9a4bea77bf53 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 6 Apr 2020 17:06:14 +0200 Subject: [PATCH 22/30] move get_easyblock_class_name, copy_easyblocks and copy_framework_files to easybuild/tools/filetools.py, doesn't really belong in github.py + add tests for copy_easyblocks and copy_framework_files --- easybuild/framework/easyconfig/easyconfig.py | 9 +- easybuild/tools/filetools.py | 97 +++++++++++++- easybuild/tools/github.py | 95 +------------- test/framework/easyconfig.py | 3 + test/framework/filetools.py | 127 +++++++++++++++++++ test/framework/github.py | 15 --- 6 files changed, 236 insertions(+), 110 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 2a2fb3103c..4b04c73aa5 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -45,6 +45,7 @@ import re from distutils.version import LooseVersion +import easybuild.tools.filetools as filetools from easybuild.base import fancylogger from easybuild.framework.easyconfig import MANDATORY from easybuild.framework.easyconfig.constants import EXTERNAL_MODULE_MARKER @@ -61,7 +62,7 @@ from easybuild.tools.config import GENERIC_EASYBLOCK_PKG, LOCAL_VAR_NAMING_CHECK_ERROR, LOCAL_VAR_NAMING_CHECK_LOG from easybuild.tools.config import LOCAL_VAR_NAMING_CHECK_WARN from easybuild.tools.config import Singleton, build_option, get_module_naming_scheme -from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, copy_file, decode_class_name, encode_class_name +from easybuild.tools.filetools import copy_file, decode_class_name, encode_class_name from easybuild.tools.filetools import find_backup_name_candidate, find_easyconfigs, read_file, write_file from easybuild.tools.hooks import PARSE, load_hooks, run_hook from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX @@ -1682,8 +1683,8 @@ def get_easyblock_class(easyblock, name=None, error_on_failed_import=True, error def is_generic_easyblock(easyblock): """Return whether specified easyblock name is a generic easyblock or not.""" - - return easyblock and not easyblock.startswith(EASYBLOCK_CLASS_PREFIX) + _log.deprecated("is_generic_easyblock function was moved to easybuild.tools.filetools", '5.0') + return filetools.is_generic_easyblock(easyblock) def get_module_path(name, generic=None, decode=True): @@ -1698,7 +1699,7 @@ def get_module_path(name, generic=None, decode=True): return None if generic is None: - generic = is_generic_easyblock(name) + generic = filetools.is_generic_easyblock(name) # example: 'EB_VSC_minus_tools' should result in 'vsc_tools' if decode: diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 3cb7979631..cd64bfda8e 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -43,6 +43,8 @@ import fileinput import glob import hashlib +import imp +import inspect import os import re import shutil @@ -57,9 +59,9 @@ 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 -from easybuild.tools.config import build_option +from easybuild.tools.config import GENERIC_EASYBLOCK_PKG, build_option from easybuild.tools.py2vs3 import std_urllib, string_type -from easybuild.tools.utilities import nub +from easybuild.tools.utilities import nub, remove_unwanted_chars try: import requests @@ -2009,3 +2011,94 @@ def install_fake_vsc(): sys.path.insert(0, fake_vsc_path) return fake_vsc_path + + +def get_easyblock_class_name(path): + """Make sure file is an easyblock and get easyblock class name""" + fn = os.path.basename(path).split('.')[0] + mod = imp.load_source(fn, path) + clsmembers = inspect.getmembers(mod, inspect.isclass) + for cn, co in clsmembers: + if co.__module__ == mod.__name__: + ancestors = inspect.getmro(co) + if any(a.__name__ == 'EasyBlock' for a in ancestors): + return cn + return None + + +def is_generic_easyblock(easyblock): + """Return whether specified easyblock name is a generic easyblock or not.""" + + return easyblock and not easyblock.startswith(EASYBLOCK_CLASS_PREFIX) + + +def copy_easyblocks(paths, target_dir): + """ Find right location for easyblock file and copy it there""" + file_info = { + 'eb_names': [], + 'paths_in_repo': [], + 'new': [], + } + + subdir = os.path.join('easybuild', 'easyblocks') + if os.path.exists(os.path.join(target_dir, subdir)): + for path in paths: + cn = get_easyblock_class_name(path) + if not cn: + raise EasyBuildError("Could not determine easyblock class from file %s" % path) + + eb_name = remove_unwanted_chars(decode_class_name(cn).replace('-', '_')).lower() + + if is_generic_easyblock(cn): + pkgdir = GENERIC_EASYBLOCK_PKG + else: + pkgdir = eb_name[0] + + target_path = os.path.join(subdir, pkgdir, eb_name + '.py') + + full_target_path = os.path.join(target_dir, target_path) + file_info['eb_names'].append(eb_name) + file_info['paths_in_repo'].append(full_target_path) + file_info['new'].append(not os.path.exists(full_target_path)) + copy_file(path, full_target_path, force_in_dry_run=True) + + else: + raise EasyBuildError("Could not find %s subdir in %s", subdir, target_dir) + + return file_info + + +def copy_framework_files(paths, target_dir): + """ Find right location for framework file and copy it there""" + file_info = { + 'paths_in_repo': [], + 'new': [], + } + + paths = [os.path.abspath(path) for path in paths] + + framework_topdir = 'easybuild-framework' + + for path in paths: + target_path = None + dirnames = os.path.dirname(path).split(os.path.sep) + + print('[copy_framework_files] %s' % dirnames) + if framework_topdir in dirnames: + ind = dirnames.index(framework_topdir) + 1 + print(ind) + subdirs = dirnames[ind:] + print(subdirs) + parent_dir = os.path.join(*subdirs) if subdirs else '' + target_path = os.path.join(target_dir, parent_dir, os.path.basename(path)) + else: + raise EasyBuildError("Specified path '%s' does not include a '%s' directory!", path, framework_topdir) + + if target_path: + file_info['paths_in_repo'].append(target_path) + file_info['new'].append(not os.path.exists(target_path)) + copy_file(path, target_path) + else: + raise EasyBuildError("Couldn't find parent folder of updated file: %s", path) + + return file_info diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index cbed9f2cab..b9c92e0d8b 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -33,8 +33,6 @@ import copy import getpass import glob -import imp -import inspect import os import random import re @@ -48,16 +46,16 @@ from easybuild.base import fancylogger from easybuild.framework.easyconfig.easyconfig import EASYCONFIGS_ARCHIVE_DIR from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs, copy_patch_files, det_file_info -from easybuild.framework.easyconfig.easyconfig import is_generic_easyblock, process_easyconfig +from easybuild.framework.easyconfig.easyconfig import process_easyconfig from easybuild.framework.easyconfig.parser import EasyConfigParser from easybuild.tools.build_log import EasyBuildError, print_msg, print_warning -from easybuild.tools.config import GENERIC_EASYBLOCK_PKG, build_option -from easybuild.tools.filetools import apply_patch, copy_dir, copy_file, det_patched_files, decode_class_name -from easybuild.tools.filetools import download_file, extract_file, mkdir, read_file, symlink -from easybuild.tools.filetools import which, write_file +from easybuild.tools.config import build_option +from easybuild.tools.filetools import apply_patch, copy_dir, copy_easyblocks, copy_file, copy_framework_files +from easybuild.tools.filetools import det_patched_files, decode_class_name, 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 from easybuild.tools.systemtools import UNKNOWN, get_tool_version -from easybuild.tools.utilities import nub, only_if_module_is_available, remove_unwanted_chars +from easybuild.tools.utilities import nub, only_if_module_is_available _log = fancylogger.getLogger('github', fname=False) @@ -983,87 +981,6 @@ def find_software_name_for_patch(patch_name, ec_dirs): return soft_name -def get_easyblock_class_name(path): - """Make sure file is an easyblock and get easyblock class name""" - fn = os.path.basename(path).split('.')[0] - mod = imp.load_source(fn, path) - clsmembers = inspect.getmembers(mod, inspect.isclass) - for cn, co in clsmembers: - if co.__module__ == mod.__name__: - ancestors = inspect.getmro(co) - if any(a.__name__ == 'EasyBlock' for a in ancestors): - return cn - return None - - -def copy_easyblocks(paths, target_dir): - """ Find right location for easyblock file and copy it there""" - file_info = { - 'eb_names': [], - 'paths_in_repo': [], - 'new': [], - } - - subdir = os.path.join('easybuild', 'easyblocks') - if os.path.exists(os.path.join(target_dir, subdir)): - for path in paths: - cn = get_easyblock_class_name(path) - if not cn: - raise EasyBuildError("Could not determine easyblock class from file %s" % path) - - eb_name = remove_unwanted_chars(decode_class_name(cn).replace('-', '_')).lower() - - if is_generic_easyblock(cn): - pkgdir = GENERIC_EASYBLOCK_PKG - else: - pkgdir = eb_name[0] - - target_path = os.path.join(subdir, pkgdir, eb_name + '.py') - - full_target_path = os.path.join(target_dir, target_path) - file_info['eb_names'].append(eb_name) - file_info['paths_in_repo'].append(full_target_path) - file_info['new'].append(not os.path.exists(full_target_path)) - copy_file(path, full_target_path, force_in_dry_run=True) - - else: - raise EasyBuildError("Could not find %s" % os.path.join(target_dir, subdir)) - - return file_info - - -def copy_framework_files(paths, target_dir): - """ Find right location for framework file and copy it there""" - file_info = { - 'paths_in_repo': [], - 'new': [], - } - - paths = [os.path.abspath(path) for path in paths] - - target_path = None - for path in paths: - dirnames = os.path.dirname(path).split(os.path.sep) - - if 'easybuild-framework' in dirnames: - ind = dirnames.index('easybuild-framework') + 1 - parent_dir = os.path.join(*dirnames[ind:]) - - if os.path.exists(os.path.join(target_dir, parent_dir)): - target_path = os.path.join(target_dir, parent_dir) - - if target_path is None: - raise EasyBuildError("Couldn't find parent folder of updated file: %s" % path) - - full_target_path = os.path.join(target_path, os.path.basename(path)) - - file_info['paths_in_repo'].append(full_target_path) - file_info['new'].append(not os.path.exists(full_target_path)) - copy_file(path, full_target_path) - - return file_info - - def check_pr_eligible_to_merge(pr_data): """ Check whether PR is eligible for merging. diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 0545010e68..c23101359c 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2983,6 +2983,9 @@ def test_get_paths_for(self): def test_is_generic_easyblock(self): """Test for is_generic_easyblock function.""" + # is_generic_easyblock in easyconfig.py is deprecated, moved to filetools.py + self.allow_deprecated_behaviour() + for name in ['Binary', 'ConfigureMake', 'CMakeMake', 'PythonPackage', 'JAR']: self.assertTrue(is_generic_easyblock(name)) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index a96ee8a6a7..416f62235e 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2097,6 +2097,133 @@ def test_fake_vsc(self): from test_fake_vsc import pkgutil self.assertTrue(pkgutil.__file__.endswith('/test_fake_vsc/pkgutil.py')) + def test_is_generic_easyblock(self): + """Test for is_generic_easyblock function.""" + + for name in ['Binary', 'ConfigureMake', 'CMakeMake', 'PythonPackage', 'JAR']: + self.assertTrue(ft.is_generic_easyblock(name)) + + for name in ['EB_bzip2', 'EB_DL_underscore_POLY_underscore_Classic', 'EB_GCC', 'EB_WRF_minus_Fire']: + self.assertFalse(ft.is_generic_easyblock(name)) + + def test_get_easyblock_class_name(self): + """Test for get_easyblock_class_name function.""" + + topdir = os.path.dirname(os.path.abspath(__file__)) + test_ebs = os.path.join(topdir, 'sandbox', 'easybuild', 'easyblocks') + + configuremake = os.path.join(test_ebs, 'generic', 'configuremake.py') + self.assertEqual(ft.get_easyblock_class_name(configuremake), 'ConfigureMake') + + gcc_eb = os.path.join(test_ebs, 'g', 'gcc.py') + self.assertEqual(ft.get_easyblock_class_name(gcc_eb), 'EB_GCC') + + toy_eb = os.path.join(test_ebs, 't', 'toy.py') + self.assertEqual(ft.get_easyblock_class_name(toy_eb), 'EB_toy') + + def test_copy_easyblocks(self): + """Test for copy_easyblocks function.""" + + topdir = os.path.dirname(os.path.abspath(__file__)) + test_ebs = os.path.join(topdir, 'sandbox', 'easybuild', 'easyblocks') + + # easybuild/easyblocks subdirectory must exist in target directory + error_pattern = "Could not find easybuild/easyblocks subdir in .*" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.copy_easyblocks, [], self.test_prefix) + + easyblocks_dir = os.path.join(self.test_prefix, 'easybuild', 'easyblocks') + + # passing empty list works fine + ft.mkdir(easyblocks_dir, parents=True) + res = ft.copy_easyblocks([], self.test_prefix) + self.assertEqual(os.listdir(easyblocks_dir), []) + self.assertEqual(res, {'eb_names': [], 'new': [], 'paths_in_repo': []}) + + # check with different types of easyblocks + configuremake = os.path.join(test_ebs, 'generic', 'configuremake.py') + gcc_eb = os.path.join(test_ebs, 'g', 'gcc.py') + toy_eb = os.path.join(test_ebs, 't', 'toy.py') + test_ebs = [gcc_eb, configuremake, toy_eb] + + # copy them straight into tmpdir first, to check whether correct subdir is derived correctly + ft.copy_files(test_ebs, self.test_prefix) + + # touch empty toy.py easyblock, to check whether 'new' aspect is determined correctly + ft.write_file(os.path.join(easyblocks_dir, 't', 'toy.py'), '') + + # check whether easyblocks were copied as expected, and returned dict is correct + test_ebs = [os.path.join(self.test_prefix, os.path.basename(e)) for e in test_ebs] + res = ft.copy_easyblocks(test_ebs, self.test_prefix) + + self.assertEqual(sorted(res.keys()), ['eb_names', 'new', 'paths_in_repo']) + self.assertEqual(res['eb_names'], ['gcc', 'configuremake', 'toy']) + self.assertEqual(res['new'], [True, True, False]) # toy.py is not new + + self.assertEqual(sorted(os.listdir(easyblocks_dir)), ['g', 'generic', 't']) + + g_dir = os.path.join(easyblocks_dir, 'g') + self.assertEqual(sorted(os.listdir(g_dir)), ['gcc.py']) + copied_gcc_eb = os.path.join(g_dir, 'gcc.py') + self.assertEqual(ft.read_file(copied_gcc_eb), ft.read_file(gcc_eb)) + self.assertTrue(os.path.samefile(res['paths_in_repo'][0], copied_gcc_eb)) + + gen_dir = os.path.join(easyblocks_dir, 'generic') + self.assertEqual(sorted(os.listdir(gen_dir)), ['configuremake.py']) + copied_configuremake = os.path.join(gen_dir, 'configuremake.py') + self.assertEqual(ft.read_file(copied_configuremake), ft.read_file(configuremake)) + self.assertTrue(os.path.samefile(res['paths_in_repo'][1], copied_configuremake)) + + t_dir = os.path.join(easyblocks_dir, 't') + self.assertEqual(sorted(os.listdir(t_dir)), ['toy.py']) + copied_toy_eb = os.path.join(t_dir, 'toy.py') + self.assertEqual(ft.read_file(copied_toy_eb), ft.read_file(toy_eb)) + self.assertTrue(os.path.samefile(res['paths_in_repo'][2], copied_toy_eb)) + + def test_copy_framework_files(self): + """Test for copy_framework_files function.""" + + target_dir = os.path.join(self.test_prefix, 'target') + ft.mkdir(target_dir) + + res = ft.copy_framework_files([], target_dir) + + self.assertEqual(os.listdir(target_dir), []) + self.assertEqual(res, {'paths_in_repo': [], 'new': []}) + + foo_py = os.path.join(self.test_prefix, 'foo.py') + ft.write_file(foo_py, '') + + error_pattern = "Specified path '.*/foo.py' does not include a 'easybuild-framework' directory!" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.copy_framework_files, [foo_py], self.test_prefix) + + # create empty test/framework/modules.py, to check whether 'new' is set correctly in result + ft.write_file(os.path.join(target_dir, 'test', 'framework', 'modules.py'), '') + + topdir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + test_files = [ + os.path.join('easybuild', 'tools', 'filetools.py'), + os.path.join('test', 'framework', 'modules.py'), + os.path.join('test', 'framework', 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz'), + os.path.join('setup.py'), + ] + test_paths = [os.path.join(topdir, f) for f in test_files] + res = ft.copy_framework_files(test_paths, target_dir) + + self.assertEqual(sorted(os.listdir(target_dir)), ['easybuild', 'setup.py', 'test']) + + self.assertEqual(sorted(res.keys()), ['new', 'paths_in_repo']) + + for idx, test_file in enumerate(test_files): + orig_path = os.path.join(topdir, test_file) + copied_path = os.path.join(target_dir, test_file) + + self.assertTrue(os.path.exists(copied_path)) + self.assertEqual(ft.read_file(orig_path), ft.read_file(copied_path)) + + self.assertTrue(os.path.samefile(copied_path, res['paths_in_repo'][idx])) + + self.assertEqual(res['new'], [True, False, True, True]) # test/framework/moduels.py is not new + def suite(): """ returns all the testcases in this module """ diff --git a/test/framework/github.py b/test/framework/github.py index 311ef988b9..5e9837d6d2 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -748,21 +748,6 @@ def test_push_branch_to_github(self): regex = re.compile(pattern) self.assertTrue(regex.match(stdout.strip()), "Pattern '%s' doesn't match: %s" % (regex.pattern, stdout)) - def test_get_easyblock_class_name(self): - """Test for get_easyblock_class_name function.""" - - topdir = os.path.dirname(os.path.abspath(__file__)) - test_ebs = os.path.join(topdir, 'sandbox', 'easybuild', 'easyblocks') - - configuremake = os.path.join(test_ebs, 'generic', 'configuremake.py') - self.assertEqual(gh.get_easyblock_class_name(configuremake), 'ConfigureMake') - - gcc_eb = os.path.join(test_ebs, 'g', 'gcc.py') - self.assertEqual(gh.get_easyblock_class_name(gcc_eb), 'EB_GCC') - - toy_eb = os.path.join(test_ebs, 't', 'toy.py') - self.assertEqual(gh.get_easyblock_class_name(toy_eb), 'EB_toy') - def suite(): """ returns all the testcases in this module """ From ad6e6f8cf997b89c2908c9f73f558339fd4d1aeb Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 08:16:47 +0200 Subject: [PATCH 23/30] fix composing of subdirectory of easybuild-framework/ in copy_framework_files --- easybuild/tools/filetools.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index a02ecba9c5..5d179a04cb 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2085,12 +2085,12 @@ def copy_framework_files(paths, target_dir): target_path = None dirnames = os.path.dirname(path).split(os.path.sep) - print('[copy_framework_files] %s' % dirnames) if framework_topdir in dirnames: - ind = dirnames.index(framework_topdir) + 1 - print(ind) - subdirs = dirnames[ind:] - print(subdirs) + # construct subdirectory by grabbing last entry in dirnames until we hit 'easybuild-framework' dir + subdirs = [] + while(dirnames[-1] != framework_topdir): + subdirs.insert(0, dirnames.pop()) + parent_dir = os.path.join(*subdirs) if subdirs else '' target_path = os.path.join(target_dir, parent_dir, os.path.basename(path)) else: From f994a831450a88046be870fb38da09b469e8a0e8 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 09:14:33 +0200 Subject: [PATCH 24/30] silence deprecation warnings for is_generic_easyblock in easyconfig.py tests --- test/framework/easyconfig.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index c2e26ce0f7..3b720f687c 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -3002,12 +3002,16 @@ def test_is_generic_easyblock(self): # is_generic_easyblock in easyconfig.py is deprecated, moved to filetools.py self.allow_deprecated_behaviour() + self.mock_stderr(True) + for name in ['Binary', 'ConfigureMake', 'CMakeMake', 'PythonPackage', 'JAR']: self.assertTrue(is_generic_easyblock(name)) for name in ['EB_bzip2', 'EB_DL_underscore_POLY_underscore_Classic', 'EB_GCC', 'EB_WRF_minus_Fire']: self.assertFalse(is_generic_easyblock(name)) + self.mock_stderr(False) + def test_get_module_path(self): """Test get_module_path function.""" self.assertEqual(get_module_path('EB_bzip2', generic=False), 'easybuild.easyblocks.bzip2') From 9e480601393341efbb6b472c9df2eccf70f6f2a8 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 09:42:28 +0200 Subject: [PATCH 25/30] read files in binary mode in test_copy_framework_files to avoid broken test in Python 3 --- test/framework/filetools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index deda12439c..8f9aa5312b 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2218,7 +2218,7 @@ def test_copy_framework_files(self): copied_path = os.path.join(target_dir, test_file) self.assertTrue(os.path.exists(copied_path)) - self.assertEqual(ft.read_file(orig_path), ft.read_file(copied_path)) + self.assertEqual(ft.read_file(orig_path, mode='rb'), ft.read_file(copied_path, mode='rb')) self.assertTrue(os.path.samefile(copied_path, res['paths_in_repo'][idx])) From e549a963d4a5908ffd0699cb771c22e1a4c56101 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 10:20:23 +0200 Subject: [PATCH 26/30] appease the Hound --- easybuild/tools/github.py | 4 ++-- test/framework/filetools.py | 2 +- test/framework/options.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index f2d2ef4b60..d24d87e7ca 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -50,8 +50,8 @@ 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_file, copy_framework_files -from easybuild.tools.filetools import det_patched_files, decode_class_name, download_file, extract_file +from easybuild.tools.filetools import apply_patch, 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 from easybuild.tools.systemtools import UNKNOWN, get_tool_version diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 8f9aa5312b..1940f29946 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2166,7 +2166,7 @@ def test_copy_easyblocks(self): copied_gcc_eb = os.path.join(g_dir, 'gcc.py') self.assertEqual(ft.read_file(copied_gcc_eb), ft.read_file(gcc_eb)) self.assertTrue(os.path.samefile(res['paths_in_repo'][0], copied_gcc_eb)) - + gen_dir = os.path.join(easyblocks_dir, 'generic') self.assertEqual(sorted(os.listdir(gen_dir)), ['configuremake.py']) copied_configuremake = os.path.join(gen_dir, 'configuremake.py') diff --git a/test/framework/options.py b/test/framework/options.py index 668d8103e4..84409ff8ea 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -3162,7 +3162,7 @@ def test_new_branch_github(self): # test framework with tweaked copy of test_module_naming_scheme.py test_mns_py = os.path.join(topdir, 'sandbox', 'easybuild', 'tools', 'module_naming_scheme', - 'test_module_naming_scheme.py') + 'test_module_naming_scheme.py') target_dir = os.path.join(self.test_prefix, 'easybuild-framework', 'test', 'framework', 'sandbox', 'easybuild', 'tools', 'module_naming_scheme') mkdir(target_dir, parents=True) From cc38187698d7723493624bba4db4d1620da863d3 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 12:47:07 +0200 Subject: [PATCH 27/30] make sure files being copied are in 'easybuild-framework' directory in test_copy_framework_files --- test/framework/filetools.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 1940f29946..c40b454507 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2206,7 +2206,15 @@ def test_copy_framework_files(self): os.path.join('test', 'framework', 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz'), os.path.join('setup.py'), ] - test_paths = [os.path.join(topdir, f) for f in test_files] + + # files being copied are expected to be in a directory named 'easybuild-framework', + # so we need to make sure that's the case here as well (may not be in workspace dir on Travis from example) + framework_dir = os.path.join(self.test_prefix, 'easybuild-framework') + for test_file in test_files: + ft.copy_file(os.path.join(topdir, test_file), os.path.join(framework_dir, test_file)) + + test_paths = [os.path.join(framework_dir, f) for f in test_files] + res = ft.copy_framework_files(test_paths, target_dir) self.assertEqual(sorted(os.listdir(target_dir)), ['easybuild', 'setup.py', 'test']) From bd91e0a2b7e63a82deec9573bd883be7e09ece26 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 12:51:19 +0200 Subject: [PATCH 28/30] make sure pr_target_repo build option is what we expect it to be in test_det_pr_target_repo --- test/framework/github.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/framework/github.py b/test/framework/github.py index 8906e7f78d..4ddce65e38 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -39,7 +39,7 @@ from easybuild.base.rest import RestClient from easybuild.framework.easyconfig.tools import categorize_files_by_type from easybuild.tools.build_log import EasyBuildError -from easybuild.tools.config import module_classes +from easybuild.tools.config import build_option, module_classes from easybuild.tools.configobj import ConfigObj from easybuild.tools.filetools import read_file, write_file from easybuild.tools.github import VALID_CLOSE_PR_REASONS @@ -697,6 +697,8 @@ def test_det_account_branch_for_pr(self): def test_det_pr_target_repo(self): """Test det_pr_target_repo.""" + self.assertEqual(build_option('pr_target_repo'), None) + # no files => return default target repo (None) self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([])), None) @@ -724,6 +726,7 @@ def test_det_pr_target_repo(self): toy_eb = os.path.join(testdir, 'sandbox', 'easybuild', 'easyblocks', 't', 'toy.py') self.assertTrue(os.path.exists(toy_eb)) + self.assertEqual(build_option('pr_target_repo'), None) self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([github_py])), 'easybuild-framework') self.assertEqual(gh.det_pr_target_repo(categorize_files_by_type([configuremake])), 'easybuild-easyblocks') py_files = [github_py, configuremake] From aab059c5dd18c8991fe56aa90c2dae54fffd2826 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 13:16:53 +0200 Subject: [PATCH 29/30] conditionally include setup.py in list of test files in test_copy_framework_files, since it may not be there... --- test/framework/filetools.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index c40b454507..9e95d45779 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2204,8 +2204,21 @@ def test_copy_framework_files(self): os.path.join('easybuild', 'tools', 'filetools.py'), os.path.join('test', 'framework', 'modules.py'), os.path.join('test', 'framework', 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz'), - os.path.join('setup.py'), ] + expected_entries = ['easybuild', 'test'] + # test/framework/modules.py is not new + expected_new = [True, False, True] + + # we include setup.py conditionally because it may not be there, + # for example when running the tests on an actual easybuild-framework instalation, + # as opposed to when running from a repository checkout... + # setup.py is an important test case, since it has no parent directory + # (it's straight in the easybuild-framework directory) + setup_py = 'setup.py' + if os.path.exists(os.path.join(topdir, setup_py)): + test_files.append(os.path.join(setup_py)) + expected_entries.append(setup_py) + expected_new.append(True) # files being copied are expected to be in a directory named 'easybuild-framework', # so we need to make sure that's the case here as well (may not be in workspace dir on Travis from example) @@ -2217,7 +2230,7 @@ def test_copy_framework_files(self): res = ft.copy_framework_files(test_paths, target_dir) - self.assertEqual(sorted(os.listdir(target_dir)), ['easybuild', 'setup.py', 'test']) + self.assertEqual(sorted(os.listdir(target_dir)), sorted(expected_entries)) self.assertEqual(sorted(res.keys()), ['new', 'paths_in_repo']) @@ -2230,7 +2243,7 @@ def test_copy_framework_files(self): self.assertTrue(os.path.samefile(copied_path, res['paths_in_repo'][idx])) - self.assertEqual(res['new'], [True, False, True, True]) # test/framework/moduels.py is not new + self.assertEqual(res['new'], expected_new) def suite(): From 1ac9137f117a0e4dfa54c9f4e7f7c2a0ffa727c6 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 7 Apr 2020 13:20:18 +0200 Subject: [PATCH 30/30] fix test_det_pr_target_repo, take into account that __file__ may point to github.pyc... --- 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 4ddce65e38..bd1e7cecd4 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -718,8 +718,8 @@ def test_det_pr_target_repo(self): # if only Python files are involved, result is easyblocks or framework repo; # all Python files are easyblocks => easyblocks repo, otherwise => framework repo; # files are opened and inspected here to discriminate between easyblocks & other Python files, so must exist! - github_py = os.path.abspath(__file__) - testdir = os.path.dirname(github_py) + testdir = os.path.dirname(os.path.abspath(__file__)) + github_py = os.path.join(testdir, 'github.py') configuremake = os.path.join(testdir, 'sandbox', 'easybuild', 'easyblocks', 'generic', 'configuremake.py') self.assertTrue(os.path.exists(configuremake))