Merge pull request #3482 from boegel/copy-ec-from-pr

fix combination of --copy-ec and --from-pr

Author: ocaisa

easybuild/framework/easyconfig/tools.py

@@ -53,8 +53,9 @@
 from easybuild.tools.build_log import EasyBuildError, print_msg, print_warning
 from easybuild.tools.config import build_option
 from easybuild.tools.environment import restore_env
-from easybuild.tools.filetools import find_easyconfigs, is_patch_file, read_file, resolve_path, which, write_file
-from easybuild.tools.github import fetch_easyconfigs_from_pr, download_repo
+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.multidiff import multidiff
 from easybuild.tools.py2vs3 import OrderedDict
 from easybuild.tools.toolchain.toolchain import is_system_toolchain
@@ -348,44 +349,13 @@ def det_easyconfig_paths(orig_paths):
             ec_files = [path for path in pr_files if path.endswith('.eb')]
 
     if ec_files and robot_path:
-        # look for easyconfigs with relative paths in robot search path,
-        # unless they were found at the given relative paths
-
-        # determine which easyconfigs files need to be found, if any
-        ecs_to_find = []
-        for idx, ec_file in enumerate(ec_files):
-            if ec_file == os.path.basename(ec_file) and not os.path.exists(ec_file):
-                ecs_to_find.append((idx, ec_file))
-        _log.debug("List of easyconfig files to find: %s" % ecs_to_find)
-
-        # find missing easyconfigs by walking paths in robot search path
-        for path in robot_path:
-            _log.debug("Looking for missing easyconfig files (%d left) in %s..." % (len(ecs_to_find), path))
-            for (subpath, dirnames, filenames) in os.walk(path, topdown=True):
-                for idx, orig_path in ecs_to_find[:]:
-                    if orig_path in filenames:
-                        full_path = os.path.join(subpath, orig_path)
-                        _log.info("Found %s in %s: %s" % (orig_path, path, full_path))
-                        ec_files[idx] = full_path
-                        # if file was found, stop looking for it (first hit wins)
-                        ecs_to_find.remove((idx, orig_path))
-
-                # stop os.walk insanity as soon as we have all we need (os.walk loop)
-                if not ecs_to_find:
-                    break
-
-                # ignore subdirs specified to be ignored by replacing items in dirnames list used by os.walk
-                dirnames[:] = [d for d in dirnames if d not in build_option('ignore_dirs')]
-
-                # ignore archived easyconfigs, unless specified otherwise
-                if not build_option('consider_archived_easyconfigs'):
-                    dirnames[:] = [d for d in dirnames if d != EASYCONFIGS_ARCHIVE_DIR]
-
-            # stop os.walk insanity as soon as we have all we need (outer loop)
-            if not ecs_to_find:
-                break
-
-    return [os.path.abspath(ec_file) for ec_file in ec_files]
+        ignore_subdirs = build_option('ignore_dirs')
+        if not build_option('consider_archived_easyconfigs'):
+            ignore_subdirs.append(EASYCONFIGS_ARCHIVE_DIR)
+
+        ec_files = locate_files(ec_files, robot_path, ignore_subdirs=ignore_subdirs)
+
+    return ec_files
 
 
 def parse_easyconfigs(paths, validate=True):
@@ -728,3 +698,65 @@ def avail_easyblocks():
                                        easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path)
 
     return easyblocks
