[ADD] runbot_merge: allow overriding statuses

Adds an `override` mergebot command. The ability to override is set on
an individual per-context per-repository basis, similar to but
independent from review rights. That is, a given individual may be
able to override the status X on repository A and unable to do so on
repository B.

Overrides are stored in the same format as regular statuses, but
independent from them in order to persist them across builds.

Only PR statuses can be overridden, statuses which are overridable on
PRs would simply not be required on stagings.

An alternative to implementing this feature in the mergebot would be
to add it to individual status-generating tools on a per-need
basis.

Pros of that alternative:

* display the correct status on PRs, currently the PR will be failing
  status-wise (on github) but correct as far as the mergebot is
  concerned
* remove complexity from the mergebot

Cons of that alternative:

* each status-generating tool would have to implement some sort of ACL
  system
* each status-generating tool would have to receive & parse PR
  comments
* each status-generating tool would have to maintain per-pr state in
  order to track overrides

Some sort of helper library / framework ought make that rather easy
though. It could also be linked into the central provisioning system
thing.

Closes #376

conftest.py

@@ -777,6 +777,15 @@ class Repo:
             self.reset = reset
 MakeCommit = Repo.Commit
 ct = itertools.count()
+
+class Comment(tuple):
+    def __new__(cls, c):
+        self = super(Comment, cls).__new__(cls, (c['user']['login'], c['body']))
+        self._c = c
+        return self
+    def __getitem__(self, item):
+        return self._c[item]
+
 class PR:
     def __init__(self, repo, number):
         self.repo = repo
@@ -815,10 +824,7 @@ class PR:
     def comments(self):
         r = self.repo._session.get('https://api.github.com/repos/{}/issues/{}/comments'.format(self.repo.name, self.number))
         assert 200 <= r.status_code < 300, r.json()
-        return [
-            (c['user']['login'], c['body'])
-            for c in r.json()
-        ]
+        return [Comment(c) for c in r.json()]
 
     @property
     def ref(self):

forwardport/models/project.py

@@ -280,12 +280,12 @@ class PullRequests(models.Model):
 
         tokens = [
             token
-            for line in re.findall('^\s*[@|#]?{}:? (.*)$'.format(self.repository.project_id.fp_github_name), comment, re.MULTILINE | re.IGNORECASE)
+            for line in re.findall('^\s*[@|#]?{}:? (.*)$'.format(self.repository.project_id.fp_github_name), comment['body'] or '', re.MULTILINE | re.IGNORECASE)
             for token in line.split()
         ]
         if not tokens:
             _logger.info("found no commands in comment of %s (%s) (%s)", author.github_login, author.display_name,
-                 utils.shorten(comment, 50)
+                 utils.shorten(comment['body'] or '', 50)
             )
             return
 
@@ -323,7 +323,7 @@ class PullRequests(models.Model):
                 # don't update the root ever
                 for pr in filter(lambda p: p.parent_id, self._iter_ancestors()):
                     # only the author is delegated explicitely on the
-                    pr._parse_commands(author, merge_bot + ' r+', login)
+                    pr._parse_commands(author, {**comment, 'body': merge_bot + ' r+'}, login)
             elif token == 'close':
                 msg = "I'm sorry, @{}. I can't close this PR for you."
                 if self.source_id._pr_acl(author).is_reviewer:

runbot_merge/controllers/__init__.py

@@ -259,7 +259,7 @@ def handle_comment(env, event):
     if event['action'] != 'created':
         return "Ignored: action (%r) is not 'created'" % event['action']
 
-    return _handle_comment(env, repo, issue, author, comment)
+    return _handle_comment(env, repo, issue, event['comment'])
 
 def handle_review(env, event):
     repo = event['repository']['full_name']
@@ -272,7 +272,7 @@ def handle_review(env, event):
         return "Ignored: action (%r) is not 'submitted'" % event['action']
 
     return _handle_comment(
-        env, repo, pr, author, comment,
+        env, repo, pr, event['review'],
         target=event['pull_request']['base']['ref'])
 
 def handle_ping(env, event):
@@ -287,14 +287,14 @@ EVENTS = {
     'ping': handle_ping,
 }
 
-def _handle_comment(env, repo, issue, author, comment, target=None):
+def _handle_comment(env, repo, issue, comment, target=None):
     repository = env['runbot_merge.repository'].search([('name', '=', repo)])
-    if not repository.project_id._find_commands(comment):
+    if not repository.project_id._find_commands(comment['body'] or ''):
         return "No commands, ignoring"
 
     pr = env['runbot_merge.pull_requests']._get_or_schedule(repo, issue, target=target)
     if not pr:
         return "Unknown PR, scheduling fetch"
 
-    partner = env['res.partner'].search([('github_login', '=', author), ])
-    return pr._parse_commands(partner, comment, author)
+    partner = env['res.partner'].search([('github_login', '=', comment['user']['login'])])
+    return pr._parse_commands(partner, comment, comment['user']['login'])

runbot_merge/models/pull_requests.py

@@ -623,6 +623,7 @@ class PullRequests(models.Model):
     delegates = fields.Many2many('res.partner', help="Delegate reviewers, not intrinsically reviewers but can review this PR")
     priority = fields.Integer(default=2, index=True)
 
+    overrides = fields.Char(required=True, default='{}')
     statuses = fields.Text(compute='_compute_statuses')
     status = fields.Char(compute='_compute_statuses')
     previous_failure = fields.Char(default='{}')
@@ -702,12 +703,13 @@ class PullRequests(models.Model):
         Commits = self.env['runbot_merge.commit']
         for pr in self:
             c = Commits.search([('sha', '=', pr.head)])
-            if not (c and c.statuses):
+            st = json.loads(c.statuses or '{}')
+            statuses = {**st, **json.loads(pr.overrides)}
+            if not statuses:
                 pr.status = pr.statuses = False
                 continue
 
-            statuses = json.loads(c.statuses)
-            pr.statuses = pprint.pformat(statuses)
+            pr.statuses = pprint.pformat(st)
 
             st = 'success'
             for ci in pr.repository.status_ids._for_pr(pr):
@@ -754,21 +756,16 @@ class PullRequests(models.Model):
 
     def _parse_command(self, commandstring):
         for m in re.finditer(
-            r'(\S+?)(?:([+-])|=(\S*))?(?:\s|$)',
+            r'(\S+?)(?:([+-])|=(\S*))?(?=\s|$)',
             commandstring,
         ):
             name, flag, param = m.groups()
-            if name in ('retry', 'check'):
-                yield (name, None)
-            elif name in ('r', 'review'):
-                if flag == '+':
-                    yield ('review', True)
-                elif flag == '-':
-                    yield ('review', False)
+            if name == 'r':
+                name = 'review'
+            if flag in ('+', '-'):
+                yield name, flag == '+'
             elif name == 'delegate':
