Merge pull request #3087 from boegel/sync_pr_with_develop

migueldiascosta · web-flow · commit 0835e72a8049 · 2019-11-18T10:50:48.000+08:00
add support for --sync-pr-with-develop
diff --git a/easybuild/main.py b/easybuild/main.py
@@ -58,7 +58,7 @@
 from easybuild.tools.docs import list_software
 from easybuild.tools.filetools import adjust_permissions, cleanup, write_file
 from easybuild.tools.github import check_github, find_easybuild_easyconfig, install_github_token
-from easybuild.tools.github import close_pr, list_prs, new_pr, merge_pr, update_pr
+from easybuild.tools.github import close_pr, list_prs, new_pr, merge_pr, sync_pr_with_develop, update_pr
 from easybuild.tools.hooks import START, END, load_hooks, run_hook
 from easybuild.tools.modules import modules_tool
 from easybuild.tools.options import set_up_configuration, use_color
@@ -293,8 +293,8 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     categorized_paths = categorize_files_by_type(orig_paths)
 
     # command line options that do not require any easyconfigs to be specified
-    new_update_preview_pr = options.new_pr or options.update_pr or options.preview_pr
-    no_ec_opts = [options.aggregate_regtest, options.regtest, search_query, new_update_preview_pr]
+    pr_options = options.new_pr or options.preview_pr or options.sync_pr_with_develop or options.update_pr
+    no_ec_opts = [options.aggregate_regtest, options.regtest, pr_options, search_query]
 
     # determine paths to easyconfigs
     determined_paths = det_easyconfig_paths(categorized_paths['easyconfigs'])
@@ -355,7 +355,7 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     dry_run_mode = options.dry_run or options.dry_run_short or options.missing_modules
 
     # skip modules that are already installed unless forced, or unless an option is used that warrants not skipping
-    if not (forced or dry_run_mode or options.extended_dry_run or new_update_preview_pr or options.inject_checksums):
+    if not (forced or dry_run_mode or options.extended_dry_run or pr_options or options.inject_checksums):
         retained_ecs = skip_available(easyconfigs, modtool)
         if not testing:
             for skipped_ec in [ec for ec in easyconfigs if ec not in retained_ecs]:
@@ -366,26 +366,30 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
     if len(easyconfigs) > 0:
         # resolve dependencies if robot is enabled, except in dry run mode
         # one exception: deps *are* resolved with --new-pr or --update-pr when dry run mode is enabled
-        if options.robot and (not dry_run_mode or new_update_preview_pr):
+        if options.robot and (not dry_run_mode or pr_options):
             print_msg("resolving dependencies ...", log=_log, silent=testing)
             ordered_ecs = resolve_dependencies(easyconfigs, modtool)
         else:
             ordered_ecs = easyconfigs
-    elif new_update_preview_pr:
+    elif pr_options:
         ordered_ecs = None
     else:
         print_msg("No easyconfigs left to be built.", log=_log, silent=testing)
         ordered_ecs = []
 
     # creating/updating PRs
-    if new_update_preview_pr:
+    if pr_options:
         if options.new_pr:
             new_pr(categorized_paths, ordered_ecs, title=options.pr_title, descr=options.pr_descr,
                    commit_msg=options.pr_commit_msg)
         elif options.preview_pr:
             print(review_pr(paths=determined_paths, colored=use_color(options.color)))
-        else:
+        elif options.sync_pr_with_develop:
+            sync_pr_with_develop(options.sync_pr_with_develop)
+        elif options.update_pr:
             update_pr(options.update_pr, categorized_paths, ordered_ecs, commit_msg=options.pr_commit_msg)
+        else:
+            raise EasyBuildError("Unknown PR option!")
 
     # dry_run: print all easyconfigs and dependencies, and whether they are already built
     elif dry_run_mode:
diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py
@@ -85,6 +85,7 @@
 GITHUB_DIR_TYPE = u'dir'
 GITHUB_EB_MAIN = 'easybuilders'
 GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs'
