diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index dd0e298aed..9025792ba7 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -46,6 +46,7 @@ import sys import tempfile from collections import OrderedDict +from typing import List from easybuild.base import fancylogger from easybuild.framework.easyconfig import EASYCONFIGS_PKG_SUBDIR @@ -60,7 +61,7 @@ from easybuild.tools.environment import restore_env from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, get_cwd, find_easyconfigs, is_patch_file from easybuild.tools.filetools import locate_files, read_file, resolve_path, which, write_file -from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO +from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO, CategorizedPaths from easybuild.tools.github import det_pr_labels, det_pr_title, download_repo, fetch_easyconfigs_from_commit from easybuild.tools.github import fetch_easyconfigs_from_pr, fetch_pr_data from easybuild.tools.github import fetch_files_from_commit, fetch_files_from_pr @@ -617,37 +618,30 @@ def dump_env_script(easyconfigs): dump_env_easyblock(app, orig_env=orig_env, ec_path=ec.path, script_path=script_path) -def categorize_files_by_type(paths): +def categorize_files_by_type(paths: List[str]) -> CategorizedPaths: """ 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': [], - } + res = CategorizedPaths([], [], [], []) for path in paths: if path.startswith(':'): - res['files_to_delete'].append(path[1:]) + res.files_to_delete.append(path[1:]) elif path.endswith('.py'): - res['py_files'].append(path) + 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) + res.patch_files.append(path) elif path.endswith('.patch'): if not os.path.exists(path): raise EasyBuildError('File %s does not exist, did you mistype the path?', path) - elif not os.path.isfile(path): + if not os.path.isfile(path): raise EasyBuildError('File %s is expected to be a regular file, but is a folder instead', path) - else: - raise EasyBuildError('%s is not detected as a valid patch file. Please verify its contents!', - path) + raise EasyBuildError('%s is not detected as a valid patch file. Please verify its contents!', path) else: # anything else is considered to be an easyconfig file - res['easyconfigs'].append(path) + res.easyconfigs.append(path) return res diff --git a/easybuild/main.py b/easybuild/main.py index dda45eb00d..22d14f2ee0 100755 --- a/easybuild/main.py +++ b/easybuild/main.py @@ -402,7 +402,7 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session no_ec_opts = [options.aggregate_regtest, options.regtest, any_pr_option_set, search_query] # determine paths to easyconfigs - determined_paths = det_easyconfig_paths(categorized_paths['easyconfigs']) + determined_paths = det_easyconfig_paths(categorized_paths.easyconfigs) # only copy easyconfigs here if we're not using --try-* (that's handled below) copy_ec = options.copy_ec and not tweaked_ecs_paths diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 9bb224537b..dd0cc09286 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -48,6 +48,7 @@ from http import HTTPStatus from http.client import HTTPException from string import ascii_letters +from typing import List, NamedTuple from urllib.request import HTTPError, URLError, urlopen from easybuild.base import fancylogger @@ -90,6 +91,14 @@ _log.warning("Failed to import 'git' Python module: %s", err) +class CategorizedPaths(NamedTuple): + easyconfigs: List[str] + easyconfigs: List[str] + files_to_delete: List[str] + patch_files: List[str] + py_files: List[str] + + GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' GITHUB_BRANCH_MAIN = 'main' @@ -1052,7 +1061,8 @@ def setup_repo(git_repo, target_account, target_repo, branch_name, silent=False, @only_if_module_is_available('git', pkgname='GitPython') -def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_account=None, commit_msg=None): +def _easyconfigs_pr_common(paths: CategorizedPaths, ecs, start_branch=None, pr_branch=None, + start_account=None, commit_msg=None): """ Common code for new_pr and update_pr functions: * check whether all supplied paths point to existing files @@ -1072,8 +1082,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'] or paths['py_files']: - for path in paths['easyconfigs'] + paths['py_files']: + 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: @@ -1085,7 +1095,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ exit_code=EasyBuildExit.OPTION_ERROR ) - if not any(paths.values()): + if not any(paths): raise EasyBuildError("No paths specified", exit_code=EasyBuildExit.OPTION_ERROR) pr_target_repo = det_pr_target_repo(paths) @@ -1139,7 +1149,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ # figure out commit message to use if commit_msg: - if (pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths['files_to_delete'] + if (pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths.files_to_delete and is_new_pr): # Only if opening a new PR msg = "When only adding new easyconfigs a PR commit msg (--pr-commit-msg) should not be used, as " msg += "the PR title will be automatically generated." @@ -1150,11 +1160,11 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ raise EasyBuildError(msg) cnt = len(file_info['paths_in_repo']) _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']: + 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']) + 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: @@ -1163,22 +1173,22 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if not new] if modified_files: msg += '\nModified: ' + ', '.join(modified_files) - if paths['files_to_delete']: - msg += '\nDeleted: ' + ', '.join(paths['files_to_delete']) + if paths.files_to_delete: + msg += '\nDeleted: ' + ', '.join(paths.files_to_delete) raise EasyBuildError("A meaningful commit message must be specified via --pr-commit-msg when " "modifying/deleting files or targeting the framework repo." + msg, exit_code=EasyBuildExit.OPTION_ERROR) # figure out to which software name patches relate, and copy them to the right place - if paths['patch_files']: - patch_specs = det_patch_specs(paths['patch_files'], file_info, [target_dir]) + if paths.patch_files: + patch_specs = det_patch_specs(paths.patch_files, file_info, [target_dir]) print_msg("copying patch files to %s..." % target_dir) patch_info = copy_patch_files(patch_specs, target_dir) # determine path to files to delete (if any) deleted_paths = [] - for fn in paths['files_to_delete']: + for fn in paths.files_to_delete: fullpath = os.path.join(repo_path, fn) if os.path.exists(fullpath): deleted_paths.append(fullpath) @@ -1217,8 +1227,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if pr_branch is None: if ec_paths and pr_target_repo == GITHUB_EASYCONFIGS_REPO: label = file_info['ecs'][0].name + re.sub('[.-]', '', file_info['ecs'][0].version) - elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and paths.get('py_files'): - label = os.path.splitext(os.path.basename(paths['py_files'][0]))[0] + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and paths.py_files: + label = os.path.splitext(os.path.basename(paths.py_files[0]))[0] else: label = ''.join(random.choice(ascii_letters) for _ in range(10)) pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), label) @@ -1233,7 +1243,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ git_repo.index.add(file_info['paths_in_repo']) git_repo.index.add(dep_info['paths_in_repo']) - if paths['patch_files']: + if paths.patch_files: _log.debug("Staging all %d new/modified patch files", len(patch_info['paths_in_repo'])) git_repo.index.add(patch_info['paths_in_repo']) @@ -1913,7 +1923,7 @@ def add_pr_labels(pr, branch=GITHUB_DEVELOP_BRANCH): @only_if_module_is_available('git', pkgname='GitPython') -def new_branch_github(paths, ecs, commit_msg=None): +def new_branch_github(paths: CategorizedPaths, ecs, commit_msg=None): """ Create new branch on GitHub using specified files @@ -1971,7 +1981,8 @@ def det_pr_title(ecs): @only_if_module_is_available('git', pkgname='GitPython') -def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, pr_metadata=None, commit_msg=None): +def new_pr_from_branch(branch_name: CategorizedPaths, title=None, descr=None, + pr_target_repo=None, pr_metadata=None, commit_msg=None): """ Create new pull request from specified branch on GitHub. """ @@ -2152,7 +2163,7 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, 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): +def new_pr(paths: CategorizedPaths, ecs, title=None, descr=None, commit_msg=None): """ Open new pull request using specified files @@ -2184,8 +2195,8 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): raise EasyBuildError(msg, exit_code=EasyBuildExit.EASYCONFIG_ERROR) patch = patch_info['name'] - if patch not in paths['patch_files'] and not os.path.isfile(os.path.join(os.path.dirname(ec_path), - patch)): + if patch not in paths.patch_files and not os.path.isfile(os.path.join(os.path.dirname(ec_path), + patch)): print_warning("new patch file %s, referenced by %s, is not included in this PR" % (patch, ec.filename())) @@ -2226,7 +2237,7 @@ def det_account_branch_for_pr(pr_id, github_user=None, pr_target_repo=None): return account, branch -def det_pr_target_repo(paths): +def det_pr_target_repo(paths: CategorizedPaths): """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, .py files) @@ -2234,21 +2245,18 @@ def det_pr_target_repo(paths): pr_target_repo = build_option('pr_target_repo') # 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): + if paths.py_files and not (paths.easyconfigs or paths.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): + if all(get_easyblock_class_name(path) for path in paths.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: @@ -2257,7 +2265,8 @@ def det_pr_target_repo(paths): # 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)): + elif paths.easyconfigs or paths.patch_files or (paths.files_to_delete and all(x.endswith('.eb') + for x in paths.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) @@ -2268,7 +2277,7 @@ def det_pr_target_repo(paths): @only_if_module_is_available('git', pkgname='GitPython') -def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None): +def update_branch(branch_name, paths: CategorizedPaths, ecs, github_account=None, commit_msg=None): """ Update specified branch in GitHub using specified files @@ -2303,7 +2312,7 @@ def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None) @only_if_module_is_available('git', pkgname='GitPython') -def update_pr(pr_id, paths, ecs, commit_msg=None): +def update_pr(pr_id, paths: CategorizedPaths, ecs, commit_msg=None): """ Update specified pull request using specified files diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index e08465e05c..089279bd7e 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -60,7 +60,7 @@ from easybuild.framework.easyconfig.style import check_easyconfigs_style from easybuild.framework.easyconfig.tools import alt_easyconfig_paths, categorize_files_by_type, check_sha256_checksums from easybuild.framework.easyconfig.tools import dep_graph, det_copy_ec_specs, find_related_easyconfigs, get_paths_for -from easybuild.framework.easyconfig.tools import parse_easyconfigs +from easybuild.framework.easyconfig.tools import parse_easyconfigs, CategorizedPaths from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak, tweak_one from easybuild.framework.extension import construct_exts_filter_cmds from easybuild.toolchains.system import SystemToolchain @@ -3876,8 +3876,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': [], 'py_files': []}, - categorize_files_by_type([])) + self.assertEqual(categorize_files_by_type([]), CategorizedPaths([], [], [], [])) test_dir = os.path.dirname(os.path.abspath(__file__)) test_ecs_dir = os.path.join(test_dir, 'easyconfigs') @@ -3904,10 +3903,10 @@ def test_categorize_files_by_type(self): gzip_ec, 'foo', ] - 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]) + 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]) # Error cases tmpdir = tempfile.mkdtemp()