Merge pull request #3605 from migueldiascosta/from_pr_multiple_prs

add support for multiple PRs in --from-pr

Author: boegel

easybuild/framework/easyconfig/tools.py

@@ -321,7 +321,7 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_pr=False):
     # path where files touched in PR will be downloaded to
     pr_path = None
     if from_pr:
-        pr_path = os.path.join(tmpdir, "files_pr%s" % from_pr)
+        pr_path = os.path.join(tmpdir, "files_pr%s" % '_'.join(str(pr) for pr in from_pr))
 
     return tweaked_ecs_paths, pr_path
 
@@ -332,14 +332,20 @@ def det_easyconfig_paths(orig_paths):
     :param orig_paths: list of original easyconfig paths
     :return: list of paths to easyconfig files
     """
-    from_pr = build_option('from_pr')
+    try:
+        from_pr_list = [int(pr_nr) for pr_nr in build_option('from_pr')]
+    except ValueError:
+        raise EasyBuildError("Argument to --from-pr must be a comma separated list of PR #s.")
+
     robot_path = build_option('robot_path')
 
     # list of specified easyconfig files
     ec_files = orig_paths[:]
 
-    if from_pr is not None:
-        pr_files = fetch_easyconfigs_from_pr(from_pr)
+    if from_pr_list is not None:
+        pr_files = []
+        for pr in from_pr_list:
+            pr_files.extend(fetch_easyconfigs_from_pr(pr))
 
         if ec_files:
             # replace paths for specified easyconfigs that are touched in PR
@@ -725,6 +731,9 @@ def avail_easyblocks():
 def det_copy_ec_specs(orig_paths, from_pr):
     """Determine list of paths + target directory for --copy-ec."""
 
+    if from_pr is not None and not isinstance(from_pr, list):
+        from_pr = [from_pr]
+
     target_path, paths = None, []
 
     # if only one argument is specified, use current directory as target directory
@@ -746,8 +755,10 @@ def det_copy_ec_specs(orig_paths, from_pr):
         # 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)
+        tmpdir = os.path.join(tempfile.gettempdir(), 'fetch_files_from_pr_%s' % '_'.join(str(pr) for pr in from_pr))
+        pr_paths = []
+        for pr in from_pr:
+            pr_paths.extend(fetch_files_from_pr(pr=pr, path=tmpdir))
 
         # assume that files need to be copied to current working directory for now
         target_path = os.getcwd()

easybuild/main.py

@@ -207,7 +207,8 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     options, orig_paths = eb_go.options, eb_go.args
 
     global _log
-    (build_specs, _log, logfile, robot_path, search_query, eb_tmpdir, try_to_generate, tweaked_ecs_paths) = cfg_settings
+    (build_specs, _log, logfile, robot_path, search_query, eb_tmpdir, try_to_generate,
+     from_pr_list, tweaked_ecs_paths) = cfg_settings
 
     # load hook implementations (if any)
     hooks = load_hooks(options.hooks)
@@ -318,7 +319,7 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=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)
+        orig_paths, target_path = det_copy_ec_specs(orig_paths, from_pr_list)
 
     categorized_paths = categorize_files_by_type(orig_paths)
 

easybuild/tools/filetools.py

@@ -298,11 +298,19 @@ def symlink(source_path, symlink_path, use_abspath_source=True):
     if use_abspath_source:
         source_path = os.path.abspath(source_path)
 
-    try:
-        os.symlink(source_path, symlink_path)
-        _log.info("Symlinked %s to %s", source_path, symlink_path)
-    except OSError as err:
-        raise EasyBuildError("Symlinking %s to %s failed: %s", source_path, symlink_path, err)
+    if os.path.exists(symlink_path):
+        abs_source_path = os.path.abspath(source_path)
+        symlink_target_path = os.path.abspath(os.readlink(symlink_path))
+        if abs_source_path != symlink_target_path:
+            raise EasyBuildError("Trying to symlink %s to %s, but the symlink already exists and points to %s.",
+                                 source_path, symlink_path, symlink_target_path)
+        _log.info("Skipping symlinking %s to %s, link already exists", source_path, symlink_path)
+    else:
+        try:
+            os.symlink(source_path, symlink_path)
+            _log.info("Symlinked %s to %s", source_path, symlink_path)
+        except OSError as err:
+            raise EasyBuildError("Symlinking %s to %s failed: %s", source_path, symlink_path, err)
 
 
 def remove_file(path):

easybuild/tools/options.py

@@ -655,7 +655,7 @@ def github_options(self):
             'check-style': ("Run a style check on the given easyconfigs", None, 'store_true', False),
             'cleanup-easyconfigs': ("Clean up easyconfig files for pull request", None, 'store_true', True),
             'dump-test-report': ("Dump test report to specified path", None, 'store_or_None', 'test_report.md'),
-            'from-pr': ("Obtain easyconfigs from specified PR", int, 'store', None, {'metavar': 'PR#'}),
+            'from-pr': ("Obtain easyconfigs from specified PR", 'strlist', 'store', [], {'metavar': 'PR#'}),
             'git-working-dirs-path': ("Path to Git working directories for EasyBuild repositories", str, 'store', None),
             'github-user': ("GitHub username", str, 'store', None),
             'github-org': ("GitHub organization", str, 'store', None),
@@ -1459,10 +1459,16 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
     # software name/version, toolchain name/version, extra patches, ...
     (try_to_generate, build_specs) = process_software_build_specs(options)
 
+    # map --from-pr strlist to list of ints
+    try:
+        from_pr_list = [int(pr_nr) for pr_nr in eb_go.options.from_pr]
+    except ValueError:
+        raise EasyBuildError("Argument to --from-pr must be a comma separated list of PR #s.")
+
     # determine robot path
     # --try-X, --dep-graph, --search use robot path for searching, so enable it with path of installed easyconfigs
     tweaked_ecs = try_to_generate and build_specs
-    tweaked_ecs_paths, pr_path = alt_easyconfig_paths(tmpdir, tweaked_ecs=tweaked_ecs, from_pr=options.from_pr)
+    tweaked_ecs_paths, pr_path = alt_easyconfig_paths(tmpdir, tweaked_ecs=tweaked_ecs, from_pr=from_pr_list)
     auto_robot = try_to_generate or options.check_conflicts or options.dep_graph or search_query
     robot_path = det_robot_path(options.robot_paths, tweaked_ecs_paths, pr_path, auto_robot=auto_robot)
     log.debug("Full robot path: %s" % robot_path)
@@ -1491,7 +1497,7 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
     # done here instead of in _postprocess_include because github integration requires build_options to be initialized
     if eb_go.options.include_easyblocks_from_pr:
         try:
-            easyblock_prs = map(int, eb_go.options.include_easyblocks_from_pr)
+            easyblock_prs = [int(pr_nr) for pr_nr in eb_go.options.include_easyblocks_from_pr]
         except ValueError:
             raise EasyBuildError("Argument to --include-easyblocks-from-pr must be a comma separated list of PR #s.")
 
@@ -1534,7 +1540,8 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
     sys.path.remove(fake_vsc_path)
     sys.path.insert(0, new_fake_vsc_path)
 
-    return eb_go, (build_specs, log, logfile, robot_path, search_query, tmpdir, try_to_generate, tweaked_ecs_paths)
+    return eb_go, (build_specs, log, logfile, robot_path, search_query, tmpdir, try_to_generate,
+                   from_pr_list, tweaked_ecs_paths)
 
 
 def process_software_build_specs(options):

easybuild/tools/testing.py

@@ -138,7 +138,7 @@ def session_state():
     }
 
 
-def create_test_report(msg, ecs_with_res, init_session_state, pr_nr=None, gist_log=False, easyblock_pr_nrs=None):
+def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_log=False, easyblock_pr_nrs=None):
     """Create test report for easyconfigs PR, in Markdown format."""
 
     github_user = build_option('github_user')
@@ -149,10 +149,11 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nr=None, gist_l
 
     # create a gist with a full test report
     test_report = []
-    if pr_nr is not None:
+    if pr_nrs is not None:
         repo = pr_target_repo or GITHUB_EASYCONFIGS_REPO
+        pr_urls = ["https://github.com/%s/%s/pull/%s" % (pr_target_account, repo, pr_nr) for pr_nr in pr_nrs]
         test_report.extend([
-            "Test report for https://github.com/%s/%s/pull/%s" % (pr_target_account, repo, pr_nr),
+            "Test report for %s" % ', '.join(pr_urls),
             "",
         ])
     if easyblock_pr_nrs:
@@ -190,10 +191,13 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nr=None, gist_l
                 logtxt = read_file(ec_res['log_file'])
                 partial_log_txt = '\n'.join(logtxt.split('\n')[-500:])
                 descr = "(partial) EasyBuild log for failed build of %s" % ec['spec']
-                if pr_nr is not None:
-                    descr += " (PR #%s)" % pr_nr
+
+                if pr_nrs is not None:
+                    descr += " (PR #%s)" % ', #'.join(pr_nrs)
+
                 if easyblock_pr_nrs:
                     descr += "".join(" (easyblock PR #%s)" % nr for nr in easyblock_pr_nrs)
+
                 fn = '%s_partial.log' % os.path.basename(ec['spec'])[:-3]
                 gist_url = create_gist(partial_log_txt, fn, descr=descr, github_user=github_user)
                 test_log = "(partial log available at %s)" % gist_url
@@ -291,7 +295,7 @@ def post_pr_test_report(pr_nr, repo_type, test_report, msg, init_session_state,
 
     if build_option('include_easyblocks_from_pr'):
         if repo_type == GITHUB_EASYCONFIGS_REPO:
-            easyblocks_pr_nrs = map(int, build_option('include_easyblocks_from_pr'))
+            easyblocks_pr_nrs = [int(pr_nr) for pr_nr in build_option('include_easyblocks_from_pr')]
             comment_lines.append("Using easyblocks from PR(s) %s" %
                                  ", ".join(["https://github.com/%s/%s/pull/%s" %
                                             (pr_target_account, GITHUB_EASYBLOCKS_REPO, easyblocks_pr_nr)
@@ -327,22 +331,35 @@ def overall_test_report(ecs_with_res, orig_cnt, success, msg, init_session_state
     :param init_session_state: initial session state info to include in test report
     """
     dump_path = build_option('dump_test_report')
-    pr_nr = build_option('from_pr')
-    easyblock_pr_nrs = build_option('include_easyblocks_from_pr')
+
+    try:
+        pr_nrs = [int(pr_nr) for pr_nr in build_option('from_pr')]
+    except ValueError:
+        raise EasyBuildError("Argument to --from-pr must be a comma separated list of PR #s.")
+
+    try:
+        easyblock_pr_nrs = [int(pr_nr) for pr_nr in build_option('include_easyblocks_from_pr')]
+    except ValueError:
+        raise EasyBuildError("Argument to --include-easyblocks-from-pr must be a comma separated list of PR #s.")
+
     upload = build_option('upload_test_report')
 
     if upload:
         msg = msg + " (%d easyconfigs in total)" % orig_cnt
-        test_report = create_test_report(msg, ecs_with_res, init_session_state, pr_nr=pr_nr, gist_log=True,
+
+        test_report = create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=pr_nrs, gist_log=True,
                                          easyblock_pr_nrs=easyblock_pr_nrs)
-        if pr_nr:
+        if pr_nrs:
             # upload test report to gist and issue a comment in the PR to notify
-            txt = post_pr_test_report(pr_nr, GITHUB_EASYCONFIGS_REPO, test_report, msg, init_session_state, success)
+            for pr_nr in pr_nrs:
+                txt = post_pr_test_report(pr_nr, GITHUB_EASYCONFIGS_REPO, test_report, msg, init_session_state,
+                                          success)
         elif easyblock_pr_nrs:
             # upload test report to gist and issue a comment in the easyblocks PR to notify
-            for easyblock_pr_nr in map(int, easyblock_pr_nrs):
+            for easyblock_pr_nr in easyblock_pr_nrs:
                 txt = post_pr_test_report(easyblock_pr_nr, GITHUB_EASYBLOCKS_REPO, test_report, msg,
                                           init_session_state, success)
+
         else:
             # only upload test report as a gist
             gist_url = upload_test_report_as_gist(test_report['full'])

test/framework/filetools.py

@@ -574,6 +574,17 @@ def test_symlink_resolve_path(self):
 
         self.assertTrue(os.path.samefile(os.path.join(self.test_prefix, 'test', 'test.txt'), link))
 
+        # test symlink when it already exists and points to the same path
+        ft.symlink(test_file, link)
+
+        # test symlink when it already exists but points to a different path
+        test_file2 = os.path.join(link_dir, 'test2.txt')
+        ft.write_file(test_file, "test123")
+        self.assertErrorRegex(EasyBuildError,
+                              "Trying to symlink %s to %s, but the symlink already exists and points to %s." %
+                              (test_file2, link, test_file),
+                              ft.symlink, test_file2, link)
+
         # test resolve_path
         self.assertEqual(test_dir, ft.resolve_path(link_dir))
         self.assertEqual(os.path.join(os.path.realpath(self.test_prefix), 'test', 'test.txt'), ft.resolve_path(link))

test/framework/options.py

@@ -825,6 +825,7 @@ def test_list_easyblocks(self):
                 r'\|   \|   \|-- EB_toytoy',
                 r'\|   \|-- Toy_Extension',
                 r'\|-- ModuleRC',
+                r'\|-- PythonBundle',
                 r'\|-- Toolchain',
                 r'Extension',
                 r'\|-- ExtensionEasyBlock',
@@ -1733,6 +1734,41 @@ def test_from_pr(self):
             print("Ignoring URLError '%s' in test_from_pr" % err)
             shutil.rmtree(tmpdir)
 
+        # test with multiple prs
+        tmpdir = tempfile.mkdtemp()
+        args = [
+            # PRs for ReFrame 3.4.1 and 3.5.0
+            '--from-pr=12150,12366',
+            '--dry-run',
+            # an argument must be specified to --robot, since easybuild-easyconfigs may not be installed
+            '--robot=%s' % os.path.join(os.path.dirname(__file__), 'easyconfigs'),
+            '--unittest-file=%s' % self.logfile,
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,  # a GitHub token should be available for this user
+            '--tmpdir=%s' % tmpdir,
+        ]
+        try:
+            outtxt = self.eb_main(args, logfile=dummylogfn, raise_error=True)
+            modules = [
+                (tmpdir, 'ReFrame/3.4.1'),
+                (tmpdir, 'ReFrame/3.5.0'),
+            ]
+            for path_prefix, module in modules:
+                ec_fn = "%s.eb" % '-'.join(module.split('/'))
+                path = '.*%s' % os.path.dirname(path_prefix)
+                regex = re.compile(r"^ \* \[.\] %s.*%s \(module: %s\)$" % (path, ec_fn, module), re.M)
+                self.assertTrue(regex.search(outtxt), "Found pattern %s in %s" % (regex.pattern, outtxt))
+
+            # make sure that *only* these modules are listed, no others
+            regex = re.compile(r"^ \* \[.\] .*/(?P<filepath>.*) \(module: (?P<module>.*)\)$", re.M)
+            self.assertTrue(sorted(regex.findall(outtxt)), sorted(modules))
+
+            pr_tmpdir = os.path.join(tmpdir, r'eb-\S{6,8}', 'files_pr12150_12366')
+            regex = re.compile("Appended list of robot search paths with %s:" % pr_tmpdir, re.M)
+            self.assertTrue(regex.search(outtxt), "Found pattern %s in %s" % (regex.pattern, outtxt))
+        except URLError as err:
+            print("Ignoring URLError '%s' in test_from_pr" % err)
+            shutil.rmtree(tmpdir)
+
     def test_from_pr_token_log(self):
         """Check that --from-pr doesn't leak GitHub token in log."""
         if self.github_token is None:

test/framework/sandbox/easybuild/easyblocks/generic/pythonbundle.py

@@ -0,0 +1,40 @@
+##
+# 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 <http://www.gnu.org/licenses/>.
+##
+"""
+Dummy easyblock for Makecp.
+
+@author: Miguel Dias Costa (National University of Singapore)
+"""
+from easybuild.framework.easyblock import EasyBlock
+
+
+class PythonBundle(EasyBlock):
+    """Dummy support for bundle of modules."""
+
+    @staticmethod
+    def extra_options(extra_vars=None):
+        if extra_vars is None:
+            extra_vars = {}
+        return EasyBlock.extra_options(extra_vars)