Syncing from upstream odoo/runbot (17.0)

conftest.py

@@ -1135,7 +1135,9 @@ def disable_hooks(self) -> typing.Iterator[None]:
         for hook in hook_urls:
             r = sess.patch(hook, json={'active': False})
             assert r.ok, r.text
+        wait_for_hook()
         yield
+        wait_for_hook()
         for hook in reversed(hook_urls):
             r = sess.patch(hook, json={'active': True})
             assert r.ok, r.text

mergebot_test_utils/saas_worker/__init__.py

@@ -3,9 +3,14 @@
 import threading
 
 import psycopg2
+import requests.sessions
 
 import odoo
-from odoo import models
+from odoo import models, fields
+
+from . import auth_util
+
+requests.sessions.Session = auth_util.SaasSession
 
 _logger = logging.getLogger(__name__)
 
@@ -49,3 +54,11 @@ def _process_jobs(cls, db_name):
         finally:
             if hasattr(t, 'dbname'):
                 del t.dbname
+
+
+class SaasCalls(models.Model):
+    _name = _description = 'saas.calls'
+
+    method = fields.Char()
+    url = fields.Char()
+    body = fields.Binary(attachment=False)

mergebot_test_utils/saas_worker/__manifest__.py

@@ -2,4 +2,7 @@
     'name': 'dummy saas_worker',
     'version': '1.0',
     'license': 'BSD-0-Clause',
+    'data': [
+        'access.xml',
+    ]
 }

mergebot_test_utils/saas_worker/access.xml

@@ -0,0 +1,10 @@
+<odoo>
+    <record id="access_calls" model="ir.model.access">
+        <field name="name">Access to saas calls</field>
+        <field name="model_id" ref="model_saas_calls"/>
+        <field name="perm_read">1</field>
+        <field name="perm_create">0</field>
+        <field name="perm_write">0</field>
+        <field name="perm_unlink">0</field>
+    </record>
+</odoo>

mergebot_test_utils/saas_worker/auth_util.py

@@ -0,0 +1,44 @@
+import io
+import threading
+import urllib.parse
+
+import requests.adapters
+import requests.auth
+import requests.sessions
+
+import odoo
+
+
+class SaasAdapter(requests.adapters.BaseAdapter):
+    def send(self, request, stream=False, timeout=None, verify=True, cert=None, proxies=None):
+        res = requests.Response()
+        res.request = request
+        res.status_code = 204
+        res.raw = io.BytesIO()
+        return res
+
+    def close(self) -> None:
+        pass
+
+
+class SaasSession(requests.sessions.Session):
+    def __init__(self):
+        super().__init__()
+        self.mount('saas://', SaasAdapter())
+
+
+class SaasAuth(requests.auth.AuthBase):
+    def __call__(self, request: requests.PreparedRequest) -> requests.PreparedRequest:
+        dbname = threading.current_thread().dbname
+        db = odoo.sql_db.db_connect(dbname)
+        with db.cursor() as cr:
+            env = odoo.api.Environment(cr, 1, {})
+            env['saas.calls'].create({
+                'method': request.method,
+                'url': request.url,
+                'body': request.body,
+            })
+        request.url = urllib.parse.urlsplit(request.url)\
+            ._replace(scheme='saas')\
+            .geturl()
+        return request

runbot_merge/__manifest__.py

