[IMP] runbot: add dependencies to build

Before this commit, dependencies (i.e. community commit to use when testing enterprise)
were computed at checkout, when the build was going from pending to testing state and
were not stored.

Since the duplicate detection was done at create, the get_closest_branch_name was called
in a loop for each posible duplicate candidate, then a last time at checkout. The main idea of this
pr is to store the build dependecies on build at create, making the duplicate detection
faster (especially when the build name is matching many indirect builds).

The side effect of this change is that the build dependencies won't be affected if a new
commit is pushed between the build creation and the checkout. The build is fully
determined at creation. get_closest_branch is only called once per build

The duplicate detection will also be more precise since we are matching on the commits groups
that were used to run the build, and not only the branch name.

Some work has also been done to rework the closest branch detection in order to manage new corner
cases. Hopefully, everything should work as before (or in a better way).

In a soon future, it will also be possible to use this information to make an "exact rebuild"
or to find corresponding community build.

Pr: #117

runbot/__manifest__.py

@@ -6,7 +6,7 @@
     'author': "Odoo SA",
     'website': "http://runbot.odoo.com",
     'category': 'Website',
-    'version': '3.0',
+    'version': '3.1',
     'depends': ['website', 'base'],
     'data': [
         'security/runbot_security.xml',

runbot/models/__init__.py

@@ -1,4 +1,4 @@
 # -*- coding: utf-8 -*-
 
-from . import repo, branch, build, event
+from . import repo, branch, build, event, build_dependency
 from . import res_config_settings

runbot/models/branch.py

@@ -101,17 +101,15 @@ class runbot_branch(models.Model):
             last_build = branch._get_last_coverage_build()
             branch.coverage_result = last_build.coverage_result or 0.0
 
-    def _get_closest_branch_name(self, target_repo_id):
+    def _get_closest_branch(self, target_repo_id):
-        """Return (repo, branch name) of the closest common branch between build's branch and
+        """
-           any branch of target_repo or its duplicated repos.
+            Return branch id of the closest branch based on name or pr informations. 
-            to prevent the above rules to mistakenly link PR of different repos together.
         """
         self.ensure_one()
         Branch = self.env['runbot.branch']
 
-        branch, repo = self, self.repo_id
+        repo = self.repo_id
-        name = branch.pull_head_name or branch.branch_name
+        name = self.pull_head_name or self.branch_name
-        target_branch = branch.target_branch_name or 'master'
 
         target_repo = self.env['runbot.repo'].browse(target_repo_id)
 
@@ -125,68 +123,93 @@ class runbot_branch(models.Model):
 
         _logger.debug('Search closest of %s (%s) in repos %r', name, repo.name, target_repo_ids)
 
-        sort_by_repo = lambda d: (not d['sticky'],      # sticky first
+        def sort_by_repo(branch):
-                                  target_repo_ids.index(d['repo_id'][0]),
+            return (
-                                  -1 * len(d.get('branch_name', '')),
+                not branch.sticky,      # sticky first
-                                  -1 * d['id'])
+                target_repo_ids.index(branch.repo_id[0].id),
-        result_for = lambda d, match='exact': (d['repo_id'][0], d['name'], match)
+                -1 * len(branch.branch_name),  # little change of logic here, was only sorted on branch_name in prefix matching case before
-        fields = ['name', 'repo_id', 'sticky']
+                -1 * branch.id
+            )
 
         # 1. same name, not a PR
+        if not self.pull_head_name:  # not a pr
             domain = [
                 ('repo_id', 'in', target_repo_ids),
-            ('branch_name', '=', name),
+                ('branch_name', '=', self.branch_name),
                 ('name', '=like', 'refs/heads/%'),
             ]
-        targets = Branch.search_read(domain, fields, order='id DESC')
+            targets = Branch.search(domain, order='id DESC')
             targets = sorted(targets, key=sort_by_repo)
-        if targets and self._branch_exists(targets[0]['id']):
+            if targets and targets[0]._is_on_remote():
-            return result_for(targets[0])
+                return (targets[0], 'exact')
 
         # 2. PR with head name equals
+        if self.pull_head_name:
             domain = [
                 ('repo_id', 'in', target_repo_ids),
-            ('pull_head_name', '=', name),
+                ('pull_head_name', '=', self.pull_head_name),
                 ('name', '=like', 'refs/pull/%'),
             ]
-        pulls = Branch.search_read(domain, fields, order='id DESC')
+            pulls = Branch.search(domain, order='id DESC')
             pulls = sorted(pulls, key=sort_by_repo)
             for pull in Branch.browse([pu['id'] for pu in pulls]):
                 pi = pull._get_pull_info()
                 if pi.get('state') == 'open':
-                if ':' in name:  # we assume that branch exists if we got pull info
+                    if ':' in self.pull_head_name:
-                    pr_branch_name = name.split(':')[1]
+                        (repo_name, pr_branch_name) = self.pull_head_name.split(':')
-                    return (pull.repo_id.duplicate_id.id, 'refs/heads/%s' % pr_branch_name, 'exact PR')
+                        repo = self.env['runbot.repo'].browse(target_repo_ids).filtered(lambda r: ':%s/' % repo_name in r.name)
-                else:
+                        # most of the time repo will be pull.repo_id.duplicate_id, but it is still possible to have a pr pointing the same repo
-                    return (pull.repo_id.id, pull.name, 'exact PR')
+                        if repo:
-
+                            pr_branch_ref = 'refs/heads/%s' % pr_branch_name
-        # 3. Match a branch which is the dashed-prefix of current branch name
+                            pr_branch = self._get_or_create_branch(repo.id, pr_branch_ref)
-        branches = Branch.search_read(
+                            # use _get_or_create_branch in case a pr is scanned before pull_head_name branch.
-            [('repo_id', 'in', target_repo_ids), ('name', '=like', 'refs/heads/%')],
+                            return (pr_branch, 'exact PR')
-            fields + ['branch_name'], order='id DESC',
+                    return (pull, 'exact PR')
-        )
-        branches = sorted(branches, key=sort_by_repo)
-
-        for branch in branches:
-            if name.startswith(branch['branch_name'] + '-') and self._branch_exists(branch['id']):
-                return result_for(branch, 'prefix')
 
         # 4.Match a PR in enterprise without community PR
-        if self.name.startswith('refs/pull') and ':' in name:
+        # Moved before 3 because it makes more sense
-            pr_branch_name = name.split(':')[1]
+        if self.pull_head_name:
+            if self.name.startswith('refs/pull'):
+                if ':' in self.pull_head_name:
+                    (repo_name, pr_branch_name) = self.pull_head_name.split(':')
+                    repos = self.env['runbot.repo'].browse(target_repo_ids).filtered(lambda r: ':%s/' % repo_name in r.name)
+                else:
+                    pr_branch_name = self.pull_head_name
+                    repos = target_repo
+                if repos:
                     duplicate_branch_name = 'refs/heads/%s' % pr_branch_name
                     domain = [
-                ('repo_id', 'in', target_repo_ids),  # target_repo_ids should contain the target duplicate repo
+                        ('repo_id', 'in', tuple(repos.ids)),
                         ('branch_name', '=', pr_branch_name),
                         ('pull_head_name', '=', False),
                     ]
-            targets = Branch.search_read(domain, fields, order='id DESC')
+                    targets = Branch.search(domain, order='id DESC')
                     targets = sorted(targets, key=sort_by_repo)
-            if targets and self._branch_exists(targets[0]['id']):
+                    if targets and targets[0]._is_on_remote():
-                return result_for(targets[0], 'no PR')
+                        return (targets[0], 'no PR')
+
+        # 3. Match a branch which is the dashed-prefix of current branch name
+        if not self.pull_head_name:
+            if '-' in self.branch_name:
+                name_start = 'refs/heads/%s' % self.branch_name.split('-')[0]
+                domain = [('repo_id', 'in', target_repo_ids), ('name', '=like', '%s%%' % name_start)]
+                branches = Branch.search(domain, order='id DESC')
+                branches = sorted(branches, key=sort_by_repo)
+                for branch in branches:
+                    if self.branch_name.startswith('%s-' % branch.branch_name) and branch._is_on_remote():
+                        return (branch, 'prefix')
 
         # 5. last-resort value
-        return target_repo_id, 'refs/heads/%s' % target_branch, 'default'
+        if self.target_branch_name:
+            default_target_ref = 'refs/heads/%s' % self.target_branch_name
+            default_branch = self.search([('repo_id', 'in', target_repo_ids), ('name', '=', default_target_ref)], limit=1)
+            if default_branch:
+                return (default_branch, 'pr_target')
+
+        default_target_ref = 'refs/heads/master'
+        default_branch = self.search([('repo_id', 'in', target_repo_ids), ('name', '=', default_target_ref)], limit=1)
+        # we assume that master will always exists
+        return (default_branch, 'default')
 
     def _branch_exists(self, branch_id):
         Branch = self.env['runbot.branch']
@@ -194,3 +217,12 @@ class runbot_branch(models.Model):
         if branch and branch[0]._is_on_remote():
             return True
         return False
+
+    def _get_or_create_branch(self, repo_id, name):
+        res = self.search([('repo_id', '=', repo_id), ('name', '=', name)], limit=1)
+        if res:
+            return res
+        _logger.warning('creating missing branch %s', name)
+        Branch = self.env['runbot.branch']
+        branch = Branch.create({'repo_id': repo_id, 'name': name})
+        return branch

runbot/models/build.py

@@ -69,11 +69,7 @@ class runbot_build(models.Model):
     job_age = fields.Integer(compute='_get_age', string='Job age')
     duplicate_id = fields.Many2one('runbot.build', 'Corresponding Build')
     server_match = fields.Selection([('builtin', 'This branch includes Odoo server'),
-                                     ('exact', 'branch/PR exact name'),
+                                     ('match', 'This branch includes Odoo server'),
-                                     ('prefix', 'branch whose name is a prefix of current one'),
-                                     ('fuzzy', 'Fuzzy - common ancestor found'),
-                                     ('exact PR', 'Exact match between two PR'),
-                                     ('no PR', 'PR matching a branch without PR'),
                                      ('default', 'No match found - defaults to master')],
                                     string='Server branch matching')
     revdep_build_ids = fields.Many2many('runbot.build', 'runbot_rev_dep_builds',
@@ -94,8 +90,8 @@ class runbot_build(models.Model):
         ('running', 'Running job only'),
         ('all', 'All jobs'),
         ('none', 'Do not execute jobs'),
-        ],
+    ])
-    )
+    dependency_ids = fields.One2many('runbot.build.dependency', 'build_id')
 
     def copy(self, values=None):
         raise UserError("Cannot duplicate build!")
@@ -104,33 +100,75 @@ class runbot_build(models.Model):
         branch = self.env['runbot.branch'].search([('id', '=', vals.get('branch_id', False))])
         if branch.job_type == 'none' or vals.get('job_type', '') == 'none':
             return self.env['runbot.build']
+        vals['job_type'] = vals['job_type'] if 'job_type' in vals else branch.job_type
         build_id = super(runbot_build, self).create(vals)
         extra_info = {'sequence': build_id.id if not build_id.sequence else build_id.sequence}
-        job_type = vals['job_type'] if 'job_type' in vals else build_id.branch_id.job_type
-        extra_info.update({'job_type': job_type})
         context = self.env.context
 
-        if not context.get('force_rebuild'):
+        # compute dependencies
+        repo = build_id.repo_id
+        dep_create_vals = []
+        nb_deps = len(repo.dependency_ids)
+        for extra_repo in repo.dependency_ids:
+            (build_closets_branch, match_type) = build_id.branch_id._get_closest_branch(extra_repo.id)
+            closest_name = build_closets_branch.name
+            closest_branch_repo = build_closets_branch.repo_id
+            last_commit = closest_branch_repo._git_rev_parse(closest_name)
+            dep_create_vals.append({
+                'build_id': build_id.id,
+                'dependecy_repo_id': extra_repo.id,
+                'closest_branch_id': build_closets_branch.id,
+                'dependency_hash': last_commit,
+                'match_type': match_type,
+            })
+
+        for dep_vals in dep_create_vals:
+            self.env['runbot.build.dependency'].sudo().create(dep_vals)
+
+        if not context.get('force_rebuild'):  # not vals.get('build_type') == rebuild': could be enough, but some cron on runbot are using this ctx key, to do later
             # detect duplicate
             duplicate_id = None
             domain = [
-                ('repo_id', '=', build_id.repo_id.duplicate_id.id),
+                ('repo_id', 'in', (build_id.repo_id.duplicate_id.id, build_id.repo_id.id)), # before, was only looking in repo.duplicate_id looks a little better to search in both
+                ('id', '!=', build_id.id),
                 ('name', '=', build_id.name),
                 ('duplicate_id', '=', False),
-                '|', ('result', '=', False), ('result', '!=', 'skipped')
+                # ('build_type', '!=', 'indirect'),  # in case of performance issue, this little fix may improve performance a little but less duplicate will be detected when pushing an empty branch on repo with duplicates
+                ('result', '!=', 'skipped'),
+                ('job_type', '=', build_id.job_type),
             ]
+            candidates = self.search(domain)
+            if candidates and nb_deps:
+                # check that all depedencies are matching.
+
+                # Note: We avoid to compare closest_branch_id, because the same hash could be found on
+                # 2 different branches (pr + branch). 
+                # But we may want to ensure that the hash is comming from the right repo, we dont want to compare community
+                # hash with enterprise hash.
+                # this is unlikely to happen so branch comparaison is disabled
+                self.env.cr.execute("""
+                    SELECT DUPLIDEPS.build_id
+                    FROM runbot_build_dependency as DUPLIDEPS
+                    JOIN runbot_build_dependency as BUILDDEPS
+                    ON BUILDDEPS.dependency_hash = DUPLIDEPS.dependency_hash
+                    --AND BUILDDEPS.closest_branch_id = DUPLIDEPS.closest_branch_id -- only usefull if we are affraid of hash collision in different branches
+                    AND BUILDDEPS.build_id = %s
+                    AND DUPLIDEPS.build_id in %s
+                    GROUP BY DUPLIDEPS.build_id
+                    HAVING COUNT(DUPLIDEPS.*) = %s
+                    ORDER BY DUPLIDEPS.build_id  -- remove this in case of performance issue, not so usefull
+                    LIMIT 1
+                """, (build_id.id, tuple(candidates.ids), nb_deps))
+                filtered_candidates_ids = self.env.cr.fetchall()
+
+                if filtered_candidates_ids:
+                    duplicate_id = filtered_candidates_ids[0]
+            else:
+                duplicate_id = candidates[0].id if candidates else False
 
-            for duplicate in self.search(domain, limit=10):
-                duplicate_id = duplicate.id
-                # Consider the duplicate if its closest branches are the same than the current build closest branches.
-                for extra_repo in build_id.repo_id.dependency_ids:
-                    build_closest_name = build_id._get_closest_branch_name(extra_repo.id)[1]
-                    duplicate_closest_name = duplicate._get_closest_branch_name(extra_repo.id)[1]
-                    if build_closest_name != duplicate_closest_name:
-                        duplicate_id = None
             if duplicate_id:
                 extra_info.update({'state': 'duplicate', 'duplicate_id': duplicate_id})
-                    break
+                # maybe update duplicate priority if needed
 
         build_id.write(extra_info)
         if build_id.state == 'duplicate' and build_id.duplicate_id.state in ('running', 'done'):
@@ -140,21 +178,6 @@ class runbot_build(models.Model):
     def _reset(self):
         self.write({'state': 'pending'})
 
-    def _get_closest_branch_name(self, target_repo_id):
-        """Return (repo, branch name) of the closest common branch between build's branch and
-           any branch of target_repo or its duplicated repos.
-
-        Rules priority for choosing the branch from the other repo is:
-        1. Same branch name
-        2. A PR whose head name match
-        3. Match a branch which is the dashed-prefix of current branch name
-        4. Common ancestors (git merge-base)
-        Note that PR numbers are replaced by the branch name of the PR target
-        to prevent the above rules to mistakenly link PR of different repos together.
-        """
-        self.ensure_one()
-        return self.branch_id._get_closest_branch_name(target_repo_id)
-
     @api.depends('name', 'branch_id.name')
     def _get_dest(self):
         for build in self:
@@ -436,8 +459,9 @@ class runbot_build(models.Model):
         return uniq_list(filter(mod_filter, modules))
 
     def _checkout(self):
-        for build in self:
+        self.ensure_one() # will raise exception if hash not found, we don't want to fail for all build.
         # starts from scratch
+        build = self
         if os.path.isdir(build._path()):
             shutil.rmtree(build._path())
 
@@ -472,24 +496,35 @@ class runbot_build(models.Model):
                 ]
                 _logger.debug("local modules_to_test for build %s: %s", build.dest, modules_to_test)
 
-                for extra_repo in build.repo_id.dependency_ids:
+            # todo make it backward compatible, or create migration script?
-                    repo_id, closest_name, server_match = build._get_closest_branch_name(extra_repo.id)
+            for build_dependency in build.dependency_ids:
-                    repo = self.env['runbot.repo'].browse(repo_id)
+                closest_branch = build_dependency.closest_branch_id
-                    _logger.debug('branch %s of %s: %s match branch %s of %s',
+                latest_commit = build_dependency.dependency_hash
-                                  build.branch_id.name, build.repo_id.name,
+                repo = closest_branch.repo_id
-                                  server_match, closest_name, repo.name)
+                closest_name = closest_branch.name
+                if build_dependency.match_type == 'default':
+                    server_match = 'default'
+                elif server_match != 'default':
+                    server_match = 'match'
+
                 build._log(
                     'Building environment',
-                        '%s match branch %s of %s' % (server_match, closest_name, repo.name)
+                    '%s match branch %s of %s' % (build_dependency.match_type, closest_name, repo.name)
                 )
-                    repo._update_git(force=True)
+                if not repo._hash_exists(latest_commit):
-                    latest_commit = repo._git(['rev-parse', closest_name]).strip()
+                    repo._update(force=True)
+                if not repo._hash_exists(latest_commit):
+                    repo._git(['fetch', 'origin', latest_commit])
+                if not repo._hash_exists(latest_commit):
+                    build._log('_checkout',"Dependency commit %s in repo %s is unreachable" % (latest_commit, repo.name))
+                    raise Exception
+
                 commit_oneline = repo._git(['show', '--pretty="%H -- %s"', '-s', latest_commit]).strip()
                 build._log(
                     'Building environment',
                     'Server built based on commit %s from %s' % (commit_oneline, closest_name)
                 )
-                    repo._git_export(closest_name, build._path())
+                repo._git_export(latest_commit, build._path())
 
             # Finally mark all addons to move to openerp/addons
             modules_to_move += [

runbot/models/build_dependency.py

@@ -0,0 +1,10 @@
+from odoo import models, fields
+
+class RunbotBuildDependency(models.Model):
+    _name = "runbot.build.dependency"
+
+    build_id = fields.Many2one('runbot.build', 'Build', required=True, ondelete='cascade', index=True)
+    dependecy_repo_id = fields.Many2one('runbot.repo', 'Dependency repo', required=True, ondelete='cascade')
+    dependency_hash = fields.Char('Name of commit', index=True)
+    closest_branch_id = fields.Many2one('runbot.branch', 'Branch', required=True, ondelete='cascade')
+    match_type = fields.Char('Match Type')

runbot/models/repo.py

@@ -86,6 +86,9 @@ class runbot_repo(models.Model):
             _logger.info("git command: %s", ' '.join(cmd))
             return subprocess.check_output(cmd).decode('utf-8')
 
+    def _git_rev_parse(self, branch_name):
+        return self._git(['rev-parse', branch_name]).strip()
+
     def _git_export(self, treeish, dest):
         """Export a git repo to dest"""
         self.ensure_one()
@@ -130,7 +133,7 @@ class runbot_repo(models.Model):
                 else:
                     raise
 
-    def _find_new_commits(self, repo):
+    def _find_new_commits(self):
         """ Find new commits in bare repo """
         self.ensure_one()
         Branch = self.env['runbot.branch']
@@ -140,7 +143,7 @@ class runbot_repo(models.Model):
 
         fields = ['refname', 'objectname', 'committerdate:iso8601', 'authorname', 'authoremail', 'subject', 'committername', 'committeremail']
         fmt = "%00".join(["%(" + field + ")" for field in fields])
-        git_refs = repo._git(['for-each-ref', '--format', fmt, '--sort=-committerdate', 'refs/heads', 'refs/pull'])
+        git_refs = self._git(['for-each-ref', '--format', fmt, '--sort=-committerdate', 'refs/heads', 'refs/pull'])
         git_refs = git_refs.strip()
 
         refs = [[field for field in line.split('\x00')] for line in git_refs.split('\n')]
@@ -150,7 +153,7 @@ class runbot_repo(models.Model):
           SELECT t.branch, b.id
             FROM t LEFT JOIN runbot_branch b ON (b.name = t.branch)
            WHERE b.repo_id = %s;
-        """, ([r[0] for r in refs], repo.id))
+        """, ([r[0] for r in refs], self.id))
         ref_branches = {r[0]: r[1] for r in self.env.cr.fetchall()}
 
         for name, sha, date, author, author_email, subject, committer, committer_email in refs:
@@ -158,8 +161,8 @@ class runbot_repo(models.Model):
             if ref_branches.get(name):
                 branch_id = ref_branches[name]
             else:
-                _logger.debug('repo %s found new branch %s', repo.name, name)
+                _logger.debug('repo %s found new branch %s', self.name, name)
-                branch_id = Branch.create({'repo_id': repo.id, 'name': name}).id
+                branch_id = Branch.create({'repo_id': self.id, 'name': name}).id
             branch = Branch.browse([branch_id])[0]
 
             # skip the build for old branches (Could be checked before creating the branch in DB ?)
@@ -208,10 +211,10 @@ class runbot_repo(models.Model):
                         if latest_rev_build:
                             _logger.debug('Reverse dependency build %s forced in repo %s by commit %s', latest_rev_build.dest, rev_repo.name, sha[:6])
                             latest_rev_build.build_type = 'indirect'
-                            new_build.revdep_build_ids += latest_rev_build._force(message='Rebuild from dependency %s commit %s' % (repo.name, sha[:6]))
+                            new_build.revdep_build_ids += latest_rev_build._force(message='Rebuild from dependency %s commit %s' % (self.name, sha[:6]))
 
         # skip old builds (if their sequence number is too low, they will not ever be built)
-        skippable_domain = [('repo_id', '=', repo.id), ('state', '=', 'pending')]
+        skippable_domain = [('repo_id', '=', self.id), ('state', '=', 'pending')]
         icp = self.env['ir.config_parameter']
         running_max = int(icp.get_param('runbot.runbot_running_max', default=75))
         builds_to_be_skipped = Build.search(skippable_domain, order='sequence desc', offset=running_max)
@@ -221,7 +224,7 @@ class runbot_repo(models.Model):
         """ Find new commits in physical repos"""
         for repo in repos:
             try:
-                repo._find_new_commits(repo)
+                repo._find_new_commits()
             except Exception:
                 _logger.exception('Fail to find new commits in repo %s', repo.name)
 
@@ -388,6 +391,7 @@ class runbot_repo(models.Model):
             repos = self.search([('mode', '!=', 'disabled')])
             self._update(repos, force=False)
             self._create_pending_builds(repos)
+
             self.env.cr.commit()
             self.invalidate_cache()
             time.sleep(update_frequency)

runbot/security/ir.model.access.csv

@@ -2,7 +2,9 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
 access_runbot_repo,runbot_repo,runbot.model_runbot_repo,group_user,1,0,0,0
 access_runbot_branch,runbot_branch,runbot.model_runbot_branch,group_user,1,0,0,0
 access_runbot_build,runbot_build,runbot.model_runbot_build,group_user,1,0,0,0
+access_runbot_build_dependency,runbot_build_dependency,runbot.model_runbot_build_dependency,group_user,1,0,0,0
 access_runbot_repo_admin,runbot_repo_admin,runbot.model_runbot_repo,runbot.group_runbot_admin,1,1,1,1
 access_runbot_branch_admin,runbot_branch_admin,runbot.model_runbot_branch,runbot.group_runbot_admin,1,1,1,1
 access_runbot_build_admin,runbot_build_admin,runbot.model_runbot_build,runbot.group_runbot_admin,1,1,1,1
+access_runbot_build_dependency_admin,runbot_build_dependency_admin,runbot.model_runbot_build_dependency,runbot.group_runbot_admin,1,1,1,1
 access_irlogging,log by runbot users,base.model_ir_logging,group_user,0,0,1,0

runbot/templates/build.xml

@@ -15,9 +15,9 @@
         <t t-if="bu['result']=='killed'"><i class="text-danger fa fa-times"/> killed</t>
         <t t-if="bu['result']=='manually_killed'"><i class="text-danger fa fa-times"/> manually killed</t>
 
-        <t t-if="bu['server_match'] in ('default', 'fuzzy')">
+        <t t-if="bu['server_match'] == 'default'">
             <i class="text-warning fa fa-question-circle fa-fw"
-                title="Server branch cannot be determined exactly. Please use naming convention '9.0-my-branch' to build with '9.0' server branch."/>
+                title="Server branch cannot be determined exactly. Please use naming convention '12.0-my-branch' to build with '12.0' server branch."/>
         </t>
         <t t-if="bu['revdep_build_ids']">
             <small class="pull-right">Dep builds:

runbot/tests/test_build.py

@@ -158,6 +158,35 @@ class Test_Build(common.TransactionCase):
         log_first_part = '%s skip %%s' % (other_build.dest)
         mock_logger.debug.assert_called_with(log_first_part, 'A good reason')
 
+    def test_ask_kill_duplicate(self):
+        """ Test that the _ask_kill method works on duplicate"""
+        #mock_is_on_remote.return_value = True
+
+        build1 = self.Build.create({
+            'branch_id': self.branch_10.id,
+            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
+        })
+        build2 = self.Build.create({
+            'branch_id': self.branch_10.id,
+            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
+        })
+        build2.write({'state': 'duplicate', 'duplicate_id': build1.id}) # this may not be usefull if we detect duplicate in same repo.
+
+        self.assertEqual(build1.state, 'pending')
+        build2._ask_kill()
+        self.assertEqual(build1.state, 'done', 'A killed pending duplicate build should mark the real build as done')
+        self.assertEqual(build1.result, 'skipped', 'A killed pending duplicate build should mark the real build as skipped')
+
+
+def rev_parse(repo, branch_name):
+    """
+    simulate a rev parse by returning a fake hash of form
+    'rp_odoo-dev/enterprise_saas-12.2__head'
+    should be overwitten if a pr head should match a branch head
+    """
+    head_hash = 'rp_%s_%s_head' % (repo.name.split(':')[1], branch_name.split('/')[-1])
+    return head_hash
+
 
 class TestClosestBranch(common.TransactionCase):
 
@@ -165,16 +194,15 @@ class TestClosestBranch(common.TransactionCase):
         branch_type = 'pull' if 'pull' in branch.name else 'branch'
         return '%s %s:%s' % (branch_type, branch.repo_id.name.split(':')[-1], branch.name.split('/')[-1])
 
-    def assertClosest(self, build, closest):
+    def assertClosest(self, branch, closest):
-        extra_repo = build.repo_id.dependency_ids[0]
+        extra_repo = branch.repo_id.dependency_ids[0]
-        self.assertEqual(closest, build._get_closest_branch_name(extra_repo.id), "build on %s didn't had the extected closest branch" % self.branch_description(build.branch_id))
+        self.assertEqual(closest, branch._get_closest_branch(extra_repo.id), "build on %s didn't had the extected closest branch" % self.branch_description(branch))
 
-    def assertDuplicate(self, branch1, branch2, b1_closest=None, b2_closest=None):
+    def assertDuplicate(self, branch1, branch2, b1_closest=None, b2_closest=None, noDuplicate=False):
         """
         Test that the creation of a build on branch1 and branch2 detects duplicate, no matter the order.
         Also test that build on branch1 closest_branch_name result is b1_closest if given
         Also test that build on branch2 closest_branch_name result is b2_closest if given