+GITHUB_DEVELOP_BRANCH = 'develop'
 GITHUB_FILE_TYPE = u'file'
 GITHUB_PR_STATE_OPEN = 'open'
 GITHUB_PR_STATES = [GITHUB_PR_STATE_OPEN, 'closed', 'all']
@@ -808,35 +809,73 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_
     # commit
     git_repo.index.commit(commit_msg)
 
-    # push to GitHub
-    github_url = 'git@github.com:%s/%s.git' % (target_account, pr_target_repo)
+    push_branch_to_github(git_repo, target_account, pr_target_repo, pr_branch)
+
+    return file_info, deleted_paths, git_repo, pr_branch, diff_stat
+
+
+def create_remote(git_repo, account, repo, https=False):
+    """
+    Create remote in specified git working directory for specified account & repository.
+
+    :param git_repo: git.Repo instance to use (after init_repo & setup_repo)
+    :param account: GitHub account name
+    :param repo: repository name
+    :param https: use https:// URL rather than git@
+    """
+
+    if https:
+        github_url = 'https://github.com/%s/%s.git' % (account, repo)
+    else:
+        github_url = 'git@github.com:%s/%s.git' % (account, repo)
+
     salt = ''.join(random.choice(ascii_letters) for _ in range(5))
-    remote_name = 'github_%s_%s' % (target_account, salt)
+    remote_name = 'github_%s_%s' % (account, salt)
+
+    try:
+        remote = git_repo.create_remote(remote_name, github_url)
+    except GitCommandError as err:
+        raise EasyBuildError("Failed to create remote %s for %s: %s", remote_name, github_url, err)
+
+    return remote
+
+
+def push_branch_to_github(git_repo, target_account, target_repo, branch):
+    """
+    Push specified branch to GitHub from specified git repository.
+
+    :param git_repo: git.Repo instance to use (after init_repo & setup_repo)
+    :param target_account: GitHub account name
+    :param target_repo: repository name
+    :param branch: name of branch to push
+    """
+
+    # push to GitHub
+    remote = create_remote(git_repo, target_account, target_repo)
 
     dry_run = build_option('dry_run') or build_option('extended_dry_run')
 
-    push_branch_msg = "pushing branch '%s' to remote '%s' (%s)" % (pr_branch, remote_name, github_url)
+    github_url = 'git@github.com:%s/%s.git' % (target_account, target_repo)
+
+    push_branch_msg = "pushing branch '%s' to remote '%s' (%s)" % (branch, remote.name, github_url)
     if dry_run:
         print_msg(push_branch_msg + ' [DRY RUN]', log=_log)
     else:
         print_msg(push_branch_msg, log=_log)
         try:
-            my_remote = git_repo.create_remote(remote_name, github_url)
-            res = my_remote.push(pr_branch)
+            res = remote.push(branch)
         except GitCommandError as err:
-            raise EasyBuildError("Failed to push branch '%s' to GitHub (%s): %s", pr_branch, github_url, err)
+            raise EasyBuildError("Failed to push branch '%s' to GitHub (%s): %s", branch, github_url, err)
 
         if res:
             if res[0].ERROR & res[0].flags:
                 raise EasyBuildError("Pushing branch '%s' to remote %s (%s) failed: %s",
-                                     pr_branch, my_remote, github_url, res[0].summary)
+                                     branch, remote, github_url, res[0].summary)
             else:
-                _log.debug("Pushed branch %s to remote %s (%s): %s", pr_branch, my_remote, github_url, res[0].summary)
+                _log.debug("Pushed branch %s to remote %s (%s): %s", branch, remote, github_url, res[0].summary)
         else:
             raise EasyBuildError("Pushing branch '%s' to remote %s (%s) failed: empty result",
-                                 pr_branch, my_remote, github_url)
-
-    return file_info, deleted_paths, git_repo, pr_branch, diff_stat
+                                 branch, remote, github_url)
 
 
 def is_patch_for(patch_name, ec):
