Merge pull request easybuilders#2631 from boegel/fix_from_pr

make --from-pr always try to apply PR patch on top of PR target branch

Author: migueldiascosta

easybuild/tools/github.py

@@ -46,12 +46,10 @@
 from vsc.utils.missing import nub
 
 from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs, copy_patch_files, process_easyconfig
-from easybuild.framework.easyconfig.format.one import EB_FORMAT_EXTENSION
-from easybuild.framework.easyconfig.format.yeb import YEB_FORMAT_EXTENSION
 from easybuild.tools.build_log import EasyBuildError, print_msg, print_warning
 from easybuild.tools.config import build_option
 from easybuild.tools.filetools import apply_patch, copy_dir, det_patched_files, download_file, extract_file
-from easybuild.tools.filetools import mkdir, read_file, which, write_file
+from easybuild.tools.filetools import mkdir, read_file, symlink, which, write_file
 from easybuild.tools.systemtools import UNKNOWN, get_tool_version
 from easybuild.tools.utilities import only_if_module_is_available
 
@@ -368,87 +366,108 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
         # make sure path exists, create it if necessary
         mkdir(path, parents=True)
 
-    _log.debug("Fetching easyconfigs from PR #%s into %s" % (pr, path))
-    pr_url = lambda g: g.repos[GITHUB_EB_MAIN][GITHUB_EASYCONFIGS_REPO].pulls[pr]
+    github_account = build_option('pr_target_account')
+    github_repo = GITHUB_EASYCONFIGS_REPO
+
+    def pr_url(gh):
+        """Utility function to fetch data for a specific PR."""
+        return gh.repos[github_account][github_repo].pulls[pr]
+
+    _log.debug("Fetching easyconfigs from %s/%s PR #%s into %s", github_account, github_repo, pr, path)
 
     status, pr_data = github_api_get_request(pr_url, github_user)
     if status != HTTP_STATUS_OK:
         raise EasyBuildError("Failed to get data for PR #%d from %s/%s (status: %d %s)",
-                             pr, GITHUB_EB_MAIN, GITHUB_EASYCONFIGS_REPO, status, pr_data)
-
-    # if PR is open and mergable, download develop and patch
-    stable = pr_data['mergeable_state'] == GITHUB_MERGEABLE_STATE_CLEAN
-    closed = pr_data['state'] == GITHUB_STATE_CLOSED and not pr_data['merged']
+                             pr, github_account, github_repo, status, pr_data)
 
-    # 'clean' on successful (or missing) test, 'unstable' on failed tests or merge conflict
-    if not stable:
-        _log.warning("Mergeable state for PR #%d is not '%s': %s.",
-                     pr, GITHUB_MERGEABLE_STATE_CLEAN, pr_data['mergeable_state'])
+    pr_merged = pr_data['merged']
+    pr_closed = pr_data['state'] == GITHUB_STATE_CLOSED and not pr_merged
 
-    if (stable or pr_data['merged']) and not closed:
-        # whether merged or not, download develop
-        final_path = download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='develop', github_user=github_user)
+    pr_target_branch = pr_data['base']['ref']
+    _log.info("Target branch for PR #%s: %s", pr, pr_target_branch)
 
-    else:
-        final_path = path
+    # download target branch of PR so we can try and apply the PR patch on top of it
+    repo_target_branch = download_repo(repo=github_repo, account=github_account, branch=pr_target_branch,
+                                       github_user=github_user)
 
     # determine list of changed files via diff
     diff_fn = os.path.basename(pr_data['diff_url'])
-    diff_filepath = os.path.join(final_path, diff_fn)
+    diff_filepath = os.path.join(path, diff_fn)
     download_file(diff_fn, pr_data['diff_url'], diff_filepath, forced=True)
     diff_txt = read_file(diff_filepath)
 
     patched_files = det_patched_files(txt=diff_txt, omit_ab_prefix=True, github=True, filter_deleted=True)
     _log.debug("List of patched files: %s" % patched_files)
 
     for key, val in sorted(pr_data.items()):
-        _log.debug("\n%s:\n\n%s\n" % (key, val))
+        _log.debug("\n%s:\n\n%s\n", key, val)
 
     # obtain last commit
     # get all commits, increase to (max of) 100 per page
     if pr_data['commits'] > GITHUB_MAX_PER_PAGE:
