diff --git a/runbot/models/branch.py b/runbot/models/branch.py index 8deb9ec5c..1fe4ed1bb 100644 --- a/runbot/models/branch.py +++ b/runbot/models/branch.py @@ -103,7 +103,7 @@ def _compute_reference_name(self): reference_name = branch.name repo = branch.remote_id.repo_id forced_version = repo.enforce_version and repo.single_version # we don't add a depend on repo.single_version to avoid mass recompute of existing branches - if forced_version and not (reference_name.startswith(f'{forced_version.name}-') or reference_name == forced_version.name): + if forced_version and not self._has_version_prefix(reference_name, forced_version.name): reference_name = f'{forced_version.name}---{reference_name}' branch.reference_name = reference_name @@ -303,6 +303,14 @@ def _match_is_base(self, name): if regex: return re.match(regex, name) + @api.model + def _has_version_prefix(self, name, version_name): + return ( + name == version_name + or name.startswith(f'{version_name}-') + or name.startswith(f'{version_name}/') + ) + def action_recompute_infos(self): return self._recompute_infos() diff --git a/runbot/models/bundle.py b/runbot/models/bundle.py index 29d010232..29367cff0 100644 --- a/runbot/models/bundle.py +++ b/runbot/models/bundle.py @@ -114,7 +114,7 @@ def _compute_base_id(self): master_base = False fallback = False for bid, bname in self._get_base_ids(project_id): - if bundle.name.startswith('%s-' % bname): + if self.env['runbot.branch']._has_version_prefix(bundle.name, bname): bundle.base_id = self.browse(bid) break elif bname == 'master': @@ -332,11 +332,11 @@ def _consistency_warning(self): else: for branch in self.branch_ids: if branch.is_pr and branch.target_branch_name != self.base_id.name: - if branch.target_branch_name.startswith(self.base_id.name): + if self.env['runbot.branch']._has_version_prefix(branch.target_branch_name, self.base_id.name): warnings.append(('info', 'PR %s targeting a non base branch: %s' % (branch.dname, branch.target_branch_name))) else: warnings.append(('warning' if branch.alive else 'info', 'PR %s targeting wrong version: %s (expecting %s)' % (branch.dname, branch.target_branch_name, self.base_id.name))) - elif not branch.is_pr and not branch.name.startswith(self.base_id.name) and not self.defined_base_id and branch.remote_id.repo_id.enforce_version: + elif not branch.is_pr and not self.env['runbot.branch']._has_version_prefix(branch.name, self.base_id.name) and not self.defined_base_id and branch.remote_id.repo_id.enforce_version: warnings.append(('warning', 'Branch %s not starting with version name (%s)' % (branch.dname, self.base_id.name))) return warnings diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 32c12acb6..d4ff7a64f 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -575,6 +575,10 @@ def _is_branch_forbidden(self, branch_name): return re.match(self.forbidden_regex, branch_name) return False + @api.model + def _branch_name_from_ref(self, ref_name): + return ref_name.split('/', 3)[-1] + def _get_fetch_head_time(self): self.ensure_one() fname_fetch_head = self._path('FETCH_HEAD') @@ -607,9 +611,9 @@ def _get_refs(self, max_age=30, ignore=None): return [] refs = [tuple(field for field in line.split('\x00')) for line in git_refs.split('\n')] refs = [r for r in refs if not re.match(r'^refs/[\w-]+/heads/\d+$', r[0])] # remove branches with interger names to avoid confusion with pr names - refs = [r for r in refs if int(r[2]) > commit_limit or self.env['runbot.branch']._match_is_base(r[0].split('/')[-1])] + refs = [r for r in refs if int(r[2]) > commit_limit or self.env['runbot.branch']._match_is_base(self._branch_name_from_ref(r[0]))] if ignore: - refs = [r for r in refs if r[0].split('/')[-1] not in ignore] + refs = [r for r in refs if self._branch_name_from_ref(r[0]) not in ignore] return refs except Exception: _logger.exception('Fail to get refs for repo %s', self.name) @@ -625,7 +629,7 @@ def _find_or_create_branches(self, refs): """ # FIXME WIP - names = [r[0].split('/', 3)[-1] for r in refs] + names = [self._branch_name_from_ref(r[0]) for r in refs] branches = self.env['runbot.branch'].search([('name', 'in', names), ('remote_id', 'in', self.remote_ids.ids)]) ref_branches = {branch._ref(): branch for branch in branches} new_branch_values = [] diff --git a/runbot/tests/test_branch.py b/runbot/tests/test_branch.py index fc4880034..2d023e925 100644 --- a/runbot/tests/test_branch.py +++ b/runbot/tests/test_branch.py @@ -181,6 +181,36 @@ def test_relations_pr(self): self.assertEqual(sorted(bundle.intermediate_version_base_ids.mapped('name')), ['saas-13.1', 'saas-13.2']) self.assertIn(dev_branch, bundle.branch_ids) +class TestBranchSlashNames(RunbotCase): + + def setUp(self): + super().setUp() + + def create_base(name): + branch = self.Branch.create({ + 'remote_id': self.remote_odoo.id, + 'name': name, + 'is_pr': False, + }) + branch.bundle_id.is_base = True + return branch + + create_base('19.0') + self.env['runbot.bundle'].flush_model() + self.env['runbot.version'].flush_model() + + def test_branch_version_detection(self): + for branch_name in ('19.0/foo', '19.0/tests/my-feature', '19.0-dev-tri'): + with self.subTest(branch_name=branch_name): + branch = self.Branch.create({ + 'remote_id': self.remote_odoo_dev.id, + 'name': branch_name, + 'is_pr': False, + }) + self.assertEqual(branch.bundle_id.base_id.name, '19.0') + self.assertEqual(branch.bundle_id.version_id.name, '19.0') + + class TestBranchForbidden(RunbotCase): """Test that a branch matching the repo forbidden regex, goes to dummy bundle""" diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index d4c40fcae..b1bfb8d0d 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -461,7 +461,8 @@ def setUp(self): def mock_git_helper(self, repo, cmd, input_data=None, raw=False): self.assertIn('for-each-ref', cmd) - self.assertIn('refs/*/pull/*', cmd) + if any(remote.fetch_pull for remote in repo.remote_ids): + self.assertIn('refs/*/pull/*', cmd) return '\n'.join(['\x00'.join(ref_data) for ref_data in self.test_refs]) def test_get_refs(self): @@ -504,3 +505,47 @@ def test_get_refs(self): self.assertIn(good_ref, refs, 'A valid branch should appear in refs') self.assertNotIn(bad_ref, refs, 'A branch name that is an integer should be filtered out') self.assertNotIn(to_ignore_ref, refs, 'An explicitely ignored branch should be filtered out') + + def test_get_refs_slashes(self): + current = time.time() + commit_time = str(int(current) - 5000) + remote_name = self.remote_odoo_dev.remote_name + slash_refs = ( + ( + 'refs/%s/heads/19.0/foo' % remote_name, + 'da39a3ee5e6b4b0d3255bfef95601890afd80709', + commit_time, + 'foobarman', + '', + '[IMP] mail: better tests', + 'foobarman', + '', + ), + ( + 'refs/%s/heads/19.0/tests/foo' % remote_name, + 'e9b396d2dddffdb373bf2c6ad073696aa25b4f68', + commit_time, + 'foobarman', + '', + '[FIX] foo: bar', + 'foobarman', + '', + ), + ) + self.test_refs.extend(slash_refs) + refs = self.repo_odoo._get_refs() + for slash_ref in slash_refs: + self.assertIn(slash_ref, refs, 'Slash branch ref should not be truncated') + self.assertEqual( + self.repo_odoo._branch_name_from_ref(slash_ref[0]), + slash_ref[0].split('/', 3)[-1], + ) + + ref_branches = self.repo_odoo._find_or_create_branches(refs) + for branch_name in ('19.0/foo', '19.0/tests/foo'): + branch = self.env['runbot.branch'].search([ + ('name', '=', branch_name), + ('remote_id', '=', self.remote_odoo_dev.id), + ]) + self.assertEqual(len(branch), 1, 'Branch %s should have been created' % branch_name) + self.assertIn(branch._ref(), ref_branches)