From ecfdb5693d171dd7de208cf6d0e1730260ef3292 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 9 Feb 2024 11:22:09 +0100 Subject: [PATCH] [FIX] runbot: branch computation --- runbot/models/branch.py | 38 ++++++++++++++----------- runbot/models/build_config_codeowner.py | 2 +- runbot/models/repo.py | 2 +- runbot/tests/common.py | 20 +++++++------ runbot/tests/test_build_config_step.py | 4 +-- runbot/views/branch_views.xml | 1 + 6 files changed, 38 insertions(+), 29 deletions(-) diff --git a/runbot/models/branch.py b/runbot/models/branch.py index 74fe54fd1..cfcd6561f 100644 --- a/runbot/models/branch.py +++ b/runbot/models/branch.py @@ -30,9 +30,9 @@ class Branch(models.Model): pr_body = fields.Char('Pr Body') pr_author = fields.Char('Pr Author') - pull_head_name = fields.Char(compute='_compute_branch_infos', string='PR HEAD name', readonly=True, store=True) - pull_head_remote_id = fields.Many2one('runbot.remote', 'Pull head repository', compute='_compute_branch_infos', store=True, index=True) - target_branch_name = fields.Char(compute='_compute_branch_infos', string='PR target branch', store=True) + pull_head_name = fields.Char(string='PR HEAD name', readonly=True, store=True) + pull_head_remote_id = fields.Many2one('runbot.remote', 'Pull head repository', store=True, index=True) + target_branch_name = fields.Char( string='PR target branch', store=True) reviewers = fields.Char('Reviewers') reflog_ids = fields.One2many('runbot.ref.log', 'branch_id') @@ -41,7 +41,7 @@ class Branch(models.Model): dname = fields.Char('Display name', compute='_compute_dname', search='_search_dname') alive = fields.Boolean('Alive', default=True) - draft = fields.Boolean('Draft', compute='_compute_branch_infos', store=True) + draft = fields.Boolean('Draft', store=True) @api.depends('name', 'remote_id.short_name') def _compute_dname(self): @@ -64,7 +64,7 @@ def _compute_reference_name(self): - pull_head_name (organisation:branch_name) for external pr """ for branch in self: - if branch.is_pr: + if branch.is_pr and branch.pull_head_name: _, name = branch.pull_head_name.split(':') if branch.pull_head_remote_id: reference_name = name @@ -79,8 +79,7 @@ def _compute_reference_name(self): reference_name = f'{forced_version.name}---{reference_name}' branch.reference_name = reference_name - @api.depends('name') - def _compute_branch_infos(self, pull_info=None): + def _update_branch_infos(self, pull_info=None): """compute branch_url, pull_head_name and target_branch_name based on name""" name_to_remote = {} prs = self.filtered(lambda branch: branch.is_pr) @@ -140,18 +139,15 @@ def _compute_branch_url(self): else: branch.branch_url = '' - @api.depends('reference_name', 'remote_id.repo_id.project_id') - def _compute_bundle_id(self): + def _update_bundle_id(self): for branch in self: dummy = branch.remote_id.repo_id.project_id.dummy_bundle_id - if branch.bundle_id == dummy: - continue name = branch.reference_name project = branch.remote_id.repo_id.project_id or self.env.ref('runbot.main_project') project.ensure_one() bundle = self.env['runbot.bundle'].search([('name', '=', name), ('project_id', '=', project.id)]) - need_new_base = not bundle and branch._match_is_base(name) - if (bundle.is_base or need_new_base) and branch.remote_id != branch.remote_id.repo_id.main_remote_id: + is_base = bundle.is_base if bundle else branch._match_is_base(name) + if is_base and branch.remote_id != branch.remote_id.repo_id.main_remote_id: _logger.warning('Trying to add a dev branch to base bundle, falling back on dummy bundle') bundle = dummy elif name and branch.remote_id and branch.remote_id.repo_id._is_branch_forbidden(name): @@ -165,7 +161,7 @@ def _compute_bundle_id(self): 'name': name, 'project_id': project.id, } - if need_new_base: + if is_base: values['is_base'] = True if branch.is_pr and branch.target_branch_name: # most likely external_pr, use target as version @@ -183,6 +179,8 @@ def _compute_bundle_id(self): @api.model_create_multi def create(self, value_list): branches = super().create(value_list) + branches._update_branch_infos() + branches._update_bundle_id() for branch in branches: if branch.head: self.env['runbot.ref.log'].create({'commit_id': branch.head.id, 'branch_id': branch.id}) @@ -214,8 +212,9 @@ def _recompute_infos(self, payload=None): was_draft = self.draft was_alive = self.alive init_target_branch_name = self.target_branch_name - self._compute_branch_infos(payload) + self._update_branch_infos(payload) if self.target_branch_name != init_target_branch_name: + #retarget _logger.info('retargeting %s to %s', self.name, self.target_branch_name) base = self.env['runbot.bundle'].search([ ('name', '=', self.target_branch_name), @@ -230,6 +229,10 @@ def _recompute_infos(self, payload=None): if self.draft: self.reviewers = '' # reset reviewers on draft + if not self.bundle_id: + self._update_bundle_id() + return + if was_alive and not self.alive and self.bundle_id.for_next_freeze: if not any(branch.alive and branch.is_pr for branch in self.bundle_id.branch_ids): self.bundle_id.for_next_freeze = False @@ -246,10 +249,13 @@ def _match_is_base(self, name): regex = icp.get_param('runbot.runbot_is_base_regex', False) if regex: return re.match(regex, name) - + def action_recompute_infos(self): return self._recompute_infos() + def action_update_bundle_id(self): + return self._update_bundle_ids() + class RefLog(models.Model): _name = 'runbot.ref.log' diff --git a/runbot/models/build_config_codeowner.py b/runbot/models/build_config_codeowner.py index e5910fb73..2888fa14f 100644 --- a/runbot/models/build_config_codeowner.py +++ b/runbot/models/build_config_codeowner.py @@ -149,7 +149,7 @@ def _run_codeowner(self, build): build._log('', 'Requesting review for pull request [%s](%s): %s' % (pr.dname, pr.branch_url, ', '.join(new_reviewers)), log_type='markdown') response = pr.remote_id._github('/repos/:owner/:repo/pulls/%s/requested_reviewers' % pr.name, {"team_reviewers": list(new_reviewers)}, ignore_errors=False) - pr._compute_branch_infos(response) + pr._update_branch_infos(response) pr['reviewers'] = ','.join(sorted(reviewers)) else: build._log('', 'All reviewers are already on pull request [%s](%s)' % (pr.dname, pr.branch_url,), log_type='markdown') diff --git a/runbot/models/repo.py b/runbot/models/repo.py index a30b3486f..05d2b98ee 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -491,7 +491,7 @@ def _find_new_commits(self, refs, ref_branches): if not branch.alive: if branch.is_pr: _logger.info('Recomputing infos of dead pr %s', branch.name) - branch._compute_branch_infos() + branch._update_branch_infos() else: branch.alive = True diff --git a/runbot/tests/common.py b/runbot/tests/common.py index 94206f2d1..0879fb9f9 100644 --- a/runbot/tests/common.py +++ b/runbot/tests/common.py @@ -125,15 +125,17 @@ def setUp(self): 'is_pr': False, 'remote_id': self.remote_server.id, }) - self.dev_pr = self.Branch.create({ - 'name': '1234', - 'is_pr': True, - 'remote_id': self.remote_server.id, - 'target_branch_name': self.dev_bundle.base_id.name, - 'pull_head_remote_id': self.remote_server.id, - }) - self.dev_pr.pull_head_name = f'{self.remote_server.owner}:{self.dev_branch.name}' - self.dev_pr.bundle_id = self.dev_bundle.id, + with patch('odoo.addons.runbot.models.branch.Branch._update_branch_infos', return_value=None): + self.dev_pr = self.Branch.create({ + 'name': '1234', + 'is_pr': True, + 'alive': True, + 'remote_id': self.remote_server.id, + 'target_branch_name': self.dev_bundle.base_id.name, + 'pull_head_remote_id': self.remote_server.id, + 'pull_head_name': f'{self.remote_server.owner}:{self.dev_branch.name}', + }) + self.assertEqual(self.dev_pr.bundle_id.id, self.dev_bundle.id) self.dev_batch = self.Batch.create({ 'bundle_id': self.dev_bundle.id, diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 389a1c66d..0922f97d7 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -96,9 +96,9 @@ def test_codeowner_pr_duplicate(self): 'remote_id': self.remote_server.id, 'target_branch_name': self.dev_bundle.base_id.name, 'pull_head_remote_id': self.remote_server.id, + 'pull_head_name': f'{self.remote_server.owner}:{self.dev_branch.name}', }) - second_pr.pull_head_name = f'{self.remote_server.owner}:{self.dev_branch.name}' - second_pr.bundle_id = self.dev_bundle.id + self.assertEqual(second_pr.bundle_id.id, self.dev_bundle.id) self.config_step._run_codeowner(self.parent_build) self.assertEqual(self.parent_build.log_ids.mapped('message'), [ "More than one open pr in this bundle for server: ['1234', '1235']" diff --git a/runbot/views/branch_views.xml b/runbot/views/branch_views.xml index d688b84eb..946dea33e 100644 --- a/runbot/views/branch_views.xml +++ b/runbot/views/branch_views.xml @@ -7,6 +7,7 @@