Allow use of --copy-ec with --from-pr

easybuild/framework/easyconfig/tools.py

@@ -53,6 +53,7 @@
 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 copy_file, copy_files
 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.multidiff import multidiff
@@ -728,3 +729,22 @@ def avail_easyblocks():
                                        easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path)
 
     return easyblocks
+
+
+def copy_ecs_to_target(determined_paths, target_path, prefix=False, target_is_dir=False):
+    """
+    Copy list of easyconfigs to specified path
+
+    :param determined_paths: paths to ecs to copy
+    :param target_path: target to copy files to
+    :param prefix: include message prefix characters
+    :param target_is_dir: target is always a directory
+    """
+    if len(determined_paths) == 1 and not target_is_dir:
+        copy_file(determined_paths[0], target_path)
+        print_msg("%s copied to %s" % (os.path.basename(determined_paths[0]), target_path), prefix=prefix)
+    elif determined_paths:
+        copy_files(determined_paths, target_path)
+        print_msg("%d file(s) copied to %s" % (len(determined_paths), target_path), prefix=prefix)
+    else:
+        raise EasyBuildError("One of more files to copy should be specified!")

easybuild/main.py

@@ -39,6 +39,7 @@
 import os
 import stat
 import sys
+import tempfile
 import traceback
 
 # IMPORTANT this has to be the first easybuild import as it customises the logging
@@ -50,17 +51,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, copy_ecs_to_target, dep_graph
 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 adjust_permissions, cleanup, 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.github import check_github, close_pr, fetch_files_from_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 +305,16 @@ 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
+    # if only one easyconfig is specified, or if none are specified and we are using --from-pr,
+    # use current directory as target directory
+    if len(orig_paths) == 1 and not (options.copy_ec and options.from_pr):
         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
+    else:
+        # if no easyconfig files are specified and we are using --from-pr, use current directory as target directory
+        target_path = os.getcwd() if (options.copy_ec and options.from_pr) else None
 
     categorized_paths = categorize_files_by_type(orig_paths)
 
@@ -321,17 +327,37 @@ 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 are 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.from_pr:
+            # pull in the paths to all the changed files in the PR (need to do this in a new temp dir)
+            pr_paths = fetch_files_from_pr(pr=options.from_pr, path=tempfile.mktemp())
+            for pr_path in pr_paths:
+                # we assumed that the last argument from the command line was the target_path but if it appears in the
+                # PR file list then it was most likely intended to use the CWD and (also) copy that particular file
+                if target_path == os.path.basename(pr_path):
+                    if not orig_paths:
+                        # It should have been the only easyconfig selected
+                        determined_paths = [pr_path]
+                    else:
+                        if os.path.basename(pr_path) not in [os.path.basename(path) for path in determined_paths]:
+                            determined_paths.append(pr_path)
+                    target_path = os.getcwd()
+            other_pr_paths = []
+            for ec_path in determined_paths:
+                for pr_path in pr_paths:
+                    if os.path.basename(ec_path) == os.path.basename(pr_path):
+                        # Search for any associated patches (they would have the same dirname)
+                        for patch_path in pr_paths:
+                            if pr_path != patch_path and os.path.dirname(pr_path) == os.path.dirname(patch_path):
+                                # if it's an easyconfig, we already have it covered
+                                if not patch_path.endswith('.eb') and patch_path not in other_pr_paths:
+                                    other_pr_paths.append(patch_path)
+            determined_paths += other_pr_paths
 
         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!")
+            copy_ecs_to_target(determined_paths, target_path)
 
         elif options.fix_deprecated_easyconfigs:
             fix_deprecated_easyconfigs(determined_paths)
@@ -361,7 +387,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 +455,7 @@ 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_ecs_to_target(tweaked_ecs_in_all_ecs, target_path, target_is_dir=True)
 
     # creating/updating PRs
     if pr_options:

easybuild/tools/github.py

@@ -364,6 +364,7 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_
     _log.debug("%s downloaded to %s, extracting now" % (base_name, path))
 
     base_dir = extract_file(target_path, path, forced=True, change_into_dir=False)
+    working_directory = os.getcwd()
     change_dir(base_dir)
     extracted_path = os.path.join(base_dir, extracted_dir_name)
 
@@ -373,6 +374,9 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_
 
     write_file(latest_sha_path, latest_commit_sha, forced=True)
 
+    # go back to previous working directory
+    change_dir(working_directory)
+
     _log.debug("Repo %s at branch %s extracted into %s" % (repo, branch, extracted_path))
     return extracted_path
 

test/framework/options.py

@@ -1052,7 +1052,7 @@ def check_copied_files():
 
         check_copied_files()
 
-        # copying multiple easyconfig to an existing target file resuts in an error
+        # copying multiple easyconfig to an existing target file results in an error
         target = os.path.join(self.test_prefix, 'test.eb')
         self.assertTrue(os.path.isfile(target))
         args = ['--copy-ec', 'toy-0.0.eb', 'bzip2-1.0.6-GCC-4.9.2.eb', target]
@@ -1072,7 +1072,66 @@ def check_copied_files():
         self.assertTrue(os.path.exists(copied_toy_cwd))
         self.assertEqual(read_file(copied_toy_cwd), toy_ec_txt)
 
-        # --copy-ec without arguments results in a proper error
+        # Test --copy-ec coupled with --from-pr
+
+        test_working_dir = os.path.join(self.test_prefix, 'test_working_dir')
+        mkdir(test_working_dir)
+        test_target_dir = os.path.join(self.test_prefix, 'test_target_dir')
+        # Make sure the test target directory doesn't exist
+        remove_dir(test_target_dir)
+
+        all_ecs_pr8007 = [
+            'Arrow-0.7.1-intel-2017b-Python-3.6.3.eb',
+            'bat-0.3.3-fix-pyspark.patch',
+            'bat-0.3.3-intel-2017b-Python-3.6.3.eb',
+        ]
+
+        # test use of `--copy-ec` with `--from-pr` to the cwd
+        change_dir(test_working_dir)
+        args = ['--copy-ec', '--from-pr', '8007']
+        stdout = mocked_main(args)
+        self.assertEqual(stdout, '3 file(s) copied to %s' % test_working_dir)
+        # check that the files exist
+        for pr_file in all_ecs_pr8007:
+            self.assertTrue(os.path.exists(os.path.join(test_working_dir, pr_file)))
+            remove_file(os.path.join(test_working_dir, pr_file))
+
+        # copying multiple easyconfig files to a non-existing target directory (which is created automatically)
+        args = ['--copy-ec', '--from-pr', '8007', test_target_dir]
+        stdout = mocked_main(args)
+        self.assertEqual(stdout, '3 file(s) copied to %s' % test_target_dir)
+        for pr_file in all_ecs_pr8007:
+            self.assertTrue(os.path.exists(os.path.join(test_target_dir, pr_file)))
+        remove_dir(test_target_dir)
+
+        # test where we select a single file from a PR but also has a patch file
+        args = ['--copy-ec', '--from-pr', '8007', 'bat-0.3.3-intel-2017b-Python-3.6.3.eb', test_target_dir]
+        stdout = mocked_main(args)
+        self.assertEqual(stdout, '2 file(s) copied to %s' % test_target_dir)
+        for pr_file in ['bat-0.3.3-fix-pyspark.patch', 'bat-0.3.3-intel-2017b-Python-3.6.3.eb']:
+            self.assertTrue(os.path.exists(os.path.join(test_target_dir, pr_file)))
+        remove_dir(test_target_dir)
+
+        # test the same thing but where we don't provide a target directory
+        change_dir(test_working_dir)
+        args = ['--copy-ec', '--from-pr', '8007', 'bat-0.3.3-intel-2017b-Python-3.6.3.eb']
+        stdout = mocked_main(args)
+        self.assertEqual(stdout, '2 file(s) copied to %s' % test_working_dir)
+        for pr_file in ['bat-0.3.3-fix-pyspark.patch', 'bat-0.3.3-intel-2017b-Python-3.6.3.eb']:
+            path = os.path.join(test_working_dir, pr_file)
+            self.assertTrue(os.path.exists(path))
+            remove_file(path)
+
+        # test with only one ec in the PR (final argument is taken as a filename)
+        test_ec = os.path.join(self.test_prefix, 'test.eb')
+        args = ['--copy-ec', '--from-pr', '11521', test_ec]
+        ec_pr11521 = "ExifTool-12.00-GCCcore-9.3.0.eb"
+        stdout = mocked_main(args)
+        self.assertEqual(stdout, '%s copied to %s' % (ec_pr11521, test_ec))
+        self.assertTrue(os.path.exists(test_ec))
+        remove_file(test_ec)
+
+        # --copy-ec without arguments (and  no --from-pr) results in a proper error
         args = ['--copy-ec']
         error_pattern = "One of more files to copy should be specified!"
         self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, raise_error=True)