Merge pull request #3707 from migueldiascosta/fix_from_multiple_prs

fix support for specifying multiple PRs to --from-pr

Author: boegel

easybuild/framework/easyconfig/tools.py

@@ -307,7 +307,7 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None):
     return paths
 
 
-def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_pr=False):
+def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None):
     """Obtain alternative paths for easyconfig files."""
 
     # paths where tweaked easyconfigs will be placed, easyconfigs listed on the command line take priority and will be
@@ -318,12 +318,13 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_pr=False):
         tweaked_ecs_paths = (os.path.join(tmpdir, 'tweaked_easyconfigs'),
                              os.path.join(tmpdir, 'tweaked_dep_easyconfigs'))
 
-    # 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" % '_'.join(str(pr) for pr in from_pr))
+    # paths where files touched in PRs will be downloaded to,
+    # which are picked up via 'pr_paths' build option in fetch_files_from_pr
+    pr_paths = None
+    if from_prs:
+        pr_paths = [os.path.join(tmpdir, 'files_pr%s' % pr) for pr in from_prs]
 
-    return tweaked_ecs_paths, pr_path
+    return tweaked_ecs_paths, pr_paths
 
 
 def det_easyconfig_paths(orig_paths):
@@ -333,7 +334,7 @@ def det_easyconfig_paths(orig_paths):
     :return: list of paths to easyconfig files
     """
     try:
-        from_pr_list = [int(pr_nr) for pr_nr in build_option('from_pr')]
+        from_prs = [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.")
 
@@ -342,9 +343,11 @@ def det_easyconfig_paths(orig_paths):
     # list of specified easyconfig files
     ec_files = orig_paths[:]
 
-    if from_pr_list is not None:
+    if from_prs:
         pr_files = []
-        for pr in from_pr_list:
+        for pr in from_prs:
+            # path to where easyconfig files should be downloaded is determined via 'pr_paths' build option,
+            # which corresponds to the list of PR paths returned by alt_easyconfig_paths
             pr_files.extend(fetch_easyconfigs_from_pr(pr))
 
         if ec_files:

easybuild/tools/config.py

@@ -346,7 +346,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
         'build_specs',
         'command_line',
         'external_modules_metadata',
-        'pr_path',
+        'pr_paths',
         'robot_path',
         'valid_module_classes',
         'valid_stops',

easybuild/tools/github.py

@@ -447,7 +447,15 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_account=None, gi
 
     if path is None:
         if github_repo == GITHUB_EASYCONFIGS_REPO:
-            path = build_option('pr_path')
+            pr_paths = build_option('pr_paths')
+            if pr_paths:
+                # figure out directory for this specific PR (see also alt_easyconfig_paths)
+                cands = [p for p in pr_paths if p.endswith('files_pr%s' % pr)]
+                if len(cands) == 1:
+                    path = cands[0]
+                else:
+                    raise EasyBuildError("Failed to isolate path for PR #%s from list of PR paths: %s", pr, pr_paths)
+
         elif github_repo == GITHUB_EASYBLOCKS_REPO:
             path = os.path.join(tempfile.gettempdir(), 'ebs_pr%s' % pr)
         else:

easybuild/tools/options.py

@@ -1462,19 +1462,19 @@ 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
+    # map list of strings --from-pr value to list of integers
     try:
-        from_pr_list = [int(pr_nr) for pr_nr in eb_go.options.from_pr]
+        from_prs = [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=from_pr_list)
+    tweaked_ecs_paths, pr_paths = alt_easyconfig_paths(tmpdir, tweaked_ecs=tweaked_ecs, from_prs=from_prs)
     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)
+    robot_path = det_robot_path(options.robot_paths, tweaked_ecs_paths, pr_paths, auto_robot=auto_robot)
+    log.debug("Full robot path: %s", robot_path)
 
     if not robot_path:
         print_warning("Robot search path is empty!")
@@ -1487,7 +1487,7 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
         'build_specs': build_specs,
         'command_line': eb_cmd_line,
         'external_modules_metadata': parse_external_modules_metadata(options.external_modules_metadata),
-        'pr_path': pr_path,
+        'pr_paths': pr_paths,
         'robot_path': robot_path,
         'silent': testing or new_update_opt,
         'try_to_generate': try_to_generate,
@@ -1544,7 +1544,7 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
     sys.path.insert(0, new_fake_vsc_path)
 
     return eb_go, (build_specs, log, logfile, robot_path, search_query, tmpdir, try_to_generate,
-                   from_pr_list, tweaked_ecs_paths)
+                   from_prs, tweaked_ecs_paths)
 
 
 def process_software_build_specs(options):

easybuild/tools/robot.py

@@ -52,10 +52,10 @@
 _log = fancylogger.getLogger('tools.robot', fname=False)
 
 
-def det_robot_path(robot_paths_option, tweaked_ecs_paths, pr_path, auto_robot=False):
+def det_robot_path(robot_paths_option, tweaked_ecs_paths, pr_paths, auto_robot=False):
     """Determine robot path."""
     robot_path = robot_paths_option[:]
-    _log.info("Using robot path(s): %s" % robot_path)
+    _log.info("Using robot path(s): %s", robot_path)
 
     tweaked_ecs_path, tweaked_ecs_deps_path = None, None
     # paths to tweaked easyconfigs or easyconfigs downloaded from a PR have priority
@@ -67,9 +67,10 @@ def det_robot_path(robot_paths_option, tweaked_ecs_paths, pr_path, auto_robot=Fa
         robot_path.append(tweaked_ecs_deps_path)
         _log.info("Prepended list of robot search paths with %s and appended with %s: %s", tweaked_ecs_path,
                   tweaked_ecs_deps_path, robot_path)
-    if pr_path is not None:
-        robot_path.append(pr_path)
-        _log.info("Appended list of robot search paths with %s: %s" % (pr_path, robot_path))
+
+    if pr_paths is not None:
+        robot_path.extend(pr_paths)
+        _log.info("Extended list of robot search paths with %s: %s", pr_paths, robot_path)
 
     return robot_path
 

test/framework/options.py

@@ -1728,7 +1728,7 @@ def test_from_pr(self):
             self.assertTrue(sorted(regex.findall(outtxt)), sorted(modules))
 
             pr_tmpdir = os.path.join(tmpdir, r'eb-\S{6,8}', 'files_pr6424')
-            regex = re.compile("Appended list of robot search paths with %s:" % pr_tmpdir, re.M)
+            regex = re.compile(r"Extended 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)
@@ -1762,9 +1762,11 @@ def test_from_pr(self):
             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))
+            for pr in ('12150', '12366'):
+                pr_tmpdir = os.path.join(tmpdir, r'eb-\S{6,8}', 'files_pr%s' % pr)
+                regex = re.compile(r"Extended 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)

test/framework/robot.py

@@ -1065,8 +1065,8 @@ def test_tweak_robotpath(self):
         test_easyconfigs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs')
 
         # Create directories to store the tweaked easyconfigs
-        tweaked_ecs_paths, pr_path = alt_easyconfig_paths(self.test_prefix, tweaked_ecs=True)
-        robot_path = det_robot_path([test_easyconfigs], tweaked_ecs_paths, pr_path, auto_robot=True)
+        tweaked_ecs_paths, pr_paths = alt_easyconfig_paths(self.test_prefix, tweaked_ecs=True)
+        robot_path = det_robot_path([test_easyconfigs], tweaked_ecs_paths, pr_paths, auto_robot=True)
 
         init_config(build_options={
             'valid_module_classes': module_classes(),