From 9ca78fc90de040ba7a41674d7f1fa0d4e161df0e Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 31 May 2018 21:57:56 +0800 Subject: [PATCH 1/4] Added a tutorial when merge conflict happens, and show the error message. --- homu/main.py | 58 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/homu/main.py b/homu/main.py index a2dbb8a..0ed3d80 100644 --- a/homu/main.py +++ b/homu/main.py @@ -833,6 +833,32 @@ def create_merge(state, repo_cfg, branch, logger, git_cfg, state.body) desc = 'Merge conflict' + comment = ( + 'This pull request and the master branch diverged in a way that cannot' + ' be automatically merged. Please rebase on top of the latest master' + ' branch, and let the reviewer approve again.\n' + '\n' + '
How do I rebase?\n\n' + 'Assuming `self` is your fork and `upstream` is this repository,' + ' you can resolve the conflict following these steps:\n\n' + '1. `git checkout {branch}` *(switch to your branch)*\n' + '2. `git fetch upstream master` *(retrieve the latest master)*\n' + '3. `git rebase upstream/master -p` *(rebase on top of it)*\n' + '4. Follow the on-screen instruction to resolve conflicts' + ' (check `git status` if you got lost).\n' + '5. `git push self {branch} --force-with-lease` *(update this PR)*\n\n' + 'You may also read' + ' [*Git Rebasing to Resolve Conflicts* by Drew Blessing](http://blessing.io/git/git-rebase/open-source/2015/08/23/git-rebasing-to-resolve-conflicts.html)' # noqa + ' for a short tutorial.\n\n' + 'Please avoid the ["**Resolve conflicts**" button](https://help.github.com/articles/resolving-a-merge-conflict-on-github/) on GitHub.' #noqa + ' It uses `git merge` instead of `git rebase` which makes the PR commit' + ' history more difficult to read.\n\n' + 'Sometimes step 4 will complete without asking for resolution. This is' + ' usually due to difference between how `Cargo.lock` conflict is' + ' handled during merge and rebase. This is normal, and you should still' + ' perform step 5 to update this PR.\n\n' + '
\n\n' + ).format(branch=state.head_ref.split(':', 1)[1]) if git_cfg['local_git']: @@ -861,6 +887,7 @@ def create_merge(state, repo_cfg, branch, logger, git_cfg, utils.silent_call(git_cmd('rebase', '--abort')) if utils.silent_call(git_cmd('rebase', base_sha)) == 0: desc = 'Auto-squashing failed' + comment = '' else: ap = '' if state.try_ else state.approved_by text = '\nCloses: #{}\nApproved by: {}'.format(state.num, ap) @@ -903,22 +930,29 @@ def create_merge(state, repo_cfg, branch, logger, git_cfg, merge_base_sha, base_sha)) except subprocess.CalledProcessError: desc = 'Auto-squashing failed' + comment = '' ok = False if ok: utils.logged_call(git_cmd('checkout', '-B', branch, base_sha)) try: - utils.logged_call(git_cmd( - '-c', - 'user.name=' + git_cfg['name'], - '-c', - 'user.email=' + git_cfg['email'], - 'merge', - 'heads/homu-tmp', - '--no-ff', - '-m', - merge_msg)) - except subprocess.CalledProcessError: + subprocess.check_output( + git_cmd( + '-c', + 'user.name=' + git_cfg['name'], + '-c', + 'user.email=' + git_cfg['email'], + 'merge', + 'heads/homu-tmp', + '--no-ff', + '-m', + merge_msg), + stderr=subprocess.STDOUT, + universal_newlines=True) + except subprocess.CalledProcessError as e: + comment += '
Error message\n\n```text\n' + comment += e.output + comment += '\n```\n\n
' pass else: if ensure_merge_equal: @@ -964,7 +998,7 @@ def create_merge(state, repo_cfg, branch, logger, git_cfg, desc, context='homu') - state.add_comment(':lock: ' + desc) + state.add_comment(':lock: {}\n\n{}'.format(desc, comment)) state.change_labels(LabelEvent.CONFLICT) return '' From 35cfa28972766954e81cad2ffa5c538545a88e13 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 31 May 2018 22:44:05 +0800 Subject: [PATCH 2/4] Do not allow `bors try` after it has been `bors r+`'ed. Fix #3. --- homu/main.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/homu/main.py b/homu/main.py index 0ed3d80..9827b38 100644 --- a/homu/main.py +++ b/homu/main.py @@ -642,6 +642,14 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, elif word in ['try', 'try-'] and realtime: if not _try_auth_verified(): continue + if state.status == '' and state.approved_by: + state.add_comment( + ':no_good: ' + 'Please do not `try` after a pull request has been `r+`ed.' + ' If you need to `try`, unapprove (`r-`) it first.' + ) + continue + state.try_ = word == 'try' state.merge_sha = '' From 486cbf855dcf753d08ff52d413029957dc0123d3 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 31 May 2018 23:00:20 +0800 Subject: [PATCH 3/4] Add "[DO NOT MERGE]" to the list of WIP keywords. --- homu/main.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/homu/main.py b/homu/main.py index 9827b38..4e151be 100644 --- a/homu/main.py +++ b/homu/main.py @@ -488,11 +488,20 @@ def parse_commands(body, username, repo_cfg, state, my_username, db, states, continue # Ignore WIP PRs - if any(map(state.title.startswith, [ - 'WIP', 'TODO', '[WIP]', '[TODO]', - ])): - if realtime: - state.add_comment(':clipboard: Looks like this PR is still in progress, ignoring approval') # noqa + is_wip = False + for wip_kw in ['WIP', 'TODO', '[WIP]', '[TODO]', '[DO NOT MERGE]']: + if state.title.upper().startswith(wip_kw): + if realtime: + state.add_comment(( + ':clipboard:' + ' Looks like this PR is still in progress,' + ' ignoring approval.\n\n' + 'Hint: Remove **{}** from this PR\'s title when' + ' it is ready for review.' + ).format(wip_kw)) + is_wip = True + break + if is_wip: continue # Sometimes, GitHub sends the head SHA of a PR as 0000000 From 3432e1183bb0604bee418ee91e373ab7eebedf6d Mon Sep 17 00:00:00 2001 From: kennytm Date: Fri, 1 Jun 2018 00:39:21 +0800 Subject: [PATCH 4/4] Do not let the timer and PullReqState form an RC cycle. Sometimes the RC cycle will cause spurious "Test timed out" messages even when the PR has been merged. --- homu/main.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/homu/main.py b/homu/main.py index 4e151be..022e316 100644 --- a/homu/main.py +++ b/homu/main.py @@ -22,6 +22,7 @@ from .git_helper import SSH_KEY_FILE import shlex import random +import weakref STATUS_TO_PRIORITY = { 'success': 0, @@ -343,7 +344,13 @@ def blocked_by_closed_tree(self): def start_testing(self, timeout): self.test_started = time.time() # FIXME: Save in the local database self.set_status('pending') - timer = Timer(timeout, self.timed_out) + + wm = weakref.WeakMethod(self.timed_out) + def timed_out(): + m = wm() + if m: + m() + timer = Timer(timeout, timed_out) timer.start() self.timeout_timer = timer