@@ -1,6 +1,6 @@
 {
     'name': 'merge bot',
-    'version': '1.19',
+    'version': '1.20',
     'depends': ['contacts', 'mail', 'website'],
     'data': [
         'security/security.xml',
@@ -26,6 +26,8 @@
         'models/commands/runbot_merge.acls.csv',
         'models/commands/views.xml',
         'models/crons/validate.xml',
+        'models/crons/status_request.xml',
+        'models/crons/check_commits.xml',
     ],
     'assets': {
        'web._assets_primary_variables': [

runbot_merge/controllers/__init__.py

@@ -7,6 +7,7 @@
 from datetime import datetime, timedelta
 from typing import Callable
 
+import psycopg2.errors
 import sentry_sdk
 from werkzeug.exceptions import NotFound, UnprocessableEntity
 
@@ -74,7 +75,7 @@ def stagings_for_commits(self, **kw):
     @route('/runbot_merge/stagings/<int:staging>', auth='none', type='http', methods=['GET'])
     def prs_for_staging(self, staging):
         staging = request.env(user=1)['runbot_merge.stagings'].browse(staging).sudo()
-        if not staging.exists:
+        if not staging.exists():
             raise NotFound()
 
         return request.make_json_response(staging_dict(staging))
@@ -173,7 +174,15 @@ def _format(self, request):
 
 def handle_pr(env, event):
     pr = event['pull_request']
-    squash = pr['commits'] == 1
+    squash = False
+    check_commits = lambda _: None
+    match pr['commits']:
+        case 0:
+            check_commits = lambda pr_obj: env['runbot_merge.pull_requests.check_commits'].create({
+                'pull_request_id': pr_obj.id,
+            })
+        case 1:
+            squash = True
     r = pr['base']['repo']['full_name']
 
     if event['action'] in [
@@ -192,6 +201,7 @@ def handle_pr(env, event):
             ('squash', '!=', squash),
         ]):
             pr.squash = squash
+            check_commits(pr)
 
         return Response(
             status=200,
@@ -267,6 +277,7 @@ def find(target):
         if updates:
             # copy because it updates the `updates` dict internally
             pr_obj.write(dict(updates))
+            check_commits(pr_obj)
             return Response(
                 status=200,
                 mimetype="text/plain",
@@ -320,8 +331,13 @@ def find(target):
         author = env['res.partner'].search([('github_login', '=', author_name)], limit=1)
         if not author:
             env['res.partner'].create({'name': author_name, 'github_login': author_name})
-        pr_obj = env['runbot_merge.pull_requests']._from_gh(pr)
-        return Response(status=201, mimetype="text/plain", response=f"Tracking PR as {pr_obj.display_name}")
+        try:
+            pr_obj = env['runbot_merge.pull_requests']._from_gh(pr)
+            check_commits(pr_obj)
+            return Response(status=201, mimetype="text/plain", response=f"Tracking PR as {pr_obj.display_name}")
+        except psycopg2.errors.UniqueViolation:
+            env.cr.rollback()
+            return Response(status=200, mimetype="text/plain", response="Already known")
 
     pr_obj = env['runbot_merge.pull_requests']._get_or_schedule(r, pr['number'], closing=event['action'] == 'closed')
     if not pr_obj:
@@ -370,11 +386,13 @@ def find(target):
             'head': pr['head']['sha'],
             'squash': squash,
         })
+        check_commits(pr_obj)
         return Response(mimetype="text/plain", response=f'Updated to {pr_obj.head}')
 
     if event['action'] not in ('closed', 'reopened'):
         if pr_obj.squash != squash:
             pr_obj.squash = squash
+            check_commits(pr_obj)
 
     if event['action'] == 'ready_for_review':
         pr_obj.draft = False
@@ -422,6 +440,7 @@ def find(target):
                 'head': pr['head']['sha'],
                 'squash': pr['commits'] == 1,
             })
+            check_commits(pr_obj)
 
             return Response(mimetype="text/plain", response=f'Reopened {pr_obj.display_name}')
 

runbot_merge/controllers/misc.py

@@ -20,10 +20,10 @@ def merge_commit(self, commit_hash, repository, branch, project="RD"):
 
         target = request.env["runbot_merge.branch"].sudo().search([
             ("name", "=", branch),
-            ("project_id", "=", project)
+            ("project_id.name", "=", project)
         ])
         if not target:
-            return {"error": "Target branch %s:%s not found" % (branch, target)}
+            return {"error": f"Target branch {project}:{branch} not found"}
 
         patch = request.env["runbot_merge.patch"].sudo().create({
             "repository": repository_id.id,

runbot_merge/controllers/reviewer_provisioning.py

@@ -55,6 +55,17 @@ def provision_user(self, users):
                 # unique, and should not be able to collide with emails
                 partners[p.github_login.casefold()] = p
 
+        existing_users = Users.with_context(active_test=False).search([
+            ('partner_id', 'not in', existing_partners.ids),
+            '|', ('login', 'in', [u['email'] for u in users]),
+                 ('oauth_uid', 'in', [s for u in users if (s := u.get('sub'))]),
+        ])
+        _logger.info("Found %d existing matching users.", len(existing_users))
+        users_map = {}
+        for u in existing_users:
+            users_map[u.login] = u
+            users_map[u.oauth_uid] = u
+
         portal = env.ref('base.group_portal')
         internal = env.ref('base.group_user')
         odoo_provider = env.ref('auth_oauth.provider_openerp')
@@ -68,7 +79,12 @@ def provision_user(self, users):
                 new['oauth_uid'] = new.pop('sub')
 
             # prioritise by github_login as that's the unique-est point of information
-            current = partners.get(new['github_login'].casefold()) or partners.get(new['email']) or Partners
+            current = partners.get(new['github_login'].casefold()) \
+                   or partners.get(new['email']) \
+                   or (users_map.get(new['email'])
+                       or users_map.get(new.get('oauth_uid'))
+                       or Users
+                   ).partner_id
             if not current.active:
                 to_activate |= current
 
@@ -127,6 +143,7 @@ def provision_user(self, users):
                     new['partner_id'] = current.id
                 to_create.append(new)
 
+
         created = len(to_create)
         if to_create:
             # only create 100 users at a time to avoid request timeout
@@ -152,7 +169,16 @@ def fetch_reviewers(self, **kwargs):
         '/runbot_merge/remove_reviewers', # deprecated URL
     ], type='json', auth='public', methods=['POST'])
     def disable_users(self, github_logins, **kwargs):