@@ -1359,41 +1398,55 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None):
                 _log.info("Failed to add labels to PR# %s: %s." % (pr, err))
 
 
+def det_account_branch_for_pr(pr_id, github_user=None):
+    """Determine account & branch corresponding to pull request with specified id."""
+
+    if github_user is None:
+        github_user = build_option('github_user')
+
+    if github_user is None:
+        raise EasyBuildError("GitHub username (--github-user) must be specified!")
+
+    pr_target_account = build_option('pr_target_account')
+    pr_target_repo = build_option('pr_target_repo')
+
+    pr_data, _ = fetch_pr_data(pr_id, pr_target_account, pr_target_repo, github_user)
+
+    # branch that corresponds with PR is supplied in form <account>:<branch_label>
+    account = pr_data['head']['label'].split(':')[0]
+    branch = ':'.join(pr_data['head']['label'].split(':')[1:])
+    github_target = '%s/%s' % (pr_target_account, pr_target_repo)
+    print_msg("Determined branch name corresponding to %s PR #%s: %s" % (github_target, pr_id, branch), log=_log)
+
+    return account, branch
+
+
 @only_if_module_is_available('git', pkgname='GitPython')
-def update_pr(pr, paths, ecs, commit_msg=None):
+def update_pr(pr_id, paths, ecs, commit_msg=None):
     """
     Update specified pull request using specified files
 
-    :param pr: ID of pull request to update
+    :param pr_id: ID of pull request to update
     :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches)
     :param ecs: list of parsed easyconfigs, incl. for dependencies (if robot is enabled)
     :param commit_msg: commit message to use
     """
-    github_user = build_option('github_user')
-    if github_user is None:
-        raise EasyBuildError("GitHub user must be specified to use --update-pr")
 
     if commit_msg is None:
         raise EasyBuildError("A meaningful commit message must be specified via --pr-commit-msg when using --update-pr")
 
     pr_target_account = build_option('pr_target_account')
     pr_target_repo = build_option('pr_target_repo')
 
-    pr_data, _ = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user)
-
-    # branch that corresponds with PR is supplied in form <account>:<branch_label>
-    account = pr_data['head']['label'].split(':')[0]
-    branch = ':'.join(pr_data['head']['label'].split(':')[1:])
-    github_target = '%s/%s' % (pr_target_account, pr_target_repo)
-    print_msg("Determined branch name corresponding to %s PR #%s: %s" % (github_target, pr, branch), log=_log)
+    account, branch = det_account_branch_for_pr(pr_id)
 
     _, _, _, _, diff_stat = _easyconfigs_pr_common(paths, ecs, start_branch=branch, pr_branch=branch,
                                                    start_account=account, commit_msg=commit_msg)
 
     print_msg("Overview of changes:\n%s\n" % diff_stat, log=_log, prefix=False)
 
     full_repo = '%s/%s' % (pr_target_account, pr_target_repo)
-    msg = "Updated %s PR #%s by pushing to branch %s/%s" % (full_repo, pr, account, branch)
+    msg = "Updated %s PR #%s by pushing to branch %s/%s" % (full_repo, pr_id, account, branch)
     if build_option('dry_run') or build_option('extended_dry_run'):
         msg += " [DRY RUN]"
     print_msg(msg, log=_log, prefix=False)
@@ -1777,3 +1830,47 @@ def reviews_url(gh):
         pr_data['reviews'] = reviews_data
 
     return pr_data, pr_url
+
+
+def sync_pr_with_develop(pr_id):
+    """Sync pull request with specified ID with current develop branch."""
+    github_user = build_option('github_user')
+    if github_user is None:
+        raise EasyBuildError("GitHub user must be specified to use --sync-pr-with-develop")
+
+    target_account = build_option('pr_target_account')
+    target_repo = build_option('pr_target_repo')
+
+    pr_account, pr_branch = det_account_branch_for_pr(pr_id)
+
+    # initialize repository
+    git_working_dir = tempfile.mkdtemp(prefix='git-working-dir')
+    git_repo = init_repo(git_working_dir, target_repo)
+
+    setup_repo(git_repo, pr_account, target_repo, pr_branch)
+
+    # pull in latest version of 'develop' branch from central repository
+    msg = "pulling latest version of '%s' branch from %s/%s..." % (target_account, target_repo, GITHUB_DEVELOP_BRANCH)
+    print_msg(msg, log=_log)
+    easybuilders_remote = create_remote(git_repo, target_account, target_repo, https=True)
+    pull_out = git_repo.git.pull(easybuilders_remote.name, GITHUB_DEVELOP_BRANCH)
+    _log.debug("Output of 'git pull %s %s': %s", easybuilders_remote.name, GITHUB_DEVELOP_BRANCH, pull_out)
+
+    # create 'develop' branch (with force if one already exists),
+    # and check it out to check git log
+    git_repo.create_head(GITHUB_DEVELOP_BRANCH, force=True).checkout()
+    git_log_develop = git_repo.git.log('-n 3')
+    _log.debug("Top of 'git log' for %s branch:\n%s", GITHUB_DEVELOP_BRANCH, git_log_develop)
+
+    # checkout PR branch, and merge develop branch in it (which will create a merge commit)
+    print_msg("merging '%s' branch into PR branch '%s'..." % (GITHUB_DEVELOP_BRANCH, pr_branch), log=_log)
+    git_repo.git.checkout(pr_branch)
+    merge_out = git_repo.git.merge(GITHUB_DEVELOP_BRANCH)
+    _log.debug("Output of 'git merge %s':\n%s", GITHUB_DEVELOP_BRANCH, merge_out)
+
+    # check git log, should show merge commit on top
+    post_merge_log = git_repo.git.log('-n 3')
+    _log.debug("Top of 'git log' after 'git merge %s':\n%s", GITHUB_DEVELOP_BRANCH, post_merge_log)
+
+    # push updated branch back to GitHub (unless we're doing a dry run)
+    return push_branch_to_github(git_repo, pr_account, target_repo, pr_branch)
diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py
@@ -605,6 +605,8 @@ def github_options(self):
             'pr-target-repo': ("Target repository for new/updating PRs", str, 'store', GITHUB_EASYCONFIGS_REPO),
             'pr-title': ("Title for new pull request created with --new-pr", str, 'store', None),
             'preview-pr': ("Preview a new pull request", None, 'store_true', False),
+            'sync-pr-with-develop': ("Sync pull request with current 'develop' branch",
+                                     int, 'store', None, {'metavar': 'PR#'}),
             'review-pr': ("Review specified pull request", int, 'store', None, {'metavar': 'PR#'}),
             'test-report-env-filter': ("Regex used to filter out variables in environment dump of test report",
                                        None, 'regex', None),
diff --git a/test/framework/github.py b/test/framework/github.py
@@ -636,6 +636,65 @@ def test_create_delete_gist(self):
         gist_id = gist_url.split('/')[-1]
         gh.delete_gist(gist_id, github_user=GITHUB_TEST_ACCOUNT, github_token=self.github_token)
 
