Merge pull request #5 from kennytm/etc-updates

Various updates.

Author: kennytm

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'
+        '<details><summary>How do I rebase?</summary>\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'
+        '</details>\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 = '<try>' 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 += '<details><summary>Error message</summary>\n\n```text\n'
+                    comment += e.output
+                    comment += '\n```\n\n</details>'
                     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 ''