-                if flag == '+':
-                    yield ('delegate', True)
-                elif param:
+                if param:
                     yield ('delegate', [
                         p.lstrip('#@')
                         for p in param.split(',')
@@ -778,6 +775,8 @@ class PullRequests(models.Model):
                     yield ('priority', int(param))
             elif any(name == k for k, _ in type(self).merge_method.selection):
                 yield ('method', name)
+            else:
+                yield name, param
 
     def _parse_commands(self, author, comment, login):
         """Parses a command string prefixed by Project::github_prefix.
@@ -807,18 +806,18 @@ class PullRequests(models.Model):
 
         commands = dict(
             ps
-            for m in self.repository.project_id._find_commands(comment)
+            for m in self.repository.project_id._find_commands(comment['body'] or '')
             for ps in self._parse_command(m)
         )
 
         if not commands:
             _logger.info("found no commands in comment of %s (%s) (%s)", author.github_login, author.display_name,
-                 utils.shorten(comment, 50)
+                 utils.shorten(comment['body'] or '', 50)
             )
             return 'ok'
 
         Feedback = self.env['runbot_merge.pull_requests.feedback']
-        if not is_author:
+        if not (is_author or 'override' in commands):
             # no point even parsing commands
             _logger.info("ignoring comment of %s (%s): no ACL to %s",
                           login, name, self.display_name)
@@ -931,6 +930,27 @@ class PullRequests(models.Model):
                         'pull_request': self.number,
                         'message':"Merge method set to %s" % explanation
                     })
+            elif command == 'override':
+                overridable = author.override_rights\
+                    .filtered(lambda r: r.repository_id == self.repository)\
+                    .mapped('context')
+                if param in overridable:
+                    self.overrides = json.dumps({
+                        **json.loads(self.overrides),
+                        param: {
+                            'state': 'success',
+                            'target_url': comment['html_url'],
+                            'description': f"Overridden by @{author.github_login}",
+                        },
+                    })
+                    c = self.env['runbot_merge.commit'].search([('sha', '=', self.head)])
+                    if c:
+                        c.to_check = True
+                    else:
+                        c.create({'sha': self.head, 'statuses': '{}'})
+                    ok = True
+                else:
+                    msg = "You are not allowed to do that."
 
             _logger.info(
                 "%s %s(%s) on %s by %s (%s)",
@@ -980,17 +1000,18 @@ class PullRequests(models.Model):
         failed = self.browse(())
         for pr in self:
             required = pr.repository.status_ids._for_pr(pr).mapped('context')
+            sts = {**statuses, **json.loads(pr.overrides)}
 
             success = True
             for ci in required:
-                st = state_(statuses, ci) or 'pending'
+                st = state_(sts, ci) or 'pending'
                 if st == 'success':
                     continue
 
                 success = False
                 if st in ('error', 'failure'):
                     failed |= pr
-                    pr._notify_ci_new_failure(ci, to_status(statuses.get(ci.strip(), 'pending')))
+                    pr._notify_ci_new_failure(ci, to_status(sts.get(ci.strip(), 'pending')))
             if success:
                 oldstate = pr.state
                 if oldstate == 'opened':

runbot_merge/models/res_partner.py

@@ -13,6 +13,7 @@ class Partner(models.Model):
     delegate_reviewer = fields.Many2many('runbot_merge.pull_requests')
     formatted_email = fields.Char(string="commit email", compute='_rfc5322_formatted')
     review_rights = fields.One2many('res.partner.review', 'partner_id')
+    override_rights = fields.One2many('res.partner.override', 'partner_id')
 
     def _auto_init(self):
         res = super(Partner, self)._auto_init()
@@ -72,3 +73,17 @@ class ReviewRights(models.Model):
 
     def name_search(self, name='', args=None, operator='ilike', limit=100):
         return self.search(args + [('repository_id.name', operator, name)], limit=limit).name_get()
+
+class OverrideRights(models.Model):
+    _name = 'res.partner.override'
+    _description = 'lints which the partner can override'
+
+    partner_id = fields.Many2one('res.partner', required=True, ondelete='cascade')
+    repository_id = fields.Many2one('runbot_merge.repository', required=True)
+    context = fields.Char(required=True)
+
+    def name_get(self):
+        return [
+            (r.id, f'{r.repository.name}: {r.context}')
+            for r in self
+        ]

runbot_merge/security/ir.model.access.csv

@@ -12,8 +12,11 @@ access_runbot_merge_batch_admin,Admin access to batches,model_runbot_merge_batch
 access_runbot_merge_fetch_job_admin,Admin access to fetch jobs,model_runbot_merge_fetch_job,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_pull_requests_feedback_admin,Admin access to feedback,model_runbot_merge_pull_requests_feedback,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_review_rights,Admin access to review permissions,model_res_partner_review,runbot_merge.group_admin,1,1,1,1
+access_runbot_merge_review_override,Admin access to override permissions,model_res_partner_override,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_project,User access to project,model_runbot_merge_project,base.group_user,1,0,0,0
 access_runbot_merge_repository,User access to repo,model_runbot_merge_repository,base.group_user,1,0,0,0
 access_runbot_merge_branch,User access to branches,model_runbot_merge_branch,base.group_user,1,0,0,0
 access_runbot_merge_pull_requests,User access to PR,model_runbot_merge_pull_requests,base.group_user,1,0,0,0
 access_runbot_merge_pull_requests_feedback,Users have no reason to access feedback,model_runbot_merge_pull_requests_feedback,,0,0,0,0
+access_runbot_merge_review_rights_2,Users can see partners,model_res_partner_review,base.group_user,1,0,0,0
+access_runbot_merge_review_override_2,Users can see partners,model_res_partner_override,base.group_user,1,0,0,0

runbot_merge/tests/test_oddities.py

@@ -1,3 +1,8 @@
+import json
+
+from utils import Commit
+
+
 def test_partner_merge(env):
     p_src = env['res.partner'].create({
         'name': 'kfhsf',
@@ -20,3 +25,66 @@ def test_partner_merge(env):
     assert not p_src.exists()
     assert p_dest.name == 'Partner P. Partnersson'
     assert p_dest.github_login == 'xxx'
+
+def test_override(env, project, make_repo, users, setreviewers, config):
+    """
+    Test that we can override a status on a PR:
+
+    * @mergebot override context=status
+    * target url should be the comment (?)
+    * description should be overridden by <user>
+    """
+    repo = make_repo('repo')
+    repo_id = env['runbot_merge.repository'].create({
+        'project_id': project.id,
+        'name': repo.name,
+        'status_ids': [(0, 0, {'context': 'l/int'})]
+    })
+    setreviewers(*project.repo_ids)
+    # "other" can override the lint
+    env['res.partner'].create({
+        'name': config['role_other'].get('name', 'Other'),
+        'github_login': users['other'],
+        'override_rights': [(0, 0, {
+            'repository_id': repo_id.id,
+            'context': 'l/int',
+        })]
+    })
+
+    with repo:
+        m = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')
+
+        repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change')
+        pr = repo.make_pr(target='master', title='super change', head='change')
+        pr.post_comment('hansen r+', config['role_reviewer']['token'])
+    env.run_crons()
+
+    pr_id = env['runbot_merge.pull_requests'].search([
+        ('repository.name', '=', repo.name),
+        ('number', '=', pr.number)
+    ])
+    assert pr_id.state == 'approved'
+
+    with repo:
+        pr.post_comment('hansen override=l/int', config['role_reviewer']['token'])
+    env.run_crons()
+    assert pr_id.state == 'approved'
+
+    with repo:
+        pr.post_comment('hansen override=l/int', config['role_other']['token'])
+    env.run_crons()
+    assert pr_id.state == 'ready'
+
+    comments = pr.comments
+    assert comments == [
+        (users['reviewer'], 'hansen r+'),
+        (users['reviewer'], 'hansen override=l/int'),
+        (users['user'], "I'm sorry, @{}. You are not allowed to do that.".format(users['reviewer'])),
+        (users['other'], "hansen override=l/int"),
+    ]
+    assert pr_id.statuses == '{}'
+    assert json.loads(pr_id.overrides) == {'l/int': {
+        'state': 'success',
+        'target_url': comments[-1]['html_url'],
+        'description': 'Overridden by @{}'.format(users['other']),
+    }}

runbot_merge/views/mergebot.xml

@@ -151,6 +151,7 @@
                         <group colspan="4">
                             <field name="head"/>
                             <field name="statuses"/>
+                            <field name="overrides"/>
                         </group>
                     </group>
                     <group>

runbot_merge/views/res_partner.xml

@@ -35,6 +35,9 @@
                                 </tree>
                             </field>
                         </group>
+                        <group colspan="4">
+                            <field name="override_rights" widget="many2many_tags"/>
+                        </group>
                     </group>
                     <group>
                         <group colspan="4" string="Delegate On">