-        Test that the _ask_kill method works on duplicate
         """
         closest = {
             branch1: b1_closest,
@@ -188,7 +216,7 @@ class TestClosestBranch(common.TransactionCase):
             })
 
             if b1_closest:
-                self.assertClosest(build1, closest[b1])
+                self.assertClosest(b1, closest[b1])
 
             build2 = self.Build.create({
                 'branch_id': b2.id,
@@ -196,15 +224,16 @@ class TestClosestBranch(common.TransactionCase):
             })
 
             if b2_closest:
-                self.assertClosest(build2, closest[b2])
+                self.assertClosest(b2, closest[b2])
-
+            if noDuplicate:
+                self.assertNotEqual(build2.state, 'duplicate')
+                self.assertFalse(build2.duplicate_id, "build on %s was detected as duplicate of build %s" % (self.branch_description(b2), build2.duplicate_id))
+            else:
                 self.assertEqual(build2.duplicate_id.id, build1.id, "build on %s wasn't detected as duplicate of build on %s" % (self.branch_description(b2), self.branch_description(b1)))
                 self.assertEqual(build2.state, 'duplicate')
 
-            self.assertEqual(build1.state, 'pending')
+    def assertNoDuplicate(self, branch1, branch2, b1_closest=None, b2_closest=None):
-            build2._ask_kill()
+        self.assertDuplicate(branch1, branch2, b1_closest=b1_closest, b2_closest=b2_closest, noDuplicate=True)
-            self.assertEqual(build1.state, 'done', 'A killed pending duplicate build should mark the real build as done')
-            self.assertEqual(build1.result, 'skipped', 'A killed pending duplicate build should mark the real build as skipped')
 
     def setUp(self):
         """ Setup repositories that mimick the Odoo repos """
@@ -229,28 +258,34 @@ class TestClosestBranch(common.TransactionCase):
         self.Branch = self.env['runbot.branch']
         self.branch_odoo_master = self.Branch.create({
             'repo_id': self.community_repo.id,
-            'name': 'refs/heads/master'
+            'name': 'refs/heads/master',
+            'sticky': True,
         })
         self.branch_odoo_10 = self.Branch.create({
             'repo_id': self.community_repo.id,
-            'name': 'refs/heads/10.0'
+            'name': 'refs/heads/10.0',
+            'sticky': True,
         })
         self.branch_odoo_11 = self.Branch.create({
             'repo_id': self.community_repo.id,
-            'name': 'refs/heads/11.0'
+            'name': 'refs/heads/11.0',
+            'sticky': True,
         })
 
         self.branch_enterprise_master = self.Branch.create({
             'repo_id': self.enterprise_repo.id,
-            'name': 'refs/heads/master'
+            'name': 'refs/heads/master',
+            'sticky': True,
         })
         self.branch_enterprise_10 = self.Branch.create({
             'repo_id': self.enterprise_repo.id,
-            'name': 'refs/heads/10.0'
+            'name': 'refs/heads/10.0',
+            'sticky': True,
         })
         self.branch_enterprise_11 = self.Branch.create({
             'repo_id': self.enterprise_repo.id,
-            'name': 'refs/heads/11.0'
+            'name': 'refs/heads/11.0',
+            'sticky': True,
         })
 
         self.Build = self.env['runbot.build']
@@ -279,6 +314,7 @@ class TestClosestBranch(common.TransactionCase):
     def test_closest_branch_01(self, mock_is_on_remote):
         """ test find a matching branch in a target repo based on branch name """
         mock_is_on_remote.return_value = True
+
         self.Branch.create({
             'repo_id': self.community_dev_repo.id,
             'name': 'refs/heads/10.0-fix-thing-moc'
@@ -287,14 +323,12 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_dev_repo.id,
             'name': 'refs/heads/10.0-fix-thing-moc'
         })
-        addons_build = self.Build.create({
+
-            'branch_id': addons_branch.id,
+        self.assertEqual((addons_branch, 'exact'), addons_branch._get_closest_branch(self.enterprise_dev_repo.id))
-            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
-        })
-        self.assertEqual((self.enterprise_dev_repo.id, addons_branch.name, 'exact'), addons_build._get_closest_branch_name(self.enterprise_dev_repo.id))
 
     @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
     def test_closest_branch_02(self, mock_github):
+
         """ test find two matching PR having the same head name """
         mock_github.return_value = {
             # "head label" is the repo:branch where the PR comes from
@@ -322,21 +356,17 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_repo.id,
             'name': 'refs/pull/789101'
         })
-        enterprise_build = self.Build.create({
+        self.assertEqual((community_branch, 'exact PR'), enterprise_pr._get_closest_branch(self.community_repo.id))
-            'branch_id': enterprise_pr.id,
-            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
-        })
-
-        self.assertEqual((self.community_dev_repo.id, 'refs/heads/bar_branch', 'exact PR'), enterprise_build._get_closest_branch_name(self.community_repo.id))
 
     @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
-    @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists')
+    @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
-    def test_closest_branch_02_improved(self, mock_branch_exists, mock_github):
+    def test_closest_branch_02_improved(self, mock_is_on_remote, mock_github):
         """ test that a PR in enterprise with a matching PR in Community
         uses the matching one"""
-        mock_branch_exists.return_value = True
 
-        self.Branch.create({
+        mock_is_on_remote.return_value = True
+
+        com_dev_branch = self.Branch.create({
             'repo_id': self.community_dev_repo.id,
             'name': 'refs/heads/saas-12.2-blabla'
         })
@@ -375,34 +405,30 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.community_repo.id,
             'name': 'refs/pull/32156'
         })
-
+        with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse):
             self.assertDuplicate(
                 ent_dev_branch,
                 ent_pr,
-            (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'),
+                (com_dev_branch, 'exact'),
-            (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact PR')
+                (com_dev_branch, 'exact PR')
             )
 
-    @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists')
+    @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
-    def test_closest_branch_03(self, mock_branch_exists):
+    def test_closest_branch_03(self, mock_is_on_remote):
         """ test find a branch based on dashed prefix"""
-        mock_branch_exists.return_value = True
+        mock_is_on_remote.return_value = True
         addons_branch = self.Branch.create({
             'repo_id': self.enterprise_dev_repo.id,
             'name': 'refs/heads/10.0-fix-blah-blah-moc'
         })
-        addons_build = self.Build.create({
+        self.assertEqual((self.branch_odoo_10, 'prefix'), addons_branch._get_closest_branch(self.community_repo.id))
-            'branch_id': addons_branch.id,
-            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
-        })
-        self.assertEqual((self.community_repo.id, 'refs/heads/10.0', 'prefix'), addons_build._get_closest_branch_name(self.community_repo.id))
 
     @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
-    @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists')
+    @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
-    def test_closest_branch_03_05(self, mock_branch_exists, mock_github):
+    def test_closest_branch_03_05(self, mock_is_on_remote, mock_github):
         """ test that a PR in enterprise without a matching PR in Community
         and no branch in community"""
-        mock_branch_exists.return_value = True
+        mock_is_on_remote.return_value = True
         # comm_repo = self.repo
         # self.repo.write({'token': 1})
 
@@ -429,7 +455,7 @@ class TestClosestBranch(common.TransactionCase):
 
         mock_github.side_effect = github_side_effect
 
-        self.Branch.create({
+        com_branch = self.Branch.create({
             'repo_id': self.community_repo.id,
             'name': 'refs/heads/saas-12.2'
         })
@@ -438,22 +464,22 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_repo.id,
             'name': 'refs/pull/3721'
         })
-
+        with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse):
             self.assertDuplicate(
                 ent_pr,
                 ent_dev_branch,
-            (self.community_repo.id, 'refs/heads/saas-12.2', 'default'),
+                (com_branch, 'pr_target'),
-            (self.community_repo.id, 'refs/heads/saas-12.2', 'prefix'),
+                (com_branch, 'prefix'),
             )
 
     @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
-    @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists')
+    @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
-    def test_closest_branch_04(self, mock_branch_exists, mock_github):
+    def test_closest_branch_04(self, mock_is_on_remote, mock_github):
         """ test that a PR in enterprise without a matching PR in Community
         uses the corresponding exact branch in community"""
-        mock_branch_exists.return_value = True
+        mock_is_on_remote.return_value = True
 
-        self.Branch.create({
+        com_dev_branch = self.Branch.create({
             'repo_id': self.community_dev_repo.id,
             'name': 'refs/heads/saas-12.2-blabla'
         })
@@ -465,7 +491,7 @@ class TestClosestBranch(common.TransactionCase):
 
         def github_side_effect(*args, **kwargs):
             return {
-                'head': {'label': 'ent-dev:saas-12.2-blabla'},
+                'head': {'label': 'odoo-dev:saas-12.2-blabla'},
                 'base': {'ref': 'saas-12.2'},
                 'state': 'open'
             }
@@ -476,12 +502,12 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_repo.id,
             'name': 'refs/pull/3721'
         })
-
+        with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse):
             self.assertDuplicate(
                 ent_dev_branch,
                 ent_pr,
-            (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'),
+                (com_dev_branch, 'exact'),
-            (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'no PR')
+                (com_dev_branch, 'no PR')
             )
 
     @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
@@ -506,11 +532,7 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_repo.id,
             'name': 'refs/pull/789101'
         })
-        addons_build = self.Build.create({
+        self.assertEqual((self.branch_odoo_10, 'pr_target'), addons_pr._get_closest_branch(self.community_repo.id))
-            'branch_id': addons_pr.id,
-            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
-        })
-        self.assertEqual((self.community_repo.id, 'refs/heads/%s' % server_pr.target_branch_name, 'default'), addons_build._get_closest_branch_name(self.community_repo.id))
 
     def test_closest_branch_05_master(self):
         """ test last resort value when nothing common can be found"""
@@ -519,9 +541,86 @@ class TestClosestBranch(common.TransactionCase):
             'repo_id': self.enterprise_dev_repo.id,
             'name': 'refs/head/badref-fix-foo'
         })
-        addons_build = self.Build.create({
+        self.assertEqual((self.branch_odoo_master, 'default'), addons_branch._get_closest_branch(self.community_repo.id))
-            'branch_id': addons_branch.id,
+
+    @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
+    def test_no_duplicate_update(self, mock_is_on_remote):
+        """push a dev branch in enterprise with same head as sticky, but with a matching branch in community"""
+        mock_is_on_remote.return_value = True
+        community_sticky_branch = self.Branch.create({
+            'repo_id': self.community_repo.id,
+            'name': 'refs/heads/saas-12.2',
+            'sticky': True,
+        })
+        community_dev_branch = self.Branch.create({
+            'repo_id': self.community_dev_repo.id,
+            'name': 'refs/heads/saas-12.2-dev1',
+        })
+        enterprise_sticky_branch = self.Branch.create({
+            'repo_id': self.enterprise_repo.id,
+            'name': 'refs/heads/saas-12.2',
+            'sticky': True,
+        })
+        enterprise_dev_branch = self.Branch.create({
+            'repo_id': self.enterprise_dev_repo.id,
+            'name': 'refs/heads/saas-12.2-dev1'
+        })
+        # we shouldn't have duplicate since community_dev_branch exists
+        with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse):
+            # lets create an old enterprise build
+            self.Build.create({
+                'branch_id': enterprise_sticky_branch.id,
                 'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
             })
+            self.assertNoDuplicate(
+                enterprise_sticky_branch,
+                enterprise_dev_branch,
+                (community_sticky_branch, 'exact'),
+                (community_dev_branch, 'exact'),
+            )
 
-        self.assertEqual((self.community_repo.id, 'refs/heads/master', 'default'), addons_build._get_closest_branch_name(self.community_repo.id))
+    @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
+    def test_external_pr_closest_branch(self, mock_github):
+        """ test last resort value target_name"""
+        mock_github.return_value = {
+            'head': {'label': 'external_repo:11.0-fix'},
+            'base': {'ref': '11.0'},
+            'state': 'open'
+        }
+        enterprise_pr = self.Branch.create({
+            'repo_id': self.enterprise_repo.id,
+            'name': 'refs/pull/123456'
+        })
+        dependency_repo = self.enterprise_repo.dependency_ids[0]
+        closest_branch = enterprise_pr._get_closest_branch(dependency_repo.id)
+        self.assertEqual(enterprise_pr._get_closest_branch(dependency_repo.id), (self.branch_odoo_11, 'pr_target'))
+
+    @patch('odoo.addons.runbot.models.repo.runbot_repo._github')
+    def test_external_pr_with_comunity_pr_closest_branch(self, mock_github):
+        """ test matching external pr """
+        mock_github.return_value = {
+            'head': {'label': 'external_dev_repo:11.0-fix'},
+            'base': {'ref': '11.0'},
+            'state': 'open'
+        }
+        community_pr = self.Branch.create({
+            'repo_id': self.community_repo.id,
+            'name': 'refs/pull/123456'
+        })
+        mock_github.return_value = {
+            'head': {'label': 'external_dev_repo:11.0-fix'}, # if repo doenst match, it wont work, maybe a fix to do here?
+            'base': {'ref': '11.0'},
+            'state': 'open'
+        }
+        enterprise_pr = self.Branch.create({
+            'repo_id': self.enterprise_repo.id,
+            'name': 'refs/pull/123'
+        })
+        with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse):
+            build = self.Build.create({
+                'branch_id': enterprise_pr.id,
+                'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
+            })
+            dependency_repo = build.repo_id.dependency_ids[0]
+            self.assertEqual(build.branch_id._get_closest_branch(dependency_repo.id), (community_pr, 'exact PR'))
+            # this is working here because pull_head_name is set, but on runbot pull_head_name is empty for external pr. why?

runbot/tests/test_frontend.py

@@ -37,7 +37,8 @@ class Test_Frontend(common.HttpCase):
         names = ['deadbeef', 'd0d0caca', 'deadface', 'cacafeed']
         # create 5 builds in each branch
         for i, state, branch, name in zip(range(10), cycle(states), cycle(branches), cycle(names)):
-            self.Build.create({
+            name = '%s%s' % (name, i)
+            build = self.Build.create({
                 'branch_id': branch.id,
                 'name': '%s0000ffffffffffffffffffffffffffff' % name,
                 'port': '1234',