diff --git a/easybuild/main.py b/easybuild/main.py index 0dcd3834c8..ee865bf75c 100644 --- 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 index 0a41503fca..848d964d5b 100644 --- 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,19 +1398,39 @@ 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 = 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") @@ -1379,13 +1438,7 @@ def update_pr(pr, paths, ecs, commit_msg=None): 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 = 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) @@ -1393,7 +1446,7 @@ def update_pr(pr, paths, ecs, commit_msg=None): 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 index 9059082d7c..4693dd1040 100644 --- 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 index 13ed6efd6b..657f8eeaa0 100644 --- 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 index a270c8babe..c01bb19a3f 100644 --- 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: