[IMP] runbot_merge: sanity check when rebasing

Ensure that the commits we're creating are based on the commit we're
expecting.

This is the second cause (and really the biggest issue) of the "Great
Reset" of master on November 6: a previous commit explains the issue
with non-linear github operations (update a branch, get the branch
head, they don't match).

The second issue is why @awa-odoo's PR was merged with a reversion of
@tivisse's as part of its first commit.

The stage for this issues is based on the incoherence noted above:
having updated a branch, getting that branch's head afterward may
still return the old head. However either delays allow that update to
be visible *or* different operations can have different views of the
system. Regardless it's possible that `repos/merges` "sees" a
different branch head than a `git/refs/heads` which preceded it by a
few milliseconds. This is an issue because github's API does not
provide a generic "rebase" operation, and the operation is thus
implemented by hand:

1. get the head of the branch we're trying to rebase a PR on
2. for each commit of the PR (oldest to newest), *merge* commit on the
   base and associate the merge commit with the original
3. reset the branch to the head we stored previously
4. for each commit of the PR, create a new commit with:

   - the metadata of the original
   - the tree of the merge commit
   - the "current head" as parent

   then update the "current head" to that commit's ref'

If the head fetched at (1) and the one the first merge of (2) sees are
different, the first commit created during (4) will look like it has
not only its own changes but also all the changes between the two
heads, as github records not changes but snapshots of working
copies (that's what a git tree is, a complete snapshot of the entire
state of a working copy).

As a result, we end up not only with commits from a previous staging
but the first commit of the next PR rollbacks the changes of those
commits, making a mess of the entire thing twice over. And because the
commits of the previous staging get reverted, even if there was a good
reason for them to fail (not the case here it was a false positive)
the new staging might just go through.

As noted at the start, mitigate that by asserting that the merge
commits created at (2) have the "base parent" (left parent / parent
from the base branch) we were expecting, and cancel the staging if
that's not the case.

This can probably be removed if / when odoo/runbot#247 happens.

runbot_merge/github.py

@@ -4,6 +4,7 @@ import json as json_
 import logging
 
 import requests
+import werkzeug.exceptions
 
 from odoo.tools import topological_sort
 from . import exceptions, utils
@@ -67,7 +68,10 @@ class GH(object):
         return r.json()
 
     def head(self, branch):
-        d = self('get', 'git/refs/heads/{}'.format(branch)).json()
+        d = utils.backoff(
+            lambda: self('get', 'git/refs/heads/{}'.format(branch)).json(),
+            exc=requests.HTTPError
+        )
 
         assert d['ref'] == 'refs/heads/{}'.format(branch)
         assert d['object']['type'] == 'commit'
@@ -114,7 +118,12 @@ class GH(object):
         """
         :return: nothing if successful, the incorrect HEAD otherwise
         """
-        head = self.head(branch)
+        r = self('get', 'git/refs/heads/{}'.format(branch), check=False)
+        if r.status_code == 200:
+            head = r.json()['object']['sha']
+        else:
+            head = '<Response [%s]: %s)>' % (r.status_code, r.json() if _is_json(r) else r.text)
+
         if head == to:
             _logger.info("Sanity check ref update of %s to %s: ok", branch, to)
             return
@@ -198,7 +207,7 @@ class GH(object):
             self._repo, dest, r['parents'][0]['sha'],
             shorten(message), r['sha']
         )
-        return dict(r['commit'], sha=r['sha'])
+        return dict(r['commit'], sha=r['sha'], parents=r['parents'])
 
     def rebase(self, pr, dest, reset=False, commits=None):
         """ Rebase pr's commits on top of dest, updates dest unless ``reset``
@@ -215,10 +224,23 @@ class GH(object):
                      self._repo, pr, dest, reset, len(commits))
 
         assert commits, "can't rebase a PR with no commits"
-        for c in commits:
-            assert len(c['parents']) == 1, "can't rebase commits with more than one parent"
-            tmp_msg = 'temp rebasing PR %s (%s)' % (pr, c['sha'])
-            c['new_tree'] = self.merge(c['sha'], dest, tmp_msg)['tree']['sha']
+        prev = original_head
+        for original in commits:
+            assert len(original['parents']) == 1, "can't rebase commits with more than one parent"
+            tmp_msg = 'temp rebasing PR %s (%s)' % (pr, original['sha'])
+            merged = self.merge(original['sha'], dest, tmp_msg)
+
+            # whichever parent is not original['sha'] should be what dest
+            # deref'd to, and we want to check that matches the "left parent" we
+            # expect (either original_head or the previously merged commit)
+            [base_commit] = (parent['sha'] for parent in merged['parents']
+                             if parent['sha'] != original['sha'])
+            assert prev == base_commit,\
+                "Inconsistent view of %s between head (%s) and merge (%s)" % (
+                    dest, prev, base_commit,
+                )
+            prev = merged['sha']
+            original['new_tree'] = merged['tree']['sha']
 
         prev = original_head
         mapping = {}