+
+
+def det_copy_ec_specs(orig_paths, from_pr):
+    """Determine list of paths + target directory for --copy-ec."""
+
+    target_path, paths = None, []
+
+    # if only one argument is specified, use current directory as target directory
+    if len(orig_paths) == 1:
+        target_path = os.getcwd()
+        paths = orig_paths[:]
+
+    # if multiple arguments are specified, assume that last argument is target location,
+    # and remove that from list of paths to copy
+    elif orig_paths:
+        target_path = orig_paths[-1]
+        paths = orig_paths[:-1]
+
+    # if --from-pr was used in combination with --copy-ec, some extra care must be taken
+    if from_pr:
+        # pull in the paths to all the changed files in the PR,
+        # which includes easyconfigs but also patch files (& maybe more);
+        # do this in a dedicated subdirectory of the working tmpdir,
+        # to avoid potential trouble with already existing files in the working tmpdir
+        # (note: we use a fixed subdirectory in the working tmpdir here rather than a unique random subdirectory,
+        #  to ensure that the caching for fetch_files_from_pr works across calls for the same PR)
+        tmpdir = os.path.join(tempfile.gettempdir(), 'fetch_files_from_pr_%s' % from_pr)
+        pr_paths = fetch_files_from_pr(pr=from_pr, path=tmpdir)
+
+        # assume that files need to be copied to current working directory for now
+        target_path = os.getcwd()
+
+        if orig_paths:
+            last_path = orig_paths[-1]
+
+            # check files touched by PR and see if the target directory for --copy-ec
+            # corresponds to the name of one of these files;
+            # if so we should copy the specified file(s) to the current working directory,
+            # since interpreting the last argument as target location is very unlikely to be correct in this case
+            pr_filenames = [os.path.basename(p) for p in pr_paths]
+            if last_path in pr_filenames:
+                paths = orig_paths[:]
+            else:
+                target_path = last_path
+                # exclude last argument that is used as target location
+                paths = orig_paths[:-1]
+
+        # if list of files to copy is empty at this point,
+        # we simply copy *all* files touched by the PR
+        if not paths:
+            paths = pr_paths
+
+        # replace path for files touched by PR (no need to worry about others)
+        for idx, path in enumerate(paths):
+            filename = os.path.basename(path)
+            pr_matches = [x for x in pr_paths if os.path.basename(x) == filename]
+            if len(pr_matches) == 1:
+                paths[idx] = pr_matches[0]
+            elif pr_matches:
+                raise EasyBuildError("Found multiple paths for %s in PR: %s", filename, pr_matches)
+
+    return paths, target_path

easybuild/main.py

@@ -50,17 +50,18 @@
 from easybuild.framework.easyconfig.easyconfig import clean_up_easyconfigs
 from easybuild.framework.easyconfig.easyconfig import fix_deprecated_easyconfigs, verify_easyconfig_filename
 from easybuild.framework.easyconfig.style import cmdline_easyconfigs_style_check
-from easybuild.framework.easyconfig.tools import categorize_files_by_type, dep_graph
+from easybuild.framework.easyconfig.tools import categorize_files_by_type, dep_graph, det_copy_ec_specs
 from easybuild.framework.easyconfig.tools import det_easyconfig_paths, dump_env_script, get_paths_for
 from easybuild.framework.easyconfig.tools import parse_easyconfigs, review_pr, run_contrib_checks, skip_available
 from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak
 from easybuild.tools.config import find_last_log, get_repository, get_repositorypath, build_option
 from easybuild.tools.containers.common import containerize
 from easybuild.tools.docs import list_software
-from easybuild.tools.filetools import adjust_permissions, cleanup, copy_file, copy_files, dump_index, load_index
-from easybuild.tools.filetools import read_file, register_lock_cleanup_signal_handlers, write_file
-from easybuild.tools.github import check_github, close_pr, new_branch_github, find_easybuild_easyconfig
-from easybuild.tools.github import install_github_token, list_prs, new_pr, new_pr_from_branch, merge_pr
+from easybuild.tools.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 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
 from easybuild.tools.modules import modules_tool
@@ -303,12 +304,9 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
             eb_file = find_easybuild_easyconfig()
             orig_paths.append(eb_file)
 
-    if len(orig_paths) == 1:
-        # if only one easyconfig file is specified, use current directory as target directory
-        target_path = os.getcwd()
-    elif orig_paths:
-        # last path is target when --copy-ec is used, so remove that from the list
-        target_path = orig_paths.pop() if options.copy_ec else None
+    if options.copy_ec:
+        # figure out list of files to copy + target location (taking into account --from-pr)
+        orig_paths, target_path = det_copy_ec_specs(orig_paths, options.from_pr)
 
     categorized_paths = categorize_files_by_type(orig_paths)
 