+        _logger.info("deprovisioning %s: %s")
         partners = request.env['res.partner'].sudo().search([('github_login', 'in', github_logins)])
+        _logger.info(
+            "deprovisioning %s: %s",
+            github_logins,
+            [
+                f"{p.display_name} ({', '.join(p.mapped('user_ids.login'))})"
+                for p in partners
+            ]
+        )
         partners.write({
             'email': False,
             'parent_id': False,

runbot_merge/fw_tests/test_conflicts.py

@@ -170,6 +170,55 @@ def test_conflict(env, config, make_repo, users):
         'i': 'a',
     }
 
+def test_conflict_unknown_root_head(
+    env, config, make_repo,
+):
+    """Very specific implementation detail: on conflict we try to infer
+    modify/delete conflicts in order to mark the files git reintrodued, to do so
+    we try to list the files modified by the original PR, and that requires
+    having the head of its target branch locally...
+    """
+    fw_cron = env.ref('runbot_merge.port_forward')
+    fw_cron.active = False
+    prod, _other = make_basic(env, config, make_repo, statuses="default")
+
+    with prod:
+        # generate conflict: remove f from b, while updating f in a
+        prod.make_commits(
+            'b', Commit('33', tree={'g': 'c'}, reset=True),
+            ref='heads/b'
+        )
+        [p_0] = prod.make_commits(
+            'a', Commit('p_0', tree={'f': 'xxx'}),
+            ref='heads/a_pr'
+        )
+        pr = prod.make_pr(target='a', head='a_pr')
+        prod.post_status(p_0, 'success')
+        pr.post_comment('hansen r+', config['role_reviewer']['token'])
+    env.run_crons()
+    with prod:
+        prod.post_status('staging.a', 'success')
+    env.run_crons()
+
+    # there should be no forward port (yet)
+    assert env['runbot_merge.pull_requests'].search_count([]) == 1
+    # there should be an fw job
+    assert env['forwardport.batches'].search_count([]) == 1
+
+    # add a commit to a which the mergebot should not know about
+    with prod:
+        prod.make_commits(
+            'a',
+            Commit('X', tree={'g': 'fkdshf'}),
+            ref='heads/a',
+        )
+
+    fw_cron.active = True
+    fw_cron.trigger()
+    env.run_crons()
+
+    assert env['runbot_merge.pull_requests'].search_count([]) == 2
+
 def test_massive_conflict(env, config, make_repo):
     """If the conflict is large enough, the commit message may exceed ARG_MAX
      and trigger E2BIG.

runbot_merge/git.py

@@ -193,16 +193,6 @@ def fetchone(self, repo, branch: str) -> str:
         """
         return next(self.fetch_heads(repo, f"refs/heads/{branch}"))
 
-    def remote_head(self, repo, branch: str) -> str:
-        r = self.stdout().with_config(check=True, encoding="utf-8").ls_remote(
-            source_url(repo),
-            f'refs/heads/{branch}',
-        )
-        assert r.stdout.count('\n') == 1, f"expected single line, got {r.stdout}"
-        # The output is in the format: <oid> TAB <ref> LF
-        head, _ = r.stdout.split('\t', 1)
-        return head
-
     def get_tree(self, rev: str) -> str:
         return self.stdout().with_config(check=True, encoding="utf-8")\
             .rev_parse(f'{rev}^{{tree}}')\

runbot_merge/migrations/17.0.1.20/pre-migrations.py

@@ -0,0 +1,5 @@
+def migrate(cr, _version):
+    cr.execute("""
+ALTER TYPE runbot_merge_acls_state_type
+    RENAME TO runbot_merge_acls_effect_type;
+""")

runbot_merge/models/backport/__init__.py

@@ -89,7 +89,7 @@ def action_apply(self) -> dict:
 
         old_map = self.pr_id.commits_map
         self.pr_id.commits_map = "{}"
-        conflict, head = self.pr_id._create_port_branch(repo, self.target, forward=False)
+        conflict, head, n = self.pr_id._create_port_branch(repo, self.target, forward=False)
         self.pr_id.commits_map = old_map
 
         if conflict:
@@ -122,6 +122,7 @@ def action_apply(self) -> dict:
             # the backport's own forwardport should stop right before the
             # original PR by default
             limit_id=branches[source_idx - 1],
+            squash=n==1,
         )
         _logger.info("Created backport %s for %s", backport.display_name, self.pr_id.display_name)