diff --git a/runbot/models/branch.py b/runbot/models/branch.py index 51b16631..9686c000 100644 --- a/runbot/models/branch.py +++ b/runbot/models/branch.py @@ -100,3 +100,97 @@ class runbot_branch(models.Model): for branch in self: 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): + """Return (repo, branch name) of the closest common branch between build's branch and + any branch of target_repo or its duplicated repos. + 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 + name = branch.pull_head_name or branch.branch_name + target_branch = branch.target_branch_name or 'master' + + target_repo = self.env['runbot.repo'].browse(target_repo_id) + + target_repo_ids = [target_repo.id] + r = target_repo.duplicate_id + while r: + if r.id in target_repo_ids: + break + target_repo_ids.append(r.id) + r = r.duplicate_id + + _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 + target_repo_ids.index(d['repo_id'][0]), + -1 * len(d.get('branch_name', '')), + -1 * d['id']) + result_for = lambda d, match='exact': (d['repo_id'][0], d['name'], match) + fields = ['name', 'repo_id', 'sticky'] + + # 1. same name, not a PR + domain = [ + ('repo_id', 'in', target_repo_ids), + ('branch_name', '=', name), + ('name', '=like', 'refs/heads/%'), + ] + targets = Branch.search_read(domain, fields, order='id DESC') + targets = sorted(targets, key=sort_by_repo) + if targets and self._branch_exists(targets[0]['id']): + return result_for(targets[0]) + + # 2. PR with head name equals + domain = [ + ('repo_id', 'in', target_repo_ids), + ('pull_head_name', '=', name), + ('name', '=like', 'refs/pull/%'), + ] + pulls = Branch.search_read(domain, fields, 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 + pr_branch_name = name.split(':')[1] + return (pull.repo_id.duplicate_id.id, 'refs/heads/%s' % pr_branch_name, 'exact PR') + else: + return (pull.repo_id.id, pull.name, 'exact PR') + + # 3. Match a branch which is the dashed-prefix of current branch name + branches = Branch.search_read( + [('repo_id', 'in', target_repo_ids), ('name', '=like', 'refs/heads/%')], + fields + ['branch_name'], order='id DESC', + ) + 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: + pr_branch_name = name.split(':')[1] + 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 + ('branch_name', '=', pr_branch_name), + ('pull_head_name', '=', False), + ] + targets = Branch.search_read(domain, fields, order='id DESC') + targets = sorted(targets, key=sort_by_repo) + if targets and self._branch_exists(targets[0]['id']): + return result_for(targets[0], 'no PR') + + # 5. last-resort value + return target_repo_id, 'refs/heads/%s' % target_branch, 'default' + + def _branch_exists(self, branch_id): + Branch = self.env['runbot.branch'] + branch = Branch.search([('id', '=', branch_id)]) + if branch and branch[0]._is_on_remote(): + return True + return False \ No newline at end of file diff --git a/runbot/models/build.py b/runbot/models/build.py index d0850e24..aa18b340 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -119,7 +119,7 @@ class runbot_build(models.Model): ('duplicate_id', '=', False), '|', ('result', '=', False), ('result', '!=', 'skipped') ] - # TODO xdo extract build_id._get_closest_branch_name(extra_repo.id) here + 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. @@ -140,13 +140,6 @@ class runbot_build(models.Model): def _reset(self): self.write({'state': 'pending'}) - def _branch_exists(self, branch_id): - Branch = self.env['runbot.branch'] - branch = Branch.search([('id', '=', branch_id)]) - if branch and branch[0]._is_on_remote(): - return True - return False - 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. @@ -160,87 +153,7 @@ class runbot_build(models.Model): to prevent the above rules to mistakenly link PR of different repos together. """ self.ensure_one() - Branch = self.env['runbot.branch'] - - build = self - branch, repo = build.branch_id, build.repo_id - name = branch.pull_head_name or branch.branch_name - target_branch = branch.target_branch_name or 'master' - - target_repo = self.env['runbot.repo'].browse(target_repo_id) - - target_repo_ids = [target_repo.id] - r = target_repo.duplicate_id - while r: - if r.id in target_repo_ids: - break - target_repo_ids.append(r.id) - r = r.duplicate_id - - _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 - target_repo_ids.index(d['repo_id'][0]), - -1 * len(d.get('branch_name', '')), - -1 * d['id']) - result_for = lambda d, match='exact': (d['repo_id'][0], d['name'], match) - fields = ['name', 'repo_id', 'sticky'] - - # 1. same name, not a PR - domain = [ - ('repo_id', 'in', target_repo_ids), - ('branch_name', '=', name), - ('name', '=like', 'refs/heads/%'), - ] - targets = Branch.search_read(domain, fields, order='id DESC') - targets = sorted(targets, key=sort_by_repo) - if targets and self._branch_exists(targets[0]['id']): - return result_for(targets[0]) - - # 2. PR with head name equals - domain = [ - ('repo_id', 'in', target_repo_ids), - ('pull_head_name', '=', name), - ('name', '=like', 'refs/pull/%'), - ] - pulls = Branch.search_read(domain, fields, 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 - pr_branch_name = name.split(':')[1] - return (pull.repo_id.duplicate_id.id, 'refs/heads/%s' % pr_branch_name, 'exact PR') - else: - return (pull.repo_id.id, pull.name, 'exact PR') - - # 3. Match a branch which is the dashed-prefix of current branch name - branches = Branch.search_read( - [('repo_id', 'in', target_repo_ids), ('name', '=like', 'refs/heads/%')], - fields + ['branch_name'], order='id DESC', - ) - 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 build.branch_id.name.startswith('refs/pull') and ':' in name: - pr_branch_name = name.split(':')[1] - 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 - ('branch_name', '=', pr_branch_name), - ('pull_head_name', '=', False), - ] - targets = Branch.search_read(domain, fields, order='id DESC') - targets = sorted(targets, key=sort_by_repo) - if targets and self._branch_exists(targets[0]['id']): - return result_for(targets[0], 'no PR') - - # 5. last-resort value - return target_repo_id, 'refs/heads/%s' % target_branch, 'default' + return self.branch_id._get_closest_branch_name(target_repo_id) @api.depends('name', 'branch_id.name') def _get_dest(self): diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 71d28306..3004c0f7 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -304,6 +304,15 @@ class TestClosestBranch(common.TransactionCase): 'state': 'open' } + # update to avoid test to break. we asume that bar_branch exists. + # we may want to modify the branch creation to ensure that + # -> first make all branches + # -> then make all builds + community_branch = self.Branch.create({ + 'repo_id': self.community_dev_repo.id, + 'name': 'refs/heads/bar_branch' + }) + # Create PR in community community_pr = self.Branch.create({ 'repo_id': self.community_repo.id, @@ -321,7 +330,7 @@ class TestClosestBranch(common.TransactionCase): 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.build.runbot_build._branch_exists') + @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') def test_closest_branch_02_improved(self, mock_branch_exists, mock_github): """ test that a PR in enterprise with a matching PR in Community uses the matching one""" @@ -374,7 +383,7 @@ class TestClosestBranch(common.TransactionCase): (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact PR') ) - @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') + @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') def test_closest_branch_03(self, mock_branch_exists): """ test find a branch based on dashed prefix""" mock_branch_exists.return_value = True @@ -389,7 +398,7 @@ class TestClosestBranch(common.TransactionCase): 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.build.runbot_build._branch_exists') + @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') def test_closest_branch_03_05(self, mock_branch_exists, mock_github): """ test that a PR in enterprise without a matching PR in Community and no branch in community""" @@ -438,7 +447,7 @@ class TestClosestBranch(common.TransactionCase): ) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') - @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') + @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') def test_closest_branch_04(self, mock_branch_exists, mock_github): """ test that a PR in enterprise without a matching PR in Community uses the corresponding exact branch in community"""