-        raise EasyBuildError("PR #%s contains more than %s commits, can't obtain last commit", pr, GITHUB_MAX_PER_PAGE)
-    status, commits_data = github_api_get_request(lambda g: pr_url(g).commits, github_user,
+        raise EasyBuildError("PR #%s contains more than %s commits, can't obtain last commit",
+                             pr, GITHUB_MAX_PER_PAGE)
+
+    status, commits_data = github_api_get_request(lambda gh: pr_url(gh).commits, github_user,
                                                   per_page=GITHUB_MAX_PER_PAGE)
     if status != HTTP_STATUS_OK:
         raise EasyBuildError("Failed to get data for PR #%d from %s/%s (status: %d %s)",
-                             pr, GITHUB_EB_MAIN, GITHUB_EASYCONFIGS_REPO, status, commits_data)
+                             pr, github_account, github_repo, status, commits_data)
     last_commit = commits_data[-1]
-    _log.debug("Commits: %s, last commit: %s" % (commits_data, last_commit['sha']))
-
-    if not(pr_data['merged']):
-        if not stable or closed:
-            state_msg = "unstable (pending/failed tests or merge conflict)" if not stable else "closed (but not merged)"
-            print "\n*** WARNING: Using easyconfigs from %s PR #%s ***\n" % (state_msg, pr)
-            # obtain most recent version of patched files
-            for patched_file in patched_files:
-                # path to patch file, incl. subdir it is in
-                fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:])
-                sha = last_commit['sha']
-                full_url = URL_SEPARATOR.join([GITHUB_RAW, GITHUB_EB_MAIN, GITHUB_EASYCONFIGS_REPO, sha, patched_file])
-                _log.info("Downloading %s from %s" % (fn, full_url))
-                download_file(fn, full_url, path=os.path.join(path, fn), forced=True)
-        else:
-            apply_patch(diff_filepath, final_path, level=1)
+    _log.debug("Commits: %s, last commit: %s", commits_data, last_commit['sha'])
+
+    final_path = None
 
+    # try to apply PR patch on top of target branch, unless the PR is closed or already merged
+    if pr_merged:
+        _log.info("PR is already merged, so using current version of PR target branch")
+        final_path = repo_target_branch
+
+    elif not pr_closed:
+        try:
+            _log.debug("Trying to apply PR patch %s to %s...", diff_filepath, repo_target_branch)
+            apply_patch(diff_filepath, repo_target_branch, level=1)
+            _log.info("Using %s which included PR patch to test PR #%s", repo_target_branch, pr)
+            final_path = repo_target_branch
+
+        except EasyBuildError as err:
+            _log.warning("Ignoring problem that occured when applying PR patch: %s", err)
+
+    if final_path is None:
+
+        if pr_closed:
+            print_warning("Using easyconfigs from closed PR #%s" % pr)
+
+        # obtain most recent version of patched files
+        for patched_file in patched_files:
+            # path to patch file, incl. subdir it is in
+            fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:])
+            sha = last_commit['sha']
+            full_url = URL_SEPARATOR.join([GITHUB_RAW, github_account, github_repo, sha, patched_file])
+            _log.info("Downloading %s from %s", fn, full_url)
+            download_file(fn, full_url, path=os.path.join(path, fn), forced=True)
+
+        final_path = path
+
+    # symlink directories into expected place if they're not there yet
     if final_path != path:
         dirpath = os.path.join(final_path, 'easybuild', 'easyconfigs')
         for eb_dir in os.listdir(dirpath):
-            os.symlink(os.path.join(dirpath, eb_dir), os.path.join(path, os.path.basename(eb_dir)))
+            symlink(os.path.join(dirpath, eb_dir), os.path.join(path, os.path.basename(eb_dir)))
 
     # sanity check: make sure all patched files are downloaded
     ec_files = []
     for patched_file in [f for f in patched_files if not f.startswith('test/')]:
         fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:])
-        if os.path.exists(os.path.join(path, fn)):
-            ec_files.append(os.path.join(path, fn))
+        full_path = os.path.join(path, fn)
+        if os.path.exists(full_path):
+            ec_files.append(full_path)
         else:
-            raise EasyBuildError("Couldn't find path to patched file %s", os.path.join(path, fn))
+            raise EasyBuildError("Couldn't find path to patched file %s", full_path)
 
     return ec_files
 
 
-
 def create_gist(txt, fn, descr=None, github_user=None):
     """Create a gist with the provided text."""
     if descr is None:

test/framework/github.py

@@ -120,6 +120,10 @@ def test_fetch_easyconfigs_from_pr(self):
             print "Skipping test_fetch_easyconfigs_from_pr, no GitHub token available?"
             return
 
+        init_config(build_options={
+            'pr_target_account': gh.GITHUB_EB_MAIN,
+        })
+
         # PR for rename of ffmpeg to FFmpeg,
         # see https://github.com/easybuilders/easybuild-easyconfigs/pull/2481/files
         all_ecs_pr2481 = [