+    def test_det_account_branch_for_pr(self):
+        """Test det_account_branch_for_pr."""
+        if self.skip_github_tests:
+            print("Skipping test_det_account_branch_for_pr, no GitHub token available?")
+            return
+
+        init_config(build_options={
+            'pr_target_account': 'easybuilders',
+            'pr_target_repo': 'easybuild-easyconfigs',
+        })
+
+        # see https://github.com/easybuilders/easybuild-easyconfigs/pull/9149
+        self.mock_stdout(True)
+        account, branch = gh.det_account_branch_for_pr(9149, github_user=GITHUB_TEST_ACCOUNT)
+        self.mock_stdout(False)
+        self.assertEqual(account, 'boegel')
+        self.assertEqual(branch, '20191017070734_new_pr_EasyBuild401')
+
+        init_config(build_options={
+            'pr_target_account': 'easybuilders',
+            'pr_target_repo': 'easybuild-framework',
+        })
+
+        # see https://github.com/easybuilders/easybuild-framework/pull/3069
+        self.mock_stdout(True)
+        account, branch = gh.det_account_branch_for_pr(3069, github_user=GITHUB_TEST_ACCOUNT)
+        self.mock_stdout(False)
+        self.assertEqual(account, 'migueldiascosta')
+        self.assertEqual(branch, 'fix_inject_checksums')
+
+    def test_push_branch_to_github(self):
+        """Test push_branch_to_github."""
+
+        build_options = {'dry_run': True}
+        init_config(build_options=build_options)
+
+        git_repo = gh.init_repo(self.test_prefix, GITHUB_REPO)
+        branch = 'test123'
+
+        self.mock_stderr(True)
+        self.mock_stdout(True)
+        gh.setup_repo(git_repo, GITHUB_USER, GITHUB_REPO, 'master')
+        git_repo.create_head(branch, force=True)
+        gh.push_branch_to_github(git_repo, GITHUB_USER, GITHUB_REPO, branch)
+        stderr = self.get_stderr()
+        stdout = self.get_stdout()
+        self.mock_stderr(True)
+        self.mock_stdout(True)
+
+        self.assertEqual(stderr, '')
+
+        github_path = '%s/%s.git' % (GITHUB_USER, GITHUB_REPO)
+        pattern = r'^' + '\n'.join([
+            r"== fetching branch 'master' from https://github.com/%s\.\.\." % github_path,
+            r"== pushing branch 'test123' to remote 'github_.*' \(git@github.com:%s\) \[DRY RUN\]" % github_path,
+        ]) + r'$'
+        regex = re.compile(pattern)
+        self.assertTrue(regex.match(stdout.strip()), "Pattern '%s' doesn't match: %s" % (regex.pattern, stdout))
+
 
 def suite():
     """ returns all the testcases in this module """
diff --git a/test/framework/options.py b/test/framework/options.py
@@ -3030,6 +3030,36 @@ def test_new_update_pr(self):
         ]
         self._assert_regexs(regexs, txt, assert_true=False)
 
+    def test_sync_pr_with_develop(self):
+        """Test use of --sync-pr-with-develop (dry run only)."""
+        if self.github_token is None:
+            print("Skipping test_sync_pr_with_develop, no GitHub token available?")
+            return
+
+        # use https://github.com/easybuilders/easybuild-easyconfigs/pull/9150,
+        # which is a PR from boegel:develop to easybuilders:develop
+        # (to sync 'develop' branch in boegel's fork with central develop branch);
+        # we need to test with a branch that is guaranteed to stay in place for the test to work,
+        # since it will actually be downloaded (only the final push to update the branch is skipped under --dry-run)
+        args = [
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+            '--sync-pr-with-develop=9150',
+            '--dry-run',
+        ]
+        txt, _ = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False)
+
+        github_path = r"boegel/easybuild-easyconfigs\.git"
+        pattern = '\n'.join([
+            r"== temporary log file in case of crash .*",
+            r"== Determined branch name corresponding to easybuilders/easybuild-easyconfigs PR #9150: develop",
+            r"== fetching branch 'develop' from https://github\.com/%s\.\.\." % github_path,
+            r"== pulling latest version of 'easybuilders' branch from easybuild-easyconfigs/develop\.\.\.",
+            r"== merging 'develop' branch into PR branch 'develop'\.\.\.",
+            r"== pushing branch 'develop' to remote '.*' \(git@github\.com:%s\) \[DRY RUN\]" % github_path,
+        ])
+        regex = re.compile(pattern)
+        self.assertTrue(regex.match(txt), "Pattern '%s' doesn't match: %s" % (regex.pattern, txt))
+
     def test_new_pr_python(self):
         """Check generated PR title for --new-pr on easyconfig that includes Python dependency."""
         if self.github_token is None:
