diff --git a/homu/main.py b/homu/main.py index a2dbb8a..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 @@ -488,11 +495,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 @@ -642,6 +658,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 = '' @@ -833,6 +857,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 +911,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 +954,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 +1022,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 ''