diff --git a/runbot/__manifest__.py b/runbot/__manifest__.py index 17562504..b27a51ea 100644 --- a/runbot/__manifest__.py +++ b/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', diff --git a/runbot/models/__init__.py b/runbot/models/__init__.py index f334bd9b..6e299960 100644 --- a/runbot/models/__init__.py +++ b/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 \ No newline at end of file diff --git a/runbot/models/branch.py b/runbot/models/branch.py index 9686c000..eb94b5d8 100644 --- a/runbot/models/branch.py +++ b/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): - """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. + def _get_closest_branch(self, target_repo_id): + """ + Return branch id of the closest branch based on name or pr informations. """ 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' + repo = self.repo_id + name = self.pull_head_name or self.branch_name target_repo = self.env['runbot.repo'].browse(target_repo_id) @@ -125,72 +123,106 @@ 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 - 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'] + def sort_by_repo(branch): + return ( + not branch.sticky, # sticky first + target_repo_ids.index(branch.repo_id[0].id), + -1 * len(branch.branch_name), # little change of logic here, was only sorted on branch_name in prefix matching case before + -1 * branch.id + ) # 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]) + if not self.pull_head_name: # not a pr + domain = [ + ('repo_id', 'in', target_repo_ids), + ('branch_name', '=', self.branch_name), + ('name', '=like', 'refs/heads/%'), + ] + targets = Branch.search(domain, order='id DESC') + targets = sorted(targets, key=sort_by_repo) + if targets and targets[0]._is_on_remote(): + return (targets[0], 'exact') # 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') + if self.pull_head_name: + domain = [ + ('repo_id', 'in', target_repo_ids), + ('pull_head_name', '=', self.pull_head_name), + ('name', '=like', 'refs/pull/%'), + ] + 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 self.pull_head_name: + (repo_name, pr_branch_name) = self.pull_head_name.split(':') + repo = self.env['runbot.repo'].browse(target_repo_ids).filtered(lambda r: ':%s/' % repo_name in r.name) + # 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 + if repo: + pr_branch_ref = 'refs/heads/%s' % pr_branch_name + pr_branch = self._get_or_create_branch(repo.id, pr_branch_ref) + # use _get_or_create_branch in case a pr is scanned before pull_head_name branch. + return (pr_branch, 'exact PR') + return (pull, 'exact PR') # 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') + # Moved before 3 because it makes more sense + 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', tuple(repos.ids)), + ('branch_name', '=', pr_branch_name), + ('pull_head_name', '=', False), + ] + targets = Branch.search(domain, order='id DESC') + targets = sorted(targets, key=sort_by_repo) + if targets and targets[0]._is_on_remote(): + 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'] branch = Branch.search([('id', '=', branch_id)]) if branch and branch[0]._is_on_remote(): return True - return False \ No newline at end of file + 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 diff --git a/runbot/models/build.py b/runbot/models/build.py index aa18b340..c54ed811 100644 --- a/runbot/models/build.py +++ b/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'), - ('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'), + ('match', 'This branch includes Odoo server'), ('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. - 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 + # 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 + + if duplicate_id: + extra_info.update({'state': 'duplicate', 'duplicate_id': duplicate_id}) + # 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,96 +459,108 @@ class runbot_build(models.Model): return uniq_list(filter(mod_filter, modules)) def _checkout(self): - for build in self: - # starts from scratch - if os.path.isdir(build._path()): - shutil.rmtree(build._path()) + 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()) - # runbot log path - os.makedirs(build._path("logs"), exist_ok=True) - os.makedirs(build._server('addons'), exist_ok=True) + # runbot log path + os.makedirs(build._path("logs"), exist_ok=True) + os.makedirs(build._server('addons'), exist_ok=True) - # update repo if needed - if not build.repo_id._hash_exists(build.name): - build.repo_id._update(build.repo_id) + # update repo if needed + if not build.repo_id._hash_exists(build.name): + build.repo_id._update(build.repo_id) - # checkout branch - build.branch_id.repo_id._git_export(build.name, build._path()) + # checkout branch + build.branch_id.repo_id._git_export(build.name, build._path()) - has_server = os.path.isfile(build._server('__init__.py')) - server_match = 'builtin' + has_server = os.path.isfile(build._server('__init__.py')) + server_match = 'builtin' - # build complete set of modules to install - modules_to_move = [] - modules_to_test = ((build.branch_id.modules or '') + ',' + - (build.repo_id.modules or '')) - modules_to_test = list(filter(None, modules_to_test.split(','))) # ??? - explicit_modules = set(modules_to_test) - _logger.debug("manual modules_to_test for build %s: %s", build.dest, modules_to_test) + # build complete set of modules to install + modules_to_move = [] + modules_to_test = ((build.branch_id.modules or '') + ',' + + (build.repo_id.modules or '')) + modules_to_test = list(filter(None, modules_to_test.split(','))) # ??? + explicit_modules = set(modules_to_test) + _logger.debug("manual modules_to_test for build %s: %s", build.dest, modules_to_test) - if not has_server: - if build.repo_id.modules_auto == 'repo': - modules_to_test += [ - os.path.basename(os.path.dirname(a)) - for a in (glob.glob(build._path('*/__openerp__.py')) + - glob.glob(build._path('*/__manifest__.py'))) - ] - _logger.debug("local modules_to_test for build %s: %s", build.dest, modules_to_test) - - for extra_repo in build.repo_id.dependency_ids: - repo_id, closest_name, server_match = build._get_closest_branch_name(extra_repo.id) - repo = self.env['runbot.repo'].browse(repo_id) - _logger.debug('branch %s of %s: %s match branch %s of %s', - build.branch_id.name, build.repo_id.name, - server_match, closest_name, repo.name) - build._log( - 'Building environment', - '%s match branch %s of %s' % (server_match, closest_name, repo.name) - ) - repo._update_git(force=True) - latest_commit = repo._git(['rev-parse', closest_name]).strip() - 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()) - - # Finally mark all addons to move to openerp/addons - modules_to_move += [ - os.path.dirname(module) - for module in (glob.glob(build._path('*/__openerp__.py')) + - glob.glob(build._path('*/__manifest__.py'))) + if not has_server: + if build.repo_id.modules_auto == 'repo': + modules_to_test += [ + os.path.basename(os.path.dirname(a)) + for a in (glob.glob(build._path('*/__openerp__.py')) + + glob.glob(build._path('*/__manifest__.py'))) ] + _logger.debug("local modules_to_test for build %s: %s", build.dest, modules_to_test) - # move all addons to server addons path - for module in uniq_list(glob.glob(build._path('addons/*')) + modules_to_move): - basename = os.path.basename(module) - addon_path = build._server('addons', basename) - if os.path.exists(addon_path): - build._log( - 'Building environment', - 'You have duplicate modules in your branches "%s"' % basename - ) - if os.path.islink(addon_path) or os.path.isfile(addon_path): - os.remove(addon_path) - else: - shutil.rmtree(addon_path) - shutil.move(module, build._server('addons')) + # todo make it backward compatible, or create migration script? + for build_dependency in build.dependency_ids: + closest_branch = build_dependency.closest_branch_id + latest_commit = build_dependency.dependency_hash + repo = closest_branch.repo_id + closest_name = closest_branch.name + if build_dependency.match_type == 'default': + server_match = 'default' + elif server_match != 'default': + server_match = 'match' - available_modules = [ - os.path.basename(os.path.dirname(a)) - for a in (glob.glob(build._server('addons/*/__openerp__.py')) + - glob.glob(build._server('addons/*/__manifest__.py'))) + build._log( + 'Building environment', + '%s match branch %s of %s' % (build_dependency.match_type, closest_name, repo.name) + ) + if not repo._hash_exists(latest_commit): + 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(latest_commit, build._path()) + + # Finally mark all addons to move to openerp/addons + modules_to_move += [ + os.path.dirname(module) + for module in (glob.glob(build._path('*/__openerp__.py')) + + glob.glob(build._path('*/__manifest__.py'))) ] - if build.repo_id.modules_auto == 'all' or (build.repo_id.modules_auto != 'none' and has_server): - modules_to_test += available_modules - modules_to_test = self._filter_modules(modules_to_test, - set(available_modules), explicit_modules) - _logger.debug("modules_to_test for build %s: %s", build.dest, modules_to_test) - build.write({'server_match': server_match, - 'modules': ','.join(modules_to_test)}) + # move all addons to server addons path + for module in uniq_list(glob.glob(build._path('addons/*')) + modules_to_move): + basename = os.path.basename(module) + addon_path = build._server('addons', basename) + if os.path.exists(addon_path): + build._log( + 'Building environment', + 'You have duplicate modules in your branches "%s"' % basename + ) + if os.path.islink(addon_path) or os.path.isfile(addon_path): + os.remove(addon_path) + else: + shutil.rmtree(addon_path) + shutil.move(module, build._server('addons')) + + available_modules = [ + os.path.basename(os.path.dirname(a)) + for a in (glob.glob(build._server('addons/*/__openerp__.py')) + + glob.glob(build._server('addons/*/__manifest__.py'))) + ] + if build.repo_id.modules_auto == 'all' or (build.repo_id.modules_auto != 'none' and has_server): + modules_to_test += available_modules + + modules_to_test = self._filter_modules(modules_to_test, + set(available_modules), explicit_modules) + _logger.debug("modules_to_test for build %s: %s", build.dest, modules_to_test) + build.write({'server_match': server_match, + 'modules': ','.join(modules_to_test)}) def _local_pg_dropdb(self, dbname): with local_pgadmin_cursor() as local_cr: diff --git a/runbot/models/build_dependency.py b/runbot/models/build_dependency.py new file mode 100644 index 00000000..179ce2be --- /dev/null +++ b/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') diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 575f216e..1d7b8a8a 100644 --- a/runbot/models/repo.py +++ b/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) - branch_id = Branch.create({'repo_id': repo.id, 'name': name}).id + _logger.debug('repo %s found new branch %s', self.name, name) + 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) diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index d6eebd3a..7cfdb245 100644 --- a/runbot/security/ir.model.access.csv +++ b/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 diff --git a/runbot/templates/build.xml b/runbot/templates/build.xml index 90894fa6..45b07c60 100644 --- a/runbot/templates/build.xml +++ b/runbot/templates/build.xml @@ -15,9 +15,9 @@ killed manually killed - + + title="Server branch cannot be determined exactly. Please use naming convention '12.0-my-branch' to build with '12.0' server branch."/> Dep builds: diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 3004c0f7..8b349fa1 100644 --- a/runbot/tests/test_build.py +++ b/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): - extra_repo = build.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)) + def assertClosest(self, branch, closest): + extra_repo = branch.repo_id.dependency_ids[0] + 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(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') - 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 assertNoDuplicate(self, branch1, branch2, b1_closest=None, b2_closest=None): + self.assertDuplicate(branch1, branch2, b1_closest=b1_closest, b2_closest=b2_closest, noDuplicate=True) 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, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) - self.assertEqual((self.enterprise_dev_repo.id, addons_branch.name, 'exact'), addons_build._get_closest_branch_name(self.enterprise_dev_repo.id)) + + self.assertEqual((addons_branch, 'exact'), addons_branch._get_closest_branch(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({ - '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)) + self.assertEqual((community_branch, 'exact PR'), enterprise_pr._get_closest_branch(self.community_repo.id)) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') - @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') - def test_closest_branch_02_improved(self, mock_branch_exists, mock_github): + @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote') + 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, + (com_dev_branch, 'exact'), + (com_dev_branch, 'exact PR') + ) - self.assertDuplicate( - ent_dev_branch, - ent_pr, - (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'), - (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact PR') - ) - - @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') - def test_closest_branch_03(self, mock_branch_exists): + @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote') + 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({ - '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)) + self.assertEqual((self.branch_odoo_10, 'prefix'), addons_branch._get_closest_branch(self.community_repo.id)) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') - @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') - def test_closest_branch_03_05(self, mock_branch_exists, mock_github): + @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote') + 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' }) - - self.assertDuplicate( - ent_pr, - ent_dev_branch, - (self.community_repo.id, 'refs/heads/saas-12.2', 'default'), - (self.community_repo.id, 'refs/heads/saas-12.2', 'prefix'), - ) + with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse): + self.assertDuplicate( + ent_pr, + ent_dev_branch, + (com_branch, 'pr_target'), + (com_branch, 'prefix'), + ) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') - @patch('odoo.addons.runbot.models.branch.runbot_branch._branch_exists') - def test_closest_branch_04(self, mock_branch_exists, mock_github): + @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote') + 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,13 +502,13 @@ class TestClosestBranch(common.TransactionCase): 'repo_id': self.enterprise_repo.id, 'name': 'refs/pull/3721' }) - - self.assertDuplicate( - ent_dev_branch, - ent_pr, - (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'), - (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'no PR') - ) + with patch('odoo.addons.runbot.models.repo.runbot_repo._git_rev_parse', new=rev_parse): + self.assertDuplicate( + ent_dev_branch, + ent_pr, + (com_dev_branch, 'exact'), + (com_dev_branch, 'no PR') + ) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') def test_closest_branch_05(self, mock_github): @@ -506,11 +532,7 @@ class TestClosestBranch(common.TransactionCase): 'repo_id': self.enterprise_repo.id, 'name': 'refs/pull/789101' }) - addons_build = self.Build.create({ - '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)) + self.assertEqual((self.branch_odoo_10, 'pr_target'), addons_pr._get_closest_branch(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({ - 'branch_id': addons_branch.id, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) + self.assertEqual((self.branch_odoo_master, 'default'), addons_branch._get_closest_branch(self.community_repo.id)) - 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.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'), + ) + + @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? diff --git a/runbot/tests/test_frontend.py b/runbot/tests/test_frontend.py index 05535f0e..01e69ed5 100644 --- a/runbot/tests/test_frontend.py +++ b/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',