diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 3e8cce5054..e05af337ca 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -47,7 +47,8 @@ from easybuild.base import fancylogger from easybuild.framework.easyconfig import EASYCONFIGS_PKG_SUBDIR from easybuild.framework.easyconfig.easyconfig import EASYCONFIGS_ARCHIVE_DIR, ActiveMNS, EasyConfig -from easybuild.framework.easyconfig.easyconfig import create_paths, get_easyblock_class, process_easyconfig +from easybuild.framework.easyconfig.easyconfig import create_paths, det_file_info, get_easyblock_class +from easybuild.framework.easyconfig.easyconfig import process_easyconfig from easybuild.framework.easyconfig.format.yeb import quote_yaml_special_chars from easybuild.framework.easyconfig.style import cmdline_easyconfigs_style_check from easybuild.tools.build_log import EasyBuildError, print_msg, print_warning @@ -55,7 +56,9 @@ from easybuild.tools.environment import restore_env from easybuild.tools.filetools import find_easyconfigs, is_patch_file, locate_files from easybuild.tools.filetools import read_file, resolve_path, which, write_file -from easybuild.tools.github import fetch_easyconfigs_from_pr, fetch_files_from_pr, download_repo +from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO +from easybuild.tools.github import det_pr_labels, download_repo, fetch_easyconfigs_from_pr, fetch_pr_data +from easybuild.tools.github import fetch_files_from_pr from easybuild.tools.multidiff import multidiff from easybuild.tools.py2vs3 import OrderedDict from easybuild.tools.toolchain.toolchain import is_system_toolchain @@ -474,14 +477,19 @@ def find_related_easyconfigs(path, ec): return sorted(res) -def review_pr(paths=None, pr=None, colored=True, branch='develop'): +def review_pr(paths=None, pr=None, colored=True, branch='develop', testing=False): """ Print multi-diff overview between specified easyconfigs or PR and specified branch. :param pr: pull request number in easybuild-easyconfigs repo to review :param paths: path tuples (path, generated) of easyconfigs to review :param colored: boolean indicating whether a colored multi-diff should be generated :param branch: easybuild-easyconfigs branch to compare with + :param testing: whether to ignore PR labels (used in test_review_pr) """ + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO + if pr_target_repo != GITHUB_EASYCONFIGS_REPO: + raise EasyBuildError("Reviewing PRs for repositories other than easyconfigs hasn't been implemented yet") + tmpdir = tempfile.mkdtemp() download_repo_path = download_repo(branch=branch, path=tmpdir) @@ -508,6 +516,20 @@ def review_pr(paths=None, pr=None, colored=True, branch='develop'): else: lines.extend(['', "(no related easyconfigs found for %s)\n" % os.path.basename(ec['spec'])]) + if pr: + file_info = det_file_info(pr_files, download_repo_path) + + pr_target_account = build_option('pr_target_account') + github_user = build_option('github_user') + pr_data, _ = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user) + pr_labels = [label['name'] for label in pr_data['labels']] if not testing else [] + + expected_labels = det_pr_labels(file_info, pr_target_repo) + missing_labels = [label for label in expected_labels if label not in pr_labels] + + if missing_labels: + lines.extend(['', "This PR should be labelled with %s" % ', '.join(["'%s'" % ml for ml in missing_labels])]) + return '\n'.join(lines) diff --git a/easybuild/main.py b/easybuild/main.py index f13ed688b6..3c7429ea78 100644 --- a/easybuild/main.py +++ b/easybuild/main.py @@ -61,7 +61,7 @@ from easybuild.tools.filetools import adjust_permissions, cleanup, copy_files, dump_index, load_index from easybuild.tools.filetools import locate_files, read_file, register_lock_cleanup_signal_handlers, write_file from easybuild.tools.github import check_github, close_pr, find_easybuild_easyconfig -from easybuild.tools.github import install_github_token, list_prs, merge_pr, new_branch_github, new_pr +from easybuild.tools.github import add_pr_labels, install_github_token, list_prs, merge_pr, new_branch_github, new_pr from easybuild.tools.github import new_pr_from_branch from easybuild.tools.github import sync_branch_with_develop, sync_pr_with_develop, update_branch, update_pr from easybuild.tools.hooks import START, END, load_hooks, run_hook @@ -260,7 +260,10 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): merge_pr(options.merge_pr) elif options.review_pr: - print(review_pr(pr=options.review_pr, colored=use_color(options.color))) + print(review_pr(pr=options.review_pr, colored=use_color(options.color), testing=testing)) + + elif options.add_pr_labels: + add_pr_labels(options.add_pr_labels) elif options.list_installed_software: detailed = options.list_installed_software == 'detailed' @@ -277,6 +280,7 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): # non-verbose cleanup after handling GitHub integration stuff or printing terse info early_stop_options = [ + options.add_pr_labels, options.check_github, options.create_index, options.install_github_token, diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 6be474bcd0..f3ff4629e5 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -1359,6 +1359,95 @@ def merge_url(gh): print_warning("Review indicates this PR should not be merged (use -f/--force to do so anyway)") +def det_pr_labels(file_info, pr_target_repo): + """ + Determine labels for a pull request based on provided information on files changed by that pull request. + """ + labels = [] + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: + if any(file_info['new_folder']): + labels.append('new') + if any(file_info['new_file_in_existing_folder']): + labels.append('update') + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO: + if any(file_info['new']): + labels.append('new') + return labels + + +def post_pr_labels(pr, labels): + """ + Update PR labels + """ + pr_target_account = build_option('pr_target_account') + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO + + # fetch GitHub token if available + github_user = build_option('github_user') + if github_user is None: + _log.info("GitHub user not specified, not adding labels to PR# %s" % pr) + return False + + github_token = fetch_github_token(github_user) + if github_token is None: + _log.info("GitHub token for user '%s' not found, not adding labels to PR# %s" % (github_user, pr)) + return False + + dry_run = build_option('dry_run') or build_option('extended_dry_run') + + if not dry_run: + g = RestClient(GITHUB_API_URL, username=github_user, token=github_token) + + pr_url = g.repos[pr_target_account][pr_target_repo].issues[pr] + try: + status, data = pr_url.labels.post(body=labels) + if status == HTTP_STATUS_OK: + print_msg("Added labels %s to PR#%s" % (', '.join(labels), pr), log=_log, prefix=False) + return True + except HTTPError as err: + _log.info("Failed to add labels to PR# %s: %s." % (pr, err)) + return False + else: + return True + + +def add_pr_labels(pr, branch='develop'): + """ + Try to determine and add labels to PR. + :param pr: pull request number in easybuild-easyconfigs repo + :param branch: easybuild-easyconfigs branch to compare with + """ + pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO + if pr_target_repo != GITHUB_EASYCONFIGS_REPO: + raise EasyBuildError("Adding labels to PRs for repositories other than easyconfigs hasn't been implemented yet") + + tmpdir = tempfile.mkdtemp() + + download_repo_path = download_repo(branch=branch, path=tmpdir) + + pr_files = [path for path in fetch_easyconfigs_from_pr(pr) if path.endswith('.eb')] + + file_info = det_file_info(pr_files, download_repo_path) + + pr_target_account = build_option('pr_target_account') + github_user = build_option('github_user') + pr_data, _ = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user) + pr_labels = [label['name'] for label in pr_data['labels']] + + expected_labels = det_pr_labels(file_info, pr_target_repo) + missing_labels = [label for label in expected_labels if label not in pr_labels] + + dry_run = build_option('dry_run') or build_option('extended_dry_run') + + if missing_labels: + missing_labels_txt = ', '.join(["'%s'" % ml for ml in missing_labels]) + print_msg("PR #%s should be labelled %s" % (pr, missing_labels_txt), log=_log, prefix=False) + if not dry_run and not post_pr_labels(pr, missing_labels): + print_msg("Could not add labels %s to PR #%s" % (missing_labels_txt, pr), log=_log, prefix=False) + else: + print_msg("Could not determine any missing labels for PR #%s" % pr, log=_log, prefix=False) + + @only_if_module_is_available('git', pkgname='GitPython') def new_branch_github(paths, ecs, commit_msg=None): """ @@ -1486,14 +1575,9 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, file_info = det_file_info(ec_paths, target_dir) - labels = [] - 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') + labels = det_pr_labels(file_info, pr_target_repo) + if pr_target_repo == GITHUB_EASYCONFIGS_REPO: # 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)]) @@ -1503,9 +1587,6 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=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: @@ -1581,15 +1662,9 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, print_msg("Opened pull request: %s" % data['html_url'], log=_log, prefix=False) if labels: - # post labels pr = data['html_url'].split('/')[-1] - pr_url = g.repos[pr_target_account][pr_target_repo].issues[pr] - try: - status, data = pr_url.labels.post(body=labels) - if status == HTTP_STATUS_OK: - print_msg("Added labels %s to PR#%s" % (', '.join(labels), pr), log=_log, prefix=False) - except HTTPError as err: - _log.info("Failed to add labels to PR# %s: %s." % (pr, err)) + if not post_pr_labels(pr, labels): + print_msg("This PR should be labelled %s" % ', '.join(labels), log=_log, prefix=False) def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index aa8294b074..5271be1609 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -629,6 +629,7 @@ def github_options(self): descr = ("GitHub integration options", "Integration with GitHub") opts = OrderedDict({ + 'add-pr-labels': ("Try to add labels to PR based on files changed", int, 'store', None, {'metavar': 'PR#'}), 'check-github': ("Check status of GitHub integration, and report back", None, 'store_true', False), 'check-contrib': ("Runs checks to see whether the given easyconfigs are ready to be contributed back", None, 'store_true', False), diff --git a/test/framework/github.py b/test/framework/github.py index 7281a46002..f83088c4b2 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -42,7 +42,7 @@ 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 +from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO, GITHUB_EASYBLOCKS_REPO, VALID_CLOSE_PR_REASONS from easybuild.tools.testing import post_pr_test_report, session_state from easybuild.tools.py2vs3 import HTTPError, URLError, ascii_letters import easybuild.tools.github as gh @@ -120,6 +120,48 @@ def test_read(self): except (IOError, OSError): pass + def test_add_pr_labels(self): + """Test add_pr_labels function.""" + if self.skip_github_tests: + print("Skipping test_add_pr_labels, no GitHub token available?") + return + + build_options = { + 'pr_target_account': GITHUB_USER, + 'pr_target_repo': GITHUB_EASYBLOCKS_REPO, + 'github_user': GITHUB_TEST_ACCOUNT, + 'dry_run': True, + } + init_config(build_options=build_options) + + self.mock_stdout(True) + error_pattern = "Adding labels to PRs for repositories other than easyconfigs hasn't been implemented yet" + self.assertErrorRegex(EasyBuildError, error_pattern, gh.add_pr_labels, 1) + self.mock_stdout(False) + + build_options['pr_target_repo'] = GITHUB_EASYCONFIGS_REPO + init_config(build_options=build_options) + + # PR #11262 includes easyconfigs that use 'dummy' toolchain, + # so we need to allow triggering deprecated behaviour + self.allow_deprecated_behaviour() + + self.mock_stdout(True) + self.mock_stderr(True) + gh.add_pr_labels(11262) + stdout = self.get_stdout() + self.mock_stdout(False) + self.mock_stderr(False) + self.assertTrue("Could not determine any missing labels for PR #11262" in stdout) + + self.mock_stdout(True) + self.mock_stderr(True) + gh.add_pr_labels(8006) # closed, unmerged, unlabeled PR + stdout = self.get_stdout() + self.mock_stdout(False) + self.mock_stderr(False) + self.assertTrue("PR #8006 should be labelled 'update'" in stdout) + def test_fetch_pr_data(self): """Test fetch_pr_data function.""" if self.skip_github_tests: @@ -681,6 +723,25 @@ def run_check(expected_result=False): expected_warning = '' self.assertEqual(run_check(True), '') + def test_det_pr_labels(self): + """Test for det_pr_labels function.""" + + file_info = {'new_folder': [False], 'new_file_in_existing_folder': [True]} + res = gh.det_pr_labels(file_info, GITHUB_EASYCONFIGS_REPO) + self.assertEqual(res, ['update']) + + file_info = {'new_folder': [True], 'new_file_in_existing_folder': [False]} + res = gh.det_pr_labels(file_info, GITHUB_EASYCONFIGS_REPO) + self.assertEqual(res, ['new']) + + file_info = {'new_folder': [True, False], 'new_file_in_existing_folder': [False, True]} + res = gh.det_pr_labels(file_info, GITHUB_EASYCONFIGS_REPO) + self.assertTrue(sorted(res), ['new', 'update']) + + file_info = {'new': [True]} + res = gh.det_pr_labels(file_info, GITHUB_EASYBLOCKS_REPO) + self.assertEqual(res, ['new']) + def test_det_patch_specs(self): """Test for det_patch_specs function.""" diff --git a/test/framework/options.py b/test/framework/options.py index 5deaeda657..4631f4fc76 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -754,6 +754,8 @@ def test_list_easyblocks(self): r'EasyBlock', r'\|-- bar', r'\|-- ConfigureMake', + r'\| \|-- MakeCp', + r'\|-- EB_EasyBuildMeta', r'\|-- EB_FFTW', r'\|-- EB_foo', r'\| \|-- EB_foofoo', @@ -3582,6 +3584,21 @@ def test_review_pr(self): regex = re.compile(r"^Comparing gzip-1.10-\S* with gzip-1.10-") self.assertTrue(regex.search(txt), "Pattern '%s' not found in: %s" % (regex.pattern, txt)) + self.mock_stdout(True) + self.mock_stderr(True) + # closed PR for gzip 1.2.8 easyconfig, + # see https://github.com/easybuilders/easybuild-easyconfigs/pull/5365 + args = [ + '--color=never', + '--github-user=%s' % GITHUB_TEST_ACCOUNT, + '--review-pr=5365', + ] + self.eb_main(args, raise_error=True, testing=True) + txt = self.get_stdout() + self.mock_stdout(False) + self.mock_stderr(False) + self.assertTrue("This PR should be labelled with 'update'" in txt) + def test_set_tmpdir(self): """Test set_tmpdir config function.""" self.purge_environment() diff --git a/test/framework/sandbox/easybuild/easyblocks/e/__init__.py b/test/framework/sandbox/easybuild/easyblocks/e/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/framework/sandbox/easybuild/easyblocks/e/easybuildmeta.py b/test/framework/sandbox/easybuild/easyblocks/e/easybuildmeta.py new file mode 100644 index 0000000000..e27c0c66d0 --- /dev/null +++ b/test/framework/sandbox/easybuild/easyblocks/e/easybuildmeta.py @@ -0,0 +1,34 @@ +## +# Copyright 2009-2020 Ghent University +# +# This file is part of EasyBuild, +# originally created by the HPC team of Ghent University (http://ugent.be/hpc/en), +# with support of Ghent University (http://ugent.be/hpc), +# the Flemish Supercomputer Centre (VSC) (https://www.vscentrum.be), +# Flemish Research Foundation (FWO) (http://www.fwo.be/en) +# and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en). +# +# https://github.com/easybuilders/easybuild +# +# EasyBuild is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation v2. +# +# EasyBuild is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with EasyBuild. If not, see . +## +""" +Dummy easyblock for EasyBuildMeta + +@author: Miguel Dias Costa (National University of Singapore) +""" +from easybuild.framework.easyblock import EasyBlock + + +class EB_EasyBuildMeta(EasyBlock): + pass diff --git a/test/framework/sandbox/easybuild/easyblocks/generic/makecp.py b/test/framework/sandbox/easybuild/easyblocks/generic/makecp.py new file mode 100644 index 0000000000..6b258c87d6 --- /dev/null +++ b/test/framework/sandbox/easybuild/easyblocks/generic/makecp.py @@ -0,0 +1,49 @@ +## +# Copyright 2009-2020 Ghent University +# +# This file is part of EasyBuild, +# originally created by the HPC team of Ghent University (http://ugent.be/hpc/en), +# with support of Ghent University (http://ugent.be/hpc), +# the Flemish Supercomputer Centre (VSC) (https://www.vscentrum.be), +# Flemish Research Foundation (FWO) (http://www.fwo.be/en) +# and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en). +# +# https://github.com/easybuilders/easybuild +# +# EasyBuild is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation v2. +# +# EasyBuild is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with EasyBuild. If not, see . +## +""" +Dummy easyblock for Makecp. + +@author: Miguel Dias Costa (National University of Singapore) +""" +from easybuild.easyblocks.generic.configuremake import ConfigureMake +from easybuild.framework.easyconfig import BUILD, MANDATORY + + +class MakeCp(ConfigureMake): + """Dummy support for software with no configure and no make install step.""" + + @staticmethod + def extra_options(extra_vars=None): + """ + Define list of files or directories to be copied after make + """ + extra = { + 'files_to_copy': [None, "List of files or dirs to copy", MANDATORY], + 'with_configure': [False, "Run configure script before building", BUILD], + } + if extra_vars is None: + extra_vars = {} + extra.update(extra_vars) + return ConfigureMake.extra_options(extra_vars=extra)