@@ -321,17 +319,17 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     # determine paths to easyconfigs
     determined_paths = det_easyconfig_paths(categorized_paths['easyconfigs'])
 
-    if (options.copy_ec and not tweaked_ecs_paths) or options.fix_deprecated_easyconfigs or options.show_ec:
+    # only copy easyconfigs here if we're not using --try-* (that's handled below)
+    copy_ec = options.copy_ec and not tweaked_ecs_paths
+
+    if copy_ec or options.fix_deprecated_easyconfigs or options.show_ec:
 
         if options.copy_ec:
-            if len(determined_paths) == 1:
-                copy_file(determined_paths[0], target_path)
-                print_msg("%s copied to %s" % (os.path.basename(determined_paths[0]), target_path), prefix=False)
-            elif len(determined_paths) > 1:
-                copy_files(determined_paths, target_path)
-                print_msg("%d file(s) copied to %s" % (len(determined_paths), target_path), prefix=False)
-            else:
-                raise EasyBuildError("One of more files to copy should be specified!")
+            # at this point some paths may still just be filenames rather than absolute paths,
+            # so try to determine full path for those too via robot search path
+            paths = locate_files(orig_paths, robot_path)
+
+            copy_files(paths, target_path, target_single_file=True, allow_empty=False, verbose=True)
 
         elif options.fix_deprecated_easyconfigs:
             fix_deprecated_easyconfigs(determined_paths)
@@ -361,7 +359,7 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     if options.regtest or options.aggregate_regtest:
         _log.info("Running regression test")
         # fallback: easybuild-easyconfigs install path
-        regtest_ok = regtest([path[0] for path in paths] or easyconfigs_pkg_paths, modtool)
+        regtest_ok = regtest([x for (x, _) in paths] or easyconfigs_pkg_paths, modtool)
         if not regtest_ok:
             _log.info("Regression test failed (partially)!")
             sys.exit(31)  # exit -> 3x1t -> 31
@@ -429,8 +427,9 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
         if tweaked_ecs_in_all_ecs:
             # Clean them, then copy them
             clean_up_easyconfigs(tweaked_ecs_in_all_ecs)
-            copy_files(tweaked_ecs_in_all_ecs, target_path)
-            print_msg("%d file(s) copied to %s" % (len(tweaked_ecs_in_all_ecs), target_path), prefix=False)
+            copy_files(tweaked_ecs_in_all_ecs, target_path, allow_empty=False, verbose=True)
+
+        clean_exit(logfile, eb_tmpdir, testing)
 
     # creating/updating PRs
     if pr_options:

easybuild/tools/config.py

@@ -101,6 +101,7 @@
 DEFAULT_PKG_TOOL = PKG_TOOL_FPM
 DEFAULT_PKG_TYPE = PKG_TYPE_RPM
 DEFAULT_PNS = 'EasyBuildPNS'
+DEFAULT_PR_TARGET_ACCOUNT = 'easybuilders'
 DEFAULT_PREFIX = os.path.join(os.path.expanduser('~'), ".local", "easybuild")
 DEFAULT_REPOSITORY = 'FileRepository'
 DEFAULT_WAIT_ON_LOCK_INTERVAL = 60
@@ -204,7 +205,6 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
         'pr_branch_name',
         'pr_commit_msg',
         'pr_descr',
-        'pr_target_account',
         'pr_target_repo',
         'pr_title',
         'rpath_filter',
@@ -307,6 +307,9 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
     DEFAULT_PKG_TYPE: [
         'package_type',
     ],
+    DEFAULT_PR_TARGET_ACCOUNT: [
+        'pr_target_account',
+    ],
     GENERAL_CLASS: [
         'suffix_modules_path',
     ],

easybuild/tools/options.py

@@ -64,11 +64,11 @@
 from easybuild.tools.config import DEFAULT_JOB_BACKEND, DEFAULT_LOGFILE_FORMAT, DEFAULT_MAX_FAIL_RATIO_PERMS
 from easybuild.tools.config import DEFAULT_MINIMAL_BUILD_ENV, DEFAULT_MNS, DEFAULT_MODULE_SYNTAX, DEFAULT_MODULES_TOOL
 from easybuild.tools.config import DEFAULT_MODULECLASSES, DEFAULT_PATH_SUBDIRS, DEFAULT_PKG_RELEASE, DEFAULT_PKG_TOOL
-from easybuild.tools.config import DEFAULT_PKG_TYPE, DEFAULT_PNS, DEFAULT_PREFIX, DEFAULT_REPOSITORY
-from easybuild.tools.config import DEFAULT_WAIT_ON_LOCK_INTERVAL, DEFAULT_WAIT_ON_LOCK_LIMIT, EBROOT_ENV_VAR_ACTIONS
-from easybuild.tools.config import ERROR, FORCE_DOWNLOAD_CHOICES, GENERAL_CLASS, IGNORE, JOB_DEPS_TYPE_ABORT_ON_ERROR
-from easybuild.tools.config import JOB_DEPS_TYPE_ALWAYS_RUN, LOADED_MODULES_ACTIONS, LOCAL_VAR_NAMING_CHECK_WARN
-from easybuild.tools.config import LOCAL_VAR_NAMING_CHECKS, WARN
+from easybuild.tools.config import DEFAULT_PKG_TYPE, DEFAULT_PNS, DEFAULT_PREFIX, DEFAULT_PR_TARGET_ACCOUNT
+from easybuild.tools.config import DEFAULT_REPOSITORY, DEFAULT_WAIT_ON_LOCK_INTERVAL, DEFAULT_WAIT_ON_LOCK_LIMIT
+from easybuild.tools.config import EBROOT_ENV_VAR_ACTIONS, ERROR, FORCE_DOWNLOAD_CHOICES, GENERAL_CLASS, IGNORE
+from easybuild.tools.config import JOB_DEPS_TYPE_ABORT_ON_ERROR, JOB_DEPS_TYPE_ALWAYS_RUN, LOADED_MODULES_ACTIONS
+from easybuild.tools.config import LOCAL_VAR_NAMING_CHECK_WARN, LOCAL_VAR_NAMING_CHECKS, WARN
 from easybuild.tools.config import get_pretend_installpath, init, init_build_options, mk_full_default_path
 from easybuild.tools.configobj import ConfigObj, ConfigObjError
 from easybuild.tools.docs import FORMAT_TXT, FORMAT_RST
@@ -78,7 +78,7 @@
 from easybuild.tools.environment import restore_env, unset_env_vars
 from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, expand_glob_paths, install_fake_vsc
 from easybuild.tools.filetools import move_file, which
-from easybuild.tools.github import GITHUB_EB_MAIN, GITHUB_PR_DIRECTION_DESC, GITHUB_PR_ORDER_CREATED
+from easybuild.tools.github import GITHUB_PR_DIRECTION_DESC, GITHUB_PR_ORDER_CREATED
 from easybuild.tools.github import GITHUB_PR_STATE_OPEN, GITHUB_PR_STATES, GITHUB_PR_ORDERS, GITHUB_PR_DIRECTIONS
 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS
 from easybuild.tools.github import fetch_easyblocks_from_pr, fetch_github_token
@@ -642,7 +642,7 @@ def github_options(self):
                                str, 'store', None),
             'pr-commit-msg': ("Commit message for new/updated pull request created with --new-pr", str, 'store', None),
             '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-account': ("Target account for new PRs", str, 'store', DEFAULT_PR_TARGET_ACCOUNT),
             'pr-target-branch': ("Target branch for new PRs", str, 'store', DEFAULT_BRANCH),
             'pr-target-repo': ("Target repository for new/updating PRs (default: auto-detect based on provided files)",
                                str, 'store', None),