From fde039efd9021d588bd14ed80b034edb0fcb5b9e Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 5 Feb 2026 16:15:20 +0100 Subject: [PATCH 01/38] [IMP] runbot: allow to filter on dependencies This commit adds the possibility to filter modules based on their dependencies and dependants in the build configuration. This allows to easily trigger tests on modules that are impacted by changes, even if they are not directly modified. To make it work the modules listing is now done only in the git repository, without exporting sources, this should help to easily to have faster builds when the only task is to create builds based on modified modules. This should also and mostly help to test a dynamic config without running it. --- runbot/container.py | 5 +- runbot/controllers/frontend.py | 9 +- runbot/models/batch.py | 9 +- runbot/models/build.py | 116 ++++++++--- runbot/models/build_config.py | 123 +++++++++--- runbot/models/commit.py | 47 +++-- runbot/models/repo.py | 12 +- runbot/templates/build.xml | 8 +- runbot/templates/bundle.xml | 9 +- runbot/tests/common.py | 12 +- runbot/tests/test_build_config_step.py | 256 +++++++++++++++++++++---- runbot/tests/test_repo.py | 4 +- runbot/views/build_views.xml | 2 + 13 files changed, 483 insertions(+), 129 deletions(-) diff --git a/runbot/container.py b/runbot/container.py index 679ea35f1..f52456c1e 100644 --- a/runbot/container.py +++ b/runbot/container.py @@ -259,7 +259,10 @@ def _docker_run(cmd=False, log_path=False, build_dir=False, container_name=False else: run_cmd = cmd run_cmd = f'cd /data/build;touch start-{container_name};{run_cmd};cd /data/build;touch end-{container_name}' - _logger.info('Docker run command: %s', run_cmd) + run_cmd_repr = str(run_cmd) + if len(run_cmd_repr) > 250: + run_cmd_repr = run_cmd_repr[:250] + '...' + _logger.info('Docker run command: %s', run_cmd_repr) docker_clear_state(container_name, build_dir) # ensure that no state are remaining build_dir = file_path(build_dir) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 3110aadd4..a8f15e44a 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -323,7 +323,8 @@ def build(self, build_id, search=None, from_batch=None, **post): @route([ '/runbot/build/search', ], website=True, auth='public', type='http', sitemap=False) - def builds(self, **kwargs): + def builds(self, limit=100, **kwargs): + limit = min(int(limit), 1000) domain = [] for key in ('config_id', 'version_id', 'project_id', 'trigger_id', 'create_batch_id.bundle_id', 'create_batch_id'): # allowed params value = kwargs.get(key) @@ -337,10 +338,12 @@ def builds(self, **kwargs): for key in ('description',): if key in kwargs: - domain.append((f'{key}', 'ilike', kwargs.get(key))) + value = kwargs.get(key) + operator = 'ilike' if '%' in value else '=' + domain.append((f'{key}', operator, value)) context = { - 'builds': request.env['runbot.build'].search(domain, limit=100), + 'builds': request.env['runbot.build'].search(domain, limit=limit), } return request.render('runbot.build_search', context) diff --git a/runbot/models/batch.py b/runbot/models/batch.py index 331f44b51..6cd395827 100644 --- a/runbot/models/batch.py +++ b/runbot/models/batch.py @@ -15,6 +15,7 @@ class Batch(models.Model): last_update = fields.Datetime('Last ref update') bundle_id = fields.Many2one('runbot.bundle', required=True, index=True, ondelete='cascade') + build_all = fields.Boolean('Force all triggers') commit_link_ids = fields.Many2many('runbot.commit.link') commit_ids = fields.Many2many('runbot.commit', compute='_compute_commit_ids') slot_ids = fields.One2many('runbot.batch.slot', 'batch_id') @@ -187,6 +188,7 @@ def _prepare(self, auto_rebase=False, use_base_commits=False): priority_offset = self.bundle_id.priority_offset if not priority_offset and self.bundle_id.branch_ids.forwardport_of_id and self.bundle_id.last_batchs == self: # this is the only batch of a forwardported pr. priority_offset = - 3600 * 5 + self.build_all = True # for normal pr, mergebot will request all ci on r+ if needed, for forward port, we need to ensure they are all created or the chain could be blocked self.priority_level = int(self.create_date.timestamp() - priority_offset) if use_base_commits: self._warning('This batch will use base commits instead of bundle commits') @@ -383,7 +385,7 @@ def _fill_missing(branch_commits, match_type): continue # in any case, search for an existing build config = trigger.config_id - if not trigger_custom and trigger.light_config_id and not bundle.build_all and not bundle.is_staging and not bundle.is_base: + if not trigger_custom and trigger.light_config_id and not bundle.build_all and not self.build_all and not bundle.is_staging and not bundle.is_base: if (project.use_light_default or project.use_light_draft and any(branch.draft for branch in self.bundle_id.branch_ids) @@ -455,7 +457,10 @@ def _start_builds(self): is_dev = not bundle.is_staging and not bundle.is_base for trigger in self.slot_ids.trigger_id: enable_on_bundle = (trigger.on_staging and bundle.is_staging) or (trigger.on_base and bundle.is_base) or (trigger.on_dev and is_dev) - if ((trigger.repo_ids & bundle_repos) or bundle.build_all or bundle.sticky) and enable_on_bundle: + common_repo = (trigger.repo_ids & bundle_repos) + if self.build_all and not common_repo: + common_repo = (trigger.dependency_ids & bundle_repos) + if (common_repo or bundle.build_all or bundle.sticky) and enable_on_bundle: should_start_triggers_ids.add(trigger.id) disabled_triggers = self.bundle_id.all_trigger_custom_ids.filtered(lambda tc: tc.start_mode == 'disabled').trigger_id diff --git a/runbot/models/build.py b/runbot/models/build.py index 2a5ab26d6..340aec464 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- - +import ast import datetime import getpass import hashlib @@ -10,23 +10,36 @@ import shutil import time import uuid - from collections import defaultdict -from dateutil import parser from pathlib import Path + +from dateutil import parser from psycopg2 import sql from psycopg2.extensions import TransactionRollbackError -from ..common import dt2time, now, grep, local_pgadmin_cursor, dest_reg, os, list_local_dbs, pseudo_markdown, RunbotException, findall, sanitize, markdown_escape, tail -from ..container import docker_stop, docker_state, Command, docker_run, docker_pull -from ..fields import JsonDictField - -from odoo import models, fields, api - +from odoo import api, fields, models from odoo.exceptions import ValidationError from odoo.tools import file_open, file_path from odoo.tools.safe_eval import safe_eval +from ..common import ( + RunbotException, + dest_reg, + dt2time, + findall, + grep, + list_local_dbs, + local_pgadmin_cursor, + markdown_escape, + now, + os, + pseudo_markdown, + sanitize, + tail, + transactioncache, +) +from ..container import Command, docker_pull, docker_run, docker_state, docker_stop +from ..fields import JsonDictField _logger = logging.getLogger(__name__) @@ -61,7 +74,6 @@ def remove_readonly(func, path_str, exinfo): def make_selection(array): return [(elem, elem.replace('_', ' ').capitalize()) if isinstance(elem, str) else elem for elem in array] - class BuildParameters(models.Model): _name = 'runbot.build.params' _description = "Build parameters" @@ -1091,25 +1103,28 @@ def _checkout(self): return exports + def _list_available_modules(self): + for commit in self.env.context.get('defined_commit_ids') or self.params_id.commit_ids: + for (addons_path, module, manifest_file_name) in commit._list_available_modules(): + yield commit, addons_path, module, manifest_file_name + def _get_available_modules(self): all_modules = dict() available_modules = defaultdict(list) # repo_modules = [] - for commit in self.env.context.get('defined_commit_ids') or self.params_id.commit_ids: - for (addons_path, module, manifest_file_name) in commit._get_available_modules(): - if module in all_modules: - self._log( - 'Building environment', - '%s is a duplicated modules (found in "%s", already defined in %s)' % ( - module, - commit._source_path(addons_path, module, manifest_file_name), - all_modules[module]._source_path(addons_path, module, manifest_file_name)), - level='WARNING', - ) - else: - available_modules[commit.repo_id].append(module) - all_modules[module] = commit - # return repo_modules, available_modules + for commit, addons_path, module, manifest_file_name in self._list_available_modules(): + if module in all_modules: + self._log( + 'Building environment', + '%s is a duplicated modules (found in "%s", already defined in %s)' % ( + module, + commit._source_path(addons_path, module, manifest_file_name), + all_modules[module]._source_path(addons_path, module, manifest_file_name)), + level='WARNING', + ) + else: + available_modules[commit.repo_id].append(module) + all_modules[module] = commit return available_modules def _get_modules_to_test(self, modules_patterns=''): @@ -1120,6 +1135,49 @@ def _get_modules_to_test(self, modules_patterns=''): modules_patterns = (modules_patterns or '').split(',') return trigger._filter_modules_to_test(modules, params_patterns + modules_patterns) # we may switch params_patterns and modules_patterns order + @transactioncache + def _dependency_graph(self): + dependency_graph = defaultdict(set) + dependant_graph = defaultdict(set) + for commit in self.env.context.get('defined_commit_ids') or self.params_id.commit_ids: + file_paths = [] + modules = [] + for (addons_path, module, manifest_file_name) in commit._list_available_modules(): + file_paths.append(os.path.join(addons_path, module, manifest_file_name)) + modules.append(module) + contents = commit._git_show_files(file_paths) + for module, manifest in zip(modules, contents): + manifest_content = ast.literal_eval(manifest) + depends = manifest_content.get('depends', []) + if not depends and module != 'base': + depends = ['base'] + for dep in depends: + dependency_graph[module].add(dep) + dependant_graph[dep].add(module) + return dependency_graph, dependant_graph + + def search_modules_graph(self, modules, graph, depth=None): + def search(modules, depth=None, visited=None): + visited = visited or set() + modules = set(modules) - visited + visited |= modules + dependencies = set(modules) + if depth == 0 or not modules: + return dependencies + for module in modules: + dependencies |= search(graph[module], depth - 1 if depth is not None else None, visited) + return dependencies + return sorted(search(modules, depth)) + + def _get_modules_dependencies(self, modules, depth=None): + self.ensure_one() + dependency_graph, _ = self._dependency_graph() + return self.search_modules_graph(modules, dependency_graph, depth) + + def _get_dependant_modules(self, modules, depth=None): + _, dependant_graph = self._dependency_graph() + return self.search_modules_graph(modules, dependant_graph, depth) + def _local_pg_dropdb(self, dbname): msg = '' try: @@ -1249,13 +1307,17 @@ def _modified_files(self, commit_link_links=None): modified_files[commit_link] = files return modified_files - def _modified_modules(self, commit_link_links=None): + def _modified_modules(self, commit_link_links=None, defaults=None): modified_files = self._modified_files(commit_link_links) modified_modules = set() for commit_link, files in modified_files.items(): commit = commit_link.commit_id for file in files: - modified_modules.add(commit.repo_id._get_module(file)) + module = commit.repo_id._get_module(file) + if module: + modified_modules.add(module) + elif defaults: + modified_modules |= set(defaults) return modified_modules def _get_upgrade_path(self): diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 19f8c908a..0c86e5eb8 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -46,8 +46,21 @@ def filter_all_modules(selector, build, dynamic_vars): return filter_default_modules(selector, build, dynamic_vars) +def get_dependencies(modules, build, dynamic_vars, depth=None): + depth = int(depth) if depth else None + modules = modules.split(',') + dependant = set(build._get_modules_dependencies(modules, depth)) - set(modules) + return ','.join(sorted(dependant)) + + +def get_dependant(modules, build, dynamic_vars, depth=None): + depth = int(depth) if depth else None + modules = modules.split(',') + dependant = set(build._get_dependant_modules(modules, depth)) - set(modules) + return ','.join(sorted(dependant)) + + def filter_default_modules(selector, build, dynamic_vars): - build._checkout() # we need to ensure source are exported before _get_modules_to_test modules = build._get_modules_to_test(selector) return ','.join(modules) @@ -57,22 +70,17 @@ def select_existing_modules(selector, build, dynamic_vars): return filter_default_modules(selector, build, dynamic_vars) -def keep_modified_modules(modules, build, dynamic_vars): +def keep_modified_modules(modules, build, dynamic_vars, *defaults): if build.params_id.config_data.get('skip_modified_modules_filter', False): return modules - modified_modules = build._modified_modules() + if defaults: + defaults = [d[1:-1] if re.match(r'^[\'"].*[\'"]$', d) else d for d in defaults] + modified_modules = build._modified_modules(defaults=defaults) modules = modules.split(',') filtered_modules = [module for module in modules if module in modified_modules] return ','.join(filtered_modules) -def keep_modified_modules_or_base(modules, build, dynamic_vars): - bundle = build.params_id.create_batch_id.bundle_id - if bundle.is_base or bundle.is_staging: - return modules - return keep_modified_modules(modules, build, dynamic_vars) - - def make_module_test_tags(modules, build, dynamic_vars): return ','.join([f'/{module}' for module in modules.split(',')]) @@ -93,6 +101,17 @@ def append_string(modules, build, dynamic_vars, element): return ','.join([f'{module}{element}' for module in modules.split(',')]) +def union(modules, build, dynamic_vars, element): + if re.match(r'^[\'"].*[\'"]$', element): + element = element[1:-1] + else: + element = dynamic_vars.get(element, element) + element = element.strip() + modules = set(modules.split(',')) if modules else set() + new_modules = set(element.split(',')) if element else set() + return ','.join(sorted(modules | new_modules)) + + class Config(models.Model): _name = 'runbot.build.config' _description = "Build config" @@ -227,11 +246,15 @@ def wrapper(value, path): return wrapper def VARS(vars, path): - if not isinstance(vars, dict): - raise ValidationError(f'{path} ({vars}) should be a dict') - for key, val in vars.items(): - TECHNICAL_NAME(key, f'{path}.{key}') - STR(val, f'{path}.{key}') + if isinstance(vars, list): + for item in vars: + VARS(item, path) + else: + if not isinstance(vars, dict): + raise ValidationError(f'{path} ({vars}) should be a dict') + for key, val in vars.items(): + TECHNICAL_NAME(key, f'{path}.{key}') + STR(val, f'{path}.{key}') NAME = str_checker(r'^[\w \-]+$') STR = str_checker(r'.*') @@ -246,6 +269,7 @@ def VARS(vars, path): 'vars': OPTIONAL(VARS), 'steps': REQUIRED(LIST(STEP)), 'description': OPTIONAL(DYNAMIC_VALUE), + 'log': OPTIONAL(DYNAMIC_VALUE), } valid_steps['odoo'] = { 'name': REQUIRED(NAME), @@ -260,6 +284,7 @@ def VARS(vars, path): 'cpu_limit': OPTIONAL(INT), 'export_database': OPTIONAL(BOOL), 'make_stats': OPTIONAL(BOOL), + 'log': OPTIONAL(DYNAMIC_VALUE), } valid_steps['create_build'] = { 'name': REQUIRED(NAME), @@ -268,6 +293,8 @@ def VARS(vars, path): 'for_each_vars': OPTIONAL(LIST(VARS)), 'for_each_module': OPTIONAL(DYNAMIC_VALUE), 'max_builds': OPTIONAL(INT), + 'if': OPTIONAL(DYNAMIC_VALUE), + 'log': OPTIONAL(DYNAMIC_VALUE), } valid_steps['restore'] = { 'name': REQUIRED(NAME), @@ -277,6 +304,7 @@ def VARS(vars, path): 'trigger_id': OPTIONAL(INT), 'use_current_batch': OPTIONAL(BOOL), 'zip_url': OPTIONAL(STR), + 'log': OPTIONAL(DYNAMIC_VALUE), } valid_steps['command'] = { 'name': REQUIRED(NAME), @@ -289,6 +317,7 @@ def VARS(vars, path): 'check_logs': OPTIONAL(LIST(STR)), 'expected_logs': OPTIONAL(LIST(STR)), 'make_stats': OPTIONAL(BOOL), + 'log': OPTIONAL(DYNAMIC_VALUE), } validate(config_schema, config, 'config') @@ -1192,7 +1221,7 @@ def _coverage_params(self, build, modules_to_install): docker_source_folder = build._docker_source_folder(commit) for manifest_file in commit.repo_id.manifest_files.split(','): pattern_to_omit.add('*%s' % manifest_file) - for (addons_path, module, _) in commit._get_available_modules(): + for (addons_path, module, _) in commit._list_available_modules(): if module not in modules_to_install: # we want to omit docker_source_folder/[addons/path/]module/* module_path_in_docker = os.sep.join([docker_source_folder, addons_path, module]) @@ -1505,35 +1534,70 @@ def _run_dynamic(self, build): raise RunbotException('Too many ancestors builds, possible cyclic dynamic build creation') if build.parent_id and build.dynamic_config == build.parent_id.dynamic_config: raise RunbotException('A child build cannot load the same dynamic config if parent, recursion detected') + + config_vars_list = build.dynamic_config.get('vars', {}) + if not isinstance(config_vars_list, list): + config_vars_list = [config_vars_list] + raw_vars = {} + for config_vars in config_vars_list: + raw_vars.update(config_vars) + + raw_vars.update(build.params_id.config_data.get('dynamic_vars', {})) + dynamic_vars = {} + # dynamic_vars can either be raw value like 'account', value to evaluate lazily in anothed dynamic value like 'account->!mail' + # or dynamic value that we want to evaluate early like '{{*|filter_all_modules|modified_modules}}' (between {{}}) + # this loop will evalute the third category + # this alows to evaluate only once an expression that could be expensive to use it in multiple dynamic values + # this also allow to clarify the config by chaining vars definition + # TODO check ordering + for key, value in raw_vars.items(): + dynamic_vars[key] = self._parse_dynamic_entry(value, build, dynamic_vars=dynamic_vars) + current_step = self._get_dynamic_step(build) if not current_step: build._log('Dynamic Step', 'No dynamic config or steps found, skipping', level="WARNING") return + if current_step.get('log'): + text = self._parse_dynamic_entry(current_step['log'], build, dynamic_vars=dynamic_vars) + build._log('_run_dynamic', text) if current_step['job_type'] == 'create_build': for_each_vars_list = current_step.get('for_each_vars', [{}]) if 'for_each_module' in current_step: modules_vars = [] for for_each_vars in for_each_vars_list: - modules_entry = self._parse_dynamic_entry(current_step['for_each_module'], build, additional_dynamic_vars=for_each_vars) + modules_entry = self._parse_dynamic_entry(current_step['for_each_module'], build, dynamic_vars={**dynamic_vars, **for_each_vars}) modules = [m.strip() for m in modules_entry.split(',') if m.strip()] for module in modules: module_vars = {**for_each_vars, 'module': module} modules_vars.append(module_vars) for_each_vars_list = modules_vars - parent_vars = {**build.dynamic_config.get('vars', {}), **build.params_id.config_data.get('dynamic_vars', {})} + child_data_list = [] for child_index, child in enumerate(current_step.get('children', [])): child_vars = child.get('vars', {}) for for_each_vars in for_each_vars_list: config_name = child.get('name', build.params_id.config_id.name) - dynamic_vars = {**parent_vars, **child_vars, **for_each_vars} + raw_dynamic_vars = {**dynamic_vars, **for_each_vars, **child_vars} + child_dynamic_vars = {} + # evaluate for_each_vars + for key, value in raw_dynamic_vars.items(): + child_dynamic_vars[key] = self._parse_dynamic_entry(value, build, dynamic_vars=child_dynamic_vars) + if 'if' in current_step: + condition = self._parse_dynamic_entry(current_step['if'], build, dynamic_vars=child_dynamic_vars) + if not condition: + continue if 'description' in child: - description = self._parse_dynamic_entry(child['description'], build, additional_dynamic_vars=dynamic_vars) + description = self._parse_dynamic_entry(child['description'], build, dynamic_vars=child_dynamic_vars) # note: we mainly need to provide additional_dynamic_vars because the child is not created yet at this point else: description = config_name + # filter vars not prefixed with _ to simplify child values + if child.get('log'): + text = self._parse_dynamic_entry(child['log'], build, dynamic_vars=child_dynamic_vars) + build._log('_run_dynamic', text) + public_child_dynamic_vars = {key: value for key, value in child_dynamic_vars.items() if not key.startswith('_')} child_data = { - 'config_data': {**build.params_id.config_data.dict, "dynamic_vars": dynamic_vars}, + 'config_data': {**build.params_id.config_data.dict, "dynamic_vars": public_child_dynamic_vars}, 'config_id': build.params_id.config_id.id, 'dynamic_active_step_index': 0, 'dynamic_config_position': f'{build.params_id.dynamic_config_position or ""}/{build.dynamic_active_step_index}.{child_index}', @@ -1564,14 +1628,14 @@ def _run_dynamic(self, build): install_modules_pattern = current_step.get('install_modules', '') if install_modules_pattern.split(',', 1)[0] not in ('*', '-*'): install_modules_pattern = '-*,' + install_modules_pattern - config_data['install_module_pattern'] = self._parse_dynamic_entry(install_modules_pattern, build) + config_data['install_module_pattern'] = self._parse_dynamic_entry(install_modules_pattern, build, dynamic_vars) if 'test_tags' in current_step: - config_data['test_tags'] = self._parse_dynamic_entry(current_step.get('test_tags'), build) + config_data['test_tags'] = self._parse_dynamic_entry(current_step.get('test_tags'), build, dynamic_vars) config_data['test_enable'] = bool(current_step.get('test_enable') or current_step.get('test_tags')) if 'extra_params' in current_step: - config_data['extra_params'] = self._parse_dynamic_entry(current_step.get('extra_params'), build) + config_data['extra_params'] = self._parse_dynamic_entry(current_step.get('extra_params'), build, dynamic_vars) for key in ('screencast', 'demo_mode', 'enable_auto_tags'): if key in current_step: @@ -1593,6 +1657,7 @@ def _run_dynamic(self, build): 'addons_path': ",".join(build._get_addons_path()), 'exports': ",".join(exports.keys()), 'exports_paths': ",".join(exports.values()), + **dynamic_vars, } command = [shlex.quote(self._parse_dynamic_entry(part, build, values)) for part in command] pres = [] @@ -1614,23 +1679,23 @@ def _get_dynamic_db_suffix(self, step): db_suffix = re.sub(r'[^a-z0-9_\-]', '_', db_suffix.lower()) return db_suffix - def _parse_dynamic_entry(self, entry, build, additional_dynamic_vars=None): + def _parse_dynamic_entry(self, entry, build, dynamic_vars): """ transforms a module/test-tags entry dynamically """ - dynamic_config = build.dynamic_config - expression_filters = { 'filter_all_modules': filter_all_modules, 'filter_default_modules': filter_default_modules, 'make_module_test_tags': make_module_test_tags, 'select_existing_modules': select_existing_modules, + 'get_dependencies': get_dependencies, + 'get_dependant': get_dependant, 'prepend': prepend_string, 'append': append_string, 'modified_modules': keep_modified_modules, - 'modified_modules_or_base': keep_modified_modules_or_base, + 'union': union, } - dynamic_vars = {**dynamic_config.get('vars', {}), **build.params_id.config_data.get('dynamic_vars', {}), **(additional_dynamic_vars or {})} + dynamic_vars = dynamic_vars or {} def parse_expression(match): # inspired by jinja but with limited features diff --git a/runbot/models/commit.py b/runbot/models/commit.py index b7423f494..18d6922d8 100644 --- a/runbot/models/commit.py +++ b/runbot/models/commit.py @@ -3,7 +3,6 @@ import subprocess from ..common import os, RunbotException, make_github_session, transactioncache -import glob import shutil from odoo import models, fields, api @@ -66,22 +65,14 @@ def _rebase_on(self, commit): return self return self._get(self.name, self.repo_id.id, self.read()[0], commit.id) - def _get_available_modules(self): - for manifest_file_name in self.repo_id.manifest_files.split(','): # '__manifest__.py' '__openerp__.py' - for addons_path in (self.repo_id.addons_paths or '').split(','): # '' 'addons' 'odoo/addons' - sep = os.path.join(addons_path, '*') - for manifest_path in glob.glob(self._source_path(sep, manifest_file_name)): - module = os.path.basename(os.path.dirname(manifest_path)) - yield (addons_path, module, manifest_file_name) - def _list_files(self, patterns): #example: git ls-files --with-tree=abcf390f90dbdd39fd61abc53f8516e7278e0931 ':(glob)addons/*/*.py' ':(glob)odoo/addons/*/*.py' # note that glob is needed to avoid the star matching ** self.ensure_one() + self._fetch() return self.repo_id._git(['ls-files', '--with-tree', self.name, *patterns]).split('\n') def _list_available_modules(self): - # beta version, may replace _get_available_modules latter addons_paths = (self.repo_id.addons_paths or '').split(',') patterns = [] for manifest_file_name in self.repo_id.manifest_files.split(','): # '__manifest__.py' '__openerp__.py' @@ -98,6 +89,7 @@ def _list_available_modules(self): module, manifest_file_name = elems yield (addons_path, module, manifest_file_name) + @transactioncache # hack to avoid to fetch two time the same commit inside the same transaction def _fetch(self): self.repo_id._fetch(self.name) if not self.repo_id._hash_exists(self.name): @@ -170,12 +162,43 @@ def _read_source(self, file, mode='r'): @transactioncache def _git_show_file(self, file): + return self._git_show_files([file])[0] + + def _git_show_files(self, files): self.ensure_one() + if not files: + return [] + self.repo_id._fetch(self.name) + + queries = "\n".join([f"{self.name}:{f}" for f in files]) + "\n" + try: - return self.repo_id._git(['show', '%s:%s' % (self.name, file)]) + buffer = self.repo_id._git( + ['cat-file', '--batch'], + input_data=queries, + raw=True, + ) except subprocess.CalledProcessError: - return False + return [False] * len(files) + + results = [] + offset = 0 + buffer_len = len(buffer) + while offset < buffer_len: + newline_idx = buffer.find(b'\n', offset) + if newline_idx == -1: + break + header = buffer[offset:newline_idx].decode('utf-8') + offset = newline_idx + 1 + try: + size_in_bytes = int(header.rsplit(' ', 1)[-1]) + except ValueError: # most likely missing + results.append(False) + continue + results.append(buffer[offset : offset + size_in_bytes].decode('utf-8', errors='replace')) + offset += size_in_bytes + 1 + return results def _source_path(self, *paths): if not self.tree_hash: diff --git a/runbot/models/repo.py b/runbot/models/repo.py index db7543a9d..175721603 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -503,11 +503,19 @@ def _get_git_command(self, cmd, errors='strict'): cmd = ['git', '-C', self.path] + config_args + cmd return cmd - def _git(self, cmd, errors='strict', quiet=False): + def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False): cmd = self._get_git_command(cmd, errors) if not quiet: _logger.info("git command: %s", shlex.join(cmd)) - return subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode(errors=errors) + kwargs = {'stderr': subprocess.STDOUT} + if input_data is not None: + if isinstance(input_data, str): + input_data = input_data.encode('utf-8') + kwargs['input'] = input_data + output = subprocess.check_output(cmd, **kwargs) + if raw: + return output + return output.decode(errors=errors) def _fetch(self, sha): if not self._hash_exists(sha): diff --git a/runbot/templates/build.xml b/runbot/templates/build.xml index 0e50b7ce4..3c701df09 100644 --- a/runbot/templates/build.xml +++ b/runbot/templates/build.xml @@ -44,7 +44,7 @@
  • - +
  • @@ -68,7 +68,7 @@
    -
    +
    This build is referenced in bundles
    @@ -225,7 +225,7 @@
    - + Build @@ -294,7 +294,7 @@ - + diff --git a/runbot/templates/bundle.xml b/runbot/templates/bundle.xml index af1036a40..49b4a5efe 100644 --- a/runbot/templates/bundle.xml +++ b/runbot/templates/bundle.xml @@ -91,14 +91,9 @@ Default - - - - Force all - - + Apply -
    +
    diff --git a/runbot/tests/common.py b/runbot/tests/common.py index 9f6ba86c8..81cc8e784 100644 --- a/runbot/tests/common.py +++ b/runbot/tests/common.py @@ -12,7 +12,7 @@ class RunbotCase(TransactionCase): - def mock_git_helper(self, repo, cmd): + def mock_git_helper(self, repo, cmd, input_data=None, raw=False): """Helper that returns a mock for repo._git()""" if cmd[:2] == ['show', '-s'] or cmd[:3] == ['show', '--pretty="%H -- %s"', '-s']: return 'commit message for %s' % cmd[-1] @@ -82,7 +82,9 @@ def setUp(self): self.repo_odoo: [ ('odoo/addons', 'base', '__manifest__.py'), ('odoo/addons', 'test_lint', '__manifest__.py'), + ('addons', 'account', '__manifest__.py'), ('addons', 'mail', '__manifest__.py'), + ('addons', 'test_mail', '__manifest__.py'), ('addons', 'web', '__manifest__.py'), ('addons', 'crm', '__manifest__.py'), ('addons', 'project', '__manifest__.py'), @@ -194,8 +196,8 @@ def setUp(self): self.docker_run_calls = [] self.diff = '' - def mock_git(repo, cmd, quiet=False): - return self.mock_git_helper(repo, cmd) + def mock_git(repo, cmd, quiet=False, input_data=None, raw=False): + return self.mock_git_helper(repo, cmd, input_data=input_data, raw=raw) self.start_patcher('git_patcher', 'odoo.addons.runbot.models.repo.Repo._git', new=mock_git) self.start_patcher('hostname_patcher', 'odoo.addons.runbot.common.socket.gethostname', 'host.runbot.com') @@ -232,10 +234,10 @@ def mock_git(repo, cmd, quiet=False): self.start_patcher('_write_file', 'odoo.addons.runbot.models.build.BuildResult._write_file', None) self.start_patcher('_parse_config', 'odoo.addons.runbot.models.build.BuildResult._parse_config', {'--test-enable', '--test-tags', '--with-demo'}) - def get_available_modules(self_commit): + def _list_available_modules(self_commit): return self.addons_per_repo.get(self_commit.repo_id, []) - self.start_patcher('_get_available_modules', 'odoo.addons.runbot.models.commit.Commit._get_available_modules', new=get_available_modules) + self.start_patcher('_list_available_modules', 'odoo.addons.runbot.models.commit.Commit._list_available_modules', new=_list_available_modules) def no_commit(*_args, **_kwargs): _logger.info('Skipping commit') diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 063c318ec..2e52171a3 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -439,25 +439,58 @@ def setUp(self): }).id, 'local_result': 'ok', }) + self.module_dependencies = { + "test_mail": ["mail"], + "mail": ["web"], + "account": ["web"], + "crm": ["web"], + "project": ["web"], + "test_l10n": ["l10n_be", "l10n_in"], + "l10n_be": ["account"], + "l10n_in": ["account"], + "web_enterprise": ["web"], + } + + def mock_git_helper(self, repo, cmd, input_data=None, raw=False): + def make_catfile_output(commit, content): + content_bytes = content.encode('utf-8') + header = f"{commit} blob {len(content_bytes)}\n".encode() + result = header + content_bytes + b"\n" + return result + + if cmd == ['cat-file', '--batch']: + if repo == self.repo_odoo and input_data == 'dfdfcfcf0000ffffffffffffffffffffffffffff:odoo/tests/.runbot/parallel_testing.json\n': + return make_catfile_output('dfdfcfcf0000ffffffffffffffffffffffffffff', self.config_file) + if repo == self.repo_odoo and input_data == 'dfdfcfcf0000ffffffffffffffffffffffffffff:odoo/tests/.runbot/l10n_standalone_testing.json\n': + return make_catfile_output('dfdfcfcf0000ffffffffffffffffffffffffffff', self.l10n_standalone_testing_file) + + if "__manifest__.py" in input_data: + modules_info = [ + (line, line.split(':')[-1].split('/')[-2]) + for line in input_data.splitlines() + if line.endswith('__manifest__.py') + ] + result = b"" + for original_query, module in modules_info: + content = '''{'name': '%s', 'depends': %s}''' % (module, self.module_dependencies.get(module, [])) + result += make_catfile_output(original_query.split(':')[0], content) + return result - def mock_git_helper(self, repo, cmd): - if repo == self.repo_odoo and cmd == ['show', 'dfdfcfcf0000ffffffffffffffffffffffffffff:odoo/tests/.runbot/parallel_testing.json']: - return self.config_file - elif repo == self.repo_odoo and cmd == ['show', 'dfdfcfcf0000ffffffffffffffffffffffffffff:odoo/tests/.runbot/l10n_standalone_testing.json']: - return self.l10n_standalone_testing_file - elif 'show' in cmd: + if cmd == ['cat-file', '--batch']: raise subprocess.CalledProcessError(cmd, 128) - return super().mock_git_helper(repo, cmd) + elif 'diff' in cmd: + return 'odoo/addons/crm/some/file.py\nodoo/addons/project/some/file.py' + return super().mock_git_helper(repo, cmd, input_data, raw) def test_module_filters(self): - self.assertEqual(self.build._get_modules_to_test('-> !mail'), ['base', 'crm', 'documents']) - self.assertEqual(self.build._get_modules_to_test('mail -> !web'), ['mail', 'project', 'test_l10n', 'test_lint']) + self.assertEqual(self.build._get_modules_to_test('-> !mail'), ['account', 'base', 'crm', 'documents']) + self.assertEqual(self.build._get_modules_to_test('mail -> !web'), ['mail', 'project', 'test_l10n', 'test_lint', 'test_mail']) self.assertEqual(self.build._get_modules_to_test('web -> web'), ['web']) self.assertEqual(self.build._get_modules_to_test('!web ->'), ['web_enterprise']) - self.assertEqual(self.build._get_modules_to_test('-> !mail, -crm'), ['base', 'documents']) - self.assertEqual(self.build._get_modules_to_test('mail -> !web, !project'), ['mail', 'test_l10n', 'test_lint']) - self.assertEqual(self.build._get_modules_to_test('-*,odoo/*'), ['base', 'crm', 'hw_drivers', 'mail', 'project', 'test_l10n', 'test_lint', 'web']) - self.assertEqual(self.build._get_modules_to_test('-*,odoo/test_*'), ['test_l10n', 'test_lint']) + self.assertEqual(self.build._get_modules_to_test('-> !mail, -crm'), ['account', 'base', 'documents']) + self.assertEqual(self.build._get_modules_to_test('mail -> !web, !project'), ['mail', 'test_l10n', 'test_lint', 'test_mail']) + self.assertEqual(self.build._get_modules_to_test('-*,odoo/*'), ['account', 'base', 'crm', 'hw_drivers', 'mail', 'project', 'test_l10n', 'test_lint', 'test_mail', 'web']) + self.assertEqual(self.build._get_modules_to_test('-*,odoo/test_*'), ['test_l10n', 'test_lint', 'test_mail']) self.assertEqual(self.build._get_modules_to_test('-*,enterprise/*'), ['documents', 'l10n_be', 'l10n_in', 'web_enterprise']) self.assertEqual(self.build._get_modules_to_test('-*,web*'), ['web', 'web_enterprise']) self.assertEqual(self.build._get_modules_to_test('-*,web*,-enterprise/web*'), ['web']) @@ -467,6 +500,35 @@ def test_config_extension(self): self.assertEqual(json.loads(self.config.default_dynamic_config)['vars']['module_filter'], '*,-hw_*') self.assertEqual(self.build.dynamic_config['vars']['module_filter'], '*,-hw_*,-l10n_*') + def test_parse_dynamic_entry(self): + Step = self.env['runbot.build.config.step'] + + def check_parse(entry, expected): + res = Step._parse_dynamic_entry(entry, self.build, {'key': 'value', 'test_method': '.test_method'}) + self.assertEqual(res, expected) + check_parse('{{-test_*|filter_all_modules}}', 'account,base,crm,documents,hw_drivers,l10n_be,l10n_in,mail,project,web,web_enterprise') + check_parse('{{-*,web*|filter_all_modules}}', 'web,web_enterprise') + check_parse('{{-*,web*|filter_all_modules|make_module_test_tags}}', '/web,/web_enterprise') + check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|prepend("some_tag")}}', 'some_tag/web,some_tag/web_enterprise') + check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|prepend(key)}}', 'value/web,value/web_enterprise') + check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|append(".test_method")}}', '/web.test_method,/web_enterprise.test_method') + check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|append(test_method)}}', '/web.test_method,/web_enterprise.test_method') + + self.patch(type(self.build), '_modified_modules', lambda cl, defaults=None: {'crm'}) + + check_parse('{{*|filter_all_modules|modified_modules}}', 'crm') + + def test_modules_dependencies(self): + self.assertEqual(self.build._get_modules_dependencies(['test_mail'], 1), ['mail', 'test_mail']) + self.assertEqual(self.build._get_modules_dependencies(['test_mail']), ['base', 'mail', 'test_mail', 'web']) + self.assertEqual(self.build._get_modules_dependencies(['test_l10n']), ['account', 'base', 'l10n_be', 'l10n_in', 'test_l10n', 'web']) + self.assertEqual(self.build._get_modules_dependencies(['test_mail', 'test_l10n']), ['account', 'base', 'l10n_be', 'l10n_in', 'mail', 'test_l10n', 'test_mail', 'web']) + self.assertEqual(self.build._get_modules_dependencies(['test_mail', 'test_l10n'], 1), ['l10n_be', 'l10n_in', 'mail', 'test_l10n', 'test_mail']) + + self.assertEqual(self.build._get_dependant_modules(['account'], 1), ['account', 'l10n_be', 'l10n_in']) + self.assertEqual(self.build._get_dependant_modules(['account']), ['account', 'l10n_be', 'l10n_in', 'test_l10n']) + self.assertEqual(self.build._get_dependant_modules(['base']), ['account', 'base', 'crm', 'documents', 'hw_drivers', 'l10n_be', 'l10n_in', 'mail', 'project', 'test_l10n', 'test_lint', 'test_mail', 'web', 'web_enterprise']) + def check_server_cmd(self, cmd, install, test_enable, test_tags, db=None): self.assertIn('odoo/server.py', cmd) if install: @@ -537,7 +599,7 @@ def test_dynamic_step_parallel_testing(self): cmd = self.docker_run_calls[0][0] odoo_cmd = cmd.cmd self.check_server_cmd(odoo_cmd, - install=['base', 'crm', 'documents', 'mail', 'project', 'test_l10n', 'test_lint', 'web', 'web_enterprise'], + install=['account', 'base', 'crm', 'documents', 'mail', 'project', 'test_l10n', 'test_lint', 'test_mail', 'web', 'web_enterprise'], test_enable=False, test_tags=None, db=f'{build.dest}-all', @@ -572,7 +634,7 @@ def test_dynamic_step_parallel_testing(self): cmd = self.docker_run_calls[0][0] odoo_cmd = cmd.cmd self.check_server_cmd(odoo_cmd, - install=['base', 'crm', 'documents', 'mail', 'project', 'test_l10n', 'test_lint', 'web', 'web_enterprise'], + install=['account', 'base', 'crm', 'documents', 'mail', 'project', 'test_l10n', 'test_lint', 'test_mail', 'web', 'web_enterprise'], test_enable=True, test_tags='-post_install,-/test_lint', ) @@ -589,8 +651,8 @@ def test_dynamic_step_parallel_testing(self): ) for post_install, expected_tags in [ - (post_install_1, '-at_install,/base,/crm,/documents,/hw_drivers,/l10n_be,/l10n_in'), # we need the blacklisted modules here - (post_install_2, '-at_install,/mail,/project,/test_l10n,/test_lint'), + (post_install_1, '-at_install,/account,/base,/crm,/documents,/hw_drivers,/l10n_be,/l10n_in'), # we need the blacklisted modules here + (post_install_2, '-at_install,/mail,/project,/test_l10n,/test_lint,/test_mail'), (post_install_3, '-at_install,/web'), (post_install_4, '-at_install,/web_enterprise'), ]: @@ -694,7 +756,7 @@ def test_dynamic_step_l10n_standalone(self): (post_install_1, '-external,-external_l10n,post_install_l10n/l10n_hr_payroll_be,post_install_l10n/l10n_hr_payroll_in'), # we need the blacklisted modules here (post_install_2, '-external,-external_l10n,post_install_l10n/l10n_edi_be,post_install_l10n/l10n_edi_in'), (post_install_3, '-external,-external_l10n,post_install_l10n/l10n_reports_be,post_install_l10n/l10n_reports_in'), - (post_install_4, Like('-external,-external_l10n,post_install_l10n/base,post_install_l10n/crm,...')), + (post_install_4, Like('-external,-external_l10n,post_install_l10n/account,post_install_l10n/base,post_install_l10n/crm,...')), ]: with self.subTest(post_install=expected_tags): # 4.1 post install restore @@ -738,6 +800,7 @@ def test_foreach_module(self): self.config.step_ids[0]._run_dynamic(self.build) self.assertEqual(self.build.children_ids.mapped('description'), [ + 'Post install tests for **account**', 'Post install tests for **base**', 'Post install tests for **crm**', 'Post install tests for **documents**', @@ -767,31 +830,154 @@ def test_foreach_modified_module(self): }] }''' - self.patch(type(self.build), '_modified_modules', lambda cl: {'crm'}) + self.patch(type(self.build), '_modified_modules', lambda cl, defaults=None: {'crm'}) self.config.default_dynamic_config = dynamic_config self.config.step_ids[0]._run_dynamic(self.build) self.assertEqual(self.build.children_ids.mapped('description'), - [ - 'Post install tests for **crm**', + [ + 'Post install tests for **crm**', ]) - def test_parse_dynamic_entry(self): - Step = self.env['runbot.build.config.step'] + def test_modified_existing_module(self): + dynamic_config = '''{ + "vars": { + "modified_modules": "{{*|filter_all_modules|modified_modules}}", + "test_modules": "{{modified_modules|prepend('test_')|select_existing_modules}}", + "modules_to_test": "{{modified_modules|union(test_modules)}}" + }, + "name": "Foreach module testing", + "steps": [{ + "name": "Create module builds", + "job_type": "create_build", + "children": [{ + "name": "Test single module", + "description": "Post install tests for **{{modules_to_test}}**", + "steps": [{ + "name": "Start single module test", + "job_type": "odoo", + "install_modules": "{{modules_to_test}}", + "test_tags": "{{modules_to_test|make_module_test_tags}}" + }] + }] + }] + }''' - def check_parse(entry, expected): - res = Step._parse_dynamic_entry(entry, self.build, {'key': 'value', 'test_method': '.test_method'}) - self.assertEqual(res, expected) - check_parse('{{-test_*|filter_all_modules}}', 'base,crm,documents,hw_drivers,l10n_be,l10n_in,mail,project,web,web_enterprise') - check_parse('{{-*,web*|filter_all_modules}}', 'web,web_enterprise') - check_parse('{{-*,web*|filter_all_modules|make_module_test_tags}}', '/web,/web_enterprise') - check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|prepend("some_tag")}}', 'some_tag/web,some_tag/web_enterprise') - check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|prepend(key)}}', 'value/web,value/web_enterprise') - check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|append(".test_method")}}', '/web.test_method,/web_enterprise.test_method') - check_parse('{{-*,web*|filter_all_modules|make_module_test_tags|append(test_method)}}', '/web.test_method,/web_enterprise.test_method') + self.patch(type(self.build), '_modified_modules', lambda cl, defaults=None: {'crm', 'mail'}) + self.config.default_dynamic_config = dynamic_config + self.config.step_ids[0]._run_dynamic(self.build) + self.assertEqual(self.build.children_ids.mapped('description'), + [ + 'Post install tests for **crm,mail,test_mail**', + ]) + child_dynamic_vars = self.build.children_ids.params_id.config_data['dynamic_vars'] + self.assertEqual(child_dynamic_vars, { + 'modified_modules': 'crm,mail', + 'test_modules': 'test_mail', + 'modules_to_test': 'crm,mail,test_mail', + }) - self.patch(type(self.build), '_modified_modules', lambda cl: {'crm'}) + def test_modified_existing_module_parallel(self): + dynamic_config = '''{ + "vars": { + "modified_modules": "{{*|filter_all_modules|modified_modules}}", + "modules_to_test": "{{modified_modules|prepend('test_')|select_existing_modules|union(modified_modules)}}" + }, + "name": "Parallel split modified", + "steps": [{ + "name": "Create module builds", + "job_type": "create_build", + "for_each_vars": [{ + "test_module_filter": "{{modules_to_test}},->!mail" + }, + { + "test_module_filter": "{{modules_to_test}},mail->!website" + }, + { + "test_module_filter": "{{modules_to_test}},website->" + } + ], + "if": "{{child_modules_to_test}}", + "children": [{ + "vars": { + "child_modules_to_test": "{{test_module_filter|select_existing_modules}}" + }, + "name": "Test single module", + "description": "Post install tests for **{{child_modules_to_test}}**", + "steps": [{ + "name": "Start single module test", + "job_type": "odoo", + "install_modules": "{{child_modules_to_test}}", + "test_tags": "{{child_modules_to_test|make_module_test_tags}}" + }] + }] + }] + }''' - check_parse('{{*|filter_all_modules|modified_modules}}', 'crm') + self.patch(type(self.build), '_modified_modules', lambda cl, defaults=None: {'crm', 'mail'}) + self.config.default_dynamic_config = dynamic_config + self.config.step_ids[0]._run_dynamic(self.build) + self.assertEqual(self.build.children_ids.mapped('description'), + [ + 'Post install tests for **crm**', + 'Post install tests for **mail,test_mail**', + ]) + + self.assertEqual(self.build.children_ids[0].params_id.config_data['dynamic_vars']['child_modules_to_test'], 'crm') + self.assertEqual(self.build.children_ids[1].params_id.config_data['dynamic_vars']['child_modules_to_test'], 'mail,test_mail') + + def test_modified_existing_module_parallel_relations(self): + dynamic_config = '''{ + "vars": [ + {"module_filter": "*,-hw_*,-*l10n_*,-theme_*,-account_bacs,-account_reports_cash_basis,-auth_ldap,-base_gengo,-document_ftp,-iot_drivers,-note_pad,-odoo_referral,-odoo_referral_portal,-pad,-pad_project,-pos_blackbox_be,-pos_cache,-pos_six,-social_demo,-website_gengo,-website_instantclick,test_l10n_be_hr_payroll_account,test_l10n_us_hr_payroll_account"}, + {"_modified_modules": "{{module_filter|filter_all_modules|modified_modules}}"}, + {"_modules_dependencies": "{{_modified_modules|get_dependencies(1)}}"}, + {"_dependant_modules": "{{_modified_modules|get_dependant(1)}}"}, + {"_test_modules": "{{_modified_modules|prepend('test_')|select_existing_modules}}"}, + {"_modules_to_test": "{{_modified_modules|union(_test_modules)|union(_dependant_modules)|union(_modules_dependencies)}}"} + ], + "name": "Parallel split modified", + "steps": [{ + "name": "Create module builds", + "job_type": "create_build", + "for_each_vars": [{ + "_test_module_filter": "{{_modules_to_test}},->!mail" + }, + { + "_test_module_filter": "{{_modules_to_test}},mail->!website" + }, + { + "_test_module_filter": "{{_modules_to_test}},website->" + } + ], + "if": "{{child_modules_to_test}}", + "log": "Modified modules: {{_modified_modules}}\\nDepenencies: {{_modules_dependencies}}\\nDependant: {{_dependant_modules}}\\nTest modules: {{_test_modules}}", + "children": [{ + "vars": { + "child_modules_to_test": "{{_test_module_filter|select_existing_modules}}" + }, + "name": "Test single module", + "description": "Post install tests for **{{child_modules_to_test}}**", + "steps": [{ + "name": "Start single module test", + "job_type": "odoo", + "install_modules": "{{child_modules_to_test}}", + "test_tags": "{{child_modules_to_test|make_module_test_tags}}" + }] + }] + }] + }''' + + self.patch(type(self.build), '_modified_modules', lambda cl, defaults=None: {'crm', 'mail'}) + self.config.default_dynamic_config = dynamic_config + self.config.step_ids[0]._run_dynamic(self.build) + self.assertEqual(self.build.children_ids.mapped('description'), + [ + 'Post install tests for **crm**', + 'Post install tests for **mail,test_mail,web**', + ]) + self.assertEqual(self.build.children_ids[0].params_id.config_data['dynamic_vars']['child_modules_to_test'], 'crm') + self.assertEqual(self.build.children_ids[1].params_id.config_data['dynamic_vars']['child_modules_to_test'], 'mail,test_mail,web') + self.assertEqual(list(self.build.children_ids[0].params_id.config_data['dynamic_vars'].keys()), ['module_filter', 'child_modules_to_test']) class TestBuildConfigStep(TestBuildConfigStepCommon): diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index 81c940ab0..cb8195a05 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -372,7 +372,7 @@ def setUp(self): self.fetch_count = 0 self.force_failure = False - def mock_git_helper(self, repo, cmd): + def mock_git_helper(self, repo, cmd, input_data=None, raw=False): self.assertIn('fetch', cmd) self.fetch_count += 1 if self.fetch_count < 3 or self.force_failure: @@ -457,7 +457,7 @@ def setUp(self): super().setUp() self.test_refs = [] - def mock_git_helper(self, repo, cmd): + def mock_git_helper(self, repo, cmd, input_data=None, raw=False): self.assertIn('for-each-ref', cmd) self.assertIn('refs/*/pull/*', cmd) return '\n'.join(['\x00'.join(ref_data) for ref_data in self.test_refs]) diff --git a/runbot/views/build_views.xml b/runbot/views/build_views.xml index 3a6064cfa..25532541f 100644 --- a/runbot/views/build_views.xml +++ b/runbot/views/build_views.xml @@ -12,6 +12,8 @@ + + From 01a1fc7f54e61df407ec0408a31db054d78cea86 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 16 Apr 2026 09:39:30 +0200 Subject: [PATCH 02/38] [FIX] runbot: fix fetch treehash --- runbot/models/commit.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/runbot/models/commit.py b/runbot/models/commit.py index 18d6922d8..13fa32bf4 100644 --- a/runbot/models/commit.py +++ b/runbot/models/commit.py @@ -70,7 +70,7 @@ def _list_files(self, patterns): # note that glob is needed to avoid the star matching ** self.ensure_one() self._fetch() - return self.repo_id._git(['ls-files', '--with-tree', self.name, *patterns]).split('\n') + return self.repo_id._git(['ls-files', '--with-tree', self.tree_hash, *patterns]).split('\n') def _list_available_modules(self): addons_paths = (self.repo_id.addons_paths or '').split(',') @@ -91,8 +91,9 @@ def _list_available_modules(self): @transactioncache # hack to avoid to fetch two time the same commit inside the same transaction def _fetch(self): - self.repo_id._fetch(self.name) - if not self.repo_id._hash_exists(self.name): + try: + self.repo_id._fetch(self.name) + except RunbotException: self.repo_id._fetch(self.tree_hash) def _export(self, build): From ddea235bc77156a449b46bda2bd5a2783a203503 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 16 Apr 2026 12:27:51 +0200 Subject: [PATCH 03/38] [REF] runbot: use message queue for kill requests This will be a first step to test the message queue before switching state recomputation inside it. --- runbot/__manifest__.py | 2 +- runbot/controllers/badge.py | 8 +-- runbot/controllers/frontend.py | 1 + runbot/migrations/19.0.5.17/pre-migration.py | 3 + runbot/models/batch.py | 2 +- runbot/models/build.py | 68 +++++++++++--------- runbot/models/build_config.py | 2 +- runbot/models/host.py | 12 +++- runbot/models/runbot.py | 8 +-- runbot/security/ir.model.access.csv | 4 +- runbot/templates/utils.xml | 10 +-- runbot/tests/test_build.py | 21 +++--- runbot/views/build_views.xml | 2 +- 13 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 runbot/migrations/19.0.5.17/pre-migration.py diff --git a/runbot/__manifest__.py b/runbot/__manifest__.py index 38db70949..f166684a5 100644 --- a/runbot/__manifest__.py +++ b/runbot/__manifest__.py @@ -6,7 +6,7 @@ 'author': "Odoo SA", 'website': "http://runbot.odoo.com", 'category': 'Website', - 'version': '5.16', + 'version': '5.17', 'application': True, 'depends': ['base', 'base_automation', 'website', 'auth_oauth'], 'data': [ diff --git a/runbot/controllers/badge.py b/runbot/controllers/badge.py index fa6e031ee..d0246cb21 100644 --- a/runbot/controllers/badge.py +++ b/runbot/controllers/badge.py @@ -44,13 +44,9 @@ def badge(self, name, repo_id=False, trigger_id=False, theme='default'): if not builds: state = 'testing' else: - result = builds._result_multi() - if result == 'ok': + state = 'failed' + if all(build.global_result == 'ok' for build in self): state = 'success' - elif result == 'warn': - state = 'warning' - else: - state = 'failed' etag = request.httprequest.headers.get('If-None-Match') retag = hashlib.md5(state.encode()).hexdigest() diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index a8f15e44a..fb55664ed 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -276,6 +276,7 @@ def resend_status(self, status_id=None, **kwargs): ], type='http', auth="user", methods=['POST'], csrf=False) def build_operations(self, build_id, operation, **post): build = request.env['runbot.build'].sudo().browse(build_id) + build.check_access('read') if operation == 'rebuild': build = build._rebuild() elif operation == 'kill': diff --git a/runbot/migrations/19.0.5.17/pre-migration.py b/runbot/migrations/19.0.5.17/pre-migration.py new file mode 100644 index 000000000..c21faba28 --- /dev/null +++ b/runbot/migrations/19.0.5.17/pre-migration.py @@ -0,0 +1,3 @@ +def migrate(cr, version): + cr.execute("""UPDATE runbot_build set local_result = 'killed' where local_result = 'manually_killed'""") + cr.execute("""UPDATE runbot_build set global_result = 'killed' where global_result = 'manually_killed'""") diff --git a/runbot/models/batch.py b/runbot/models/batch.py index 6cd395827..cad114cb7 100644 --- a/runbot/models/batch.py +++ b/runbot/models/batch.py @@ -154,7 +154,7 @@ def _create_build(self, params, slot): build = self.env['runbot.build'].search(domain, limit=1, order='id desc') link_type = 'matched' - killed_states = ('skipped', 'killed', 'manually_killed') + killed_states = ('skipped', 'killed') if build and build.local_result not in killed_states and build.global_result not in killed_states: if build.killable: build.killable = False diff --git a/runbot/models/build.py b/runbot/models/build.py index 340aec464..690533389 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -43,7 +43,7 @@ _logger = logging.getLogger(__name__) -result_order = ['ok', 'warn', 'ko', 'skipped', 'killed', 'manually_killed'] +result_order = ['ok', 'warn', 'ko', 'skipped', 'killed', 'manually_killed'] # TODO remove manually_killed state_order = ['pending', 'testing', 'waiting', 'running', 'done'] COPY_WHITELIST = [ @@ -297,6 +297,8 @@ class BuildResult(models.Model): local_result = fields.Selection(make_selection(result_order), string='Build Result', default='ok') requested_action = fields.Selection([('wake_up', 'To wake up'), ('deathrow', 'To kill')], string='Action requested', index=True) + to_kill = fields.Boolean('To kill', compute='_compute_to_kill') + message_ids = fields.One2many('runbot.host.message', 'build_id', string='Messages') # web infos host = fields.Char('Host name') host_id = fields.Many2one('runbot.host', string="Host", compute='_compute_host_id') @@ -394,6 +396,11 @@ def _compute_global_state(self): else: record.global_state = record.local_state + @api.depends('message_ids') + def _compute_to_kill(self): + for record in self: + record.to_kill = any(message.message == 'kill' for message in record.message_ids) + @api.depends('gc_delay', 'job_end') def _compute_gc_date(self): icp = self.env['ir.config_parameter'].sudo() @@ -543,17 +550,6 @@ def _add_child(self, param_values, orphan=False, description=False, additionnal_ **build_values, }) - def _result_multi(self): - if all(build.global_result == 'ok' or not build.global_result for build in self): - return 'ok' - if any(build.global_result in ('skipped', 'killed', 'manually_killed') for build in self): - return 'killed' - if any(build.global_result == 'ko' for build in self): - return 'ko' - if any(build.global_result == 'warning' for build in self): - return 'warning' - return 'ko' # ? - @api.depends('params_id.version_id.name') def _compute_dest(self): for build in self: @@ -826,11 +822,13 @@ def _init_pendings(self): def _process_requested_actions(self): self.ensure_one() build = self + # TODO remove, replaced by queue if build.requested_action == 'deathrow': result = None if build.local_state != 'running' and build.global_result not in ('warn', 'ko'): - result = 'manually_killed' + result = 'killed' build._kill(result=result) + build.requested_action = False return if build.requested_action == 'wake_up': @@ -1232,7 +1230,7 @@ def truncate(message, maxlenght=300000): 'line': '0', }) - def _kill(self, result=None): + def _kill(self, result='killed'): host_name = self.env['runbot.host']._get_current_name() self.ensure_one() build = self @@ -1240,30 +1238,38 @@ def _kill(self, result=None): return build._log('kill', 'Kill build %s' % build.dest) docker_stop(build._get_docker_name(), build._path()) - v = {'local_state': 'done', 'requested_action': False, 'active_step': False, 'job_end': now()} + build.local_state = 'done' + build.active_step = False + build.job_end = now() if not build.build_end: - v['build_end'] = now() + build.build_end = now() if result: - v['local_result'] = result - build.write(v) - - def _ask_kill(self, lock=True, message=None): - # if build remains in same bundle, it's ok like that - # if build can be cross bundle, need to check number of ref to build - if lock: - self.env.cr.execute("""SELECT id FROM runbot_build WHERE parent_path like %s FOR UPDATE""", ['%s%%' % self.parent_path]) + build.local_result = result + + def _ask_kill(self, message=None): self.ensure_one() user = self.env.user uid = user.id build = self message = message or 'Killing build %s, requested by %s (user #%s)' % (build.dest, user.name, uid) build._log('_ask_kill', message) - if build.local_state == 'pending': - build._skip() - elif build.local_state in ['testing', 'running']: - build.requested_action = 'deathrow' - for child in build.children_ids: - child._ask_kill(lock=False) + + self.env.cr.execute("""SELECT id, local_state FROM runbot_build WHERE parent_path like %s""", ['%s%%' % self.parent_path]) + builds = self.browse([b[0] for b in self.env.cr.fetchall()]) + pending = builds.filtered(lambda b: b.local_state == 'pending') + killable = builds.filtered(lambda b: b.local_state in ('running', 'testing')) + if pending: + pending.local_state = 'done' + pending.local_result = 'killed' + pending.flush_recordset() # faster concurrent error or lock row + + values = [{ + 'host_id': build.host_id.id, + 'build_id': build_id, + 'message': 'kill', + } for build_id in killable.ids] + + self.env['runbot.host.message'].sudo().create(values) def _wake_up(self): user = self.env.user @@ -1518,7 +1524,7 @@ def _get_color_class(self): if self.global_result == 'ok': return 'success' - if self.global_result in ('skipped', 'killed', 'manually_killed'): + if self.global_result in ('skipped', 'killed'): return 'secondary' return 'default' diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 0c86e5eb8..9c3a5ccee 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -1027,7 +1027,7 @@ def get_reference_builds_for_versions(versions): ) if self.allow_similar_build_quick_result: - existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed', 'manually_killed')), None) + existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed')), None) if existing_done_build: child._log('', 'A similar [build](%s) has been found, marking as done directly', existing_done_build.build_url, log_type='markdown') child.local_state = 'done' diff --git a/runbot/models/host.py b/runbot/models/host.py index 7dea8fba1..72d59c555 100644 --- a/runbot/models/host.py +++ b/runbot/models/host.py @@ -341,7 +341,7 @@ def _get_builds(self, domain, order=None): return self.env['runbot.build'].search(self._get_build_domain(domain), order=order) def _process_messages(self): - self.host_message_ids._process() + return self.host_message_ids._process() class MessageQueue(models.Model): @@ -351,14 +351,20 @@ class MessageQueue(models.Model): _log_access = False create_date = fields.Datetime('Create date', default=fields.Datetime.now) - host_id = fields.Many2one('runbot.host', required=True, ondelete='cascade') - build_id = fields.Many2one('runbot.build') + host_id = fields.Many2one('runbot.host', required=True, ondelete='cascade', index=True) + build_id = fields.Many2one('runbot.build', index=True) message = fields.Char('Message') def _process(self): records = self + processed = False # todo consume messages here if records: + processed = True for record in records: + if record.message == 'kill': + if record.build_id: + record.build_id._kill('killed') self.env['runbot.runbot']._warning(f'Host {record.host_id.name} got an unexpected message {record.message}') self.unlink() + return processed diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index 047e3090b..b470e1df9 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -53,10 +53,10 @@ def _scheduler(self, host): processed += 1 build._process_requested_actions() self._commit() + if host._process_messages(): + self._commit() host._process_logs() self._commit() - host._process_messages() - self._commit() for build in host._get_builds([('local_state', 'in', ['testing', 'running'])]) | self._get_builds_to_init(host): build = build.browse(build.id) # remove preftech ids, manage build one by one result = build._schedule() @@ -113,14 +113,14 @@ def _gc_running(self, host): ][:running_max] build_ids = host._get_builds([('local_state', '=', 'running'), ('id', 'not in', cannot_be_killed_ids)], order='job_start desc').ids for build in Build.browse(build_ids)[running_max:]: - build._kill() + build._kill(None) def _gc_testing(self, host): """garbage collect builds that could be killed""" # decide if we need room Build = self.env['runbot.build'] domain_host = host._get_build_domain() - testing_builds = Build.search(domain_host + [('local_state', 'in', ['testing', 'pending']), ('requested_action', '!=', 'deathrow')]) + testing_builds = Build.search(domain_host + [('local_state', 'in', ['testing', 'pending']), ('message_ids', '=', False)]) used_slots = len(testing_builds) available_slots = host.nb_worker - used_slots nb_pending = Build.search_count([('local_state', '=', 'pending'), ('host', '=', False)]) diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index 6c233c654..f0ca84182 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -68,6 +68,9 @@ access_runbot_error_regex_manager,runbot_error_regex_manager,runbot.model_runbot access_runbot_host_public,runbot_host_public,runbot.model_runbot_host,runbot.base_runbot_model_access,1,0,0,0 access_runbot_host_manager,runbot_host_manager,runbot.model_runbot_host,runbot.group_runbot_admin,1,1,1,1 +access_runbot_host_message_public,runbot_host_message_public,runbot.model_runbot_host_message,runbot.base_runbot_model_access,1,0,0,0 +access_runbot_host_message_admin,runbot_host_message_admin,runbot.model_runbot_host_message,runbot.group_runbot_admin,1,1,1,1 + access_runbot_repo_hooktime,runbot_repo_hooktime,runbot.model_runbot_repo_hooktime,group_user,1,0,0,0 access_runbot_repo_referencetime,runbot_repo_referencetime,runbot.model_runbot_repo_reftime,group_user,1,0,0,0 access_runbot_build_stat_admin,runbot_build_stat_admin,runbot.model_runbot_build_stat,runbot.group_runbot_admin,1,1,1,1 @@ -106,7 +109,6 @@ access_runbot_bundle_public,access_runbot_bundle_public,runbot.model_runbot_bund access_runbot_bundle_runbot_bundle_manager,access_runbot_bundle_runbot_manager,runbot.model_runbot_bundle,runbot.group_runbot_bundle_manager,1,1,0,0 access_runbot_bundle_runbot_admin,access_runbot_bundle_runbot_admin,runbot.model_runbot_bundle,runbot.group_runbot_admin,1,1,1,1 - access_runbot_batch_public,access_runbot_batch_public,runbot.model_runbot_batch,runbot.base_runbot_model_access,1,0,0,0 access_runbot_batch_runbot_admin,access_runbot_batch_runbot_admin,runbot.model_runbot_batch,runbot.group_runbot_admin,1,1,1,1 diff --git a/runbot/templates/utils.xml b/runbot/templates/utils.xml index bf89535f2..ccc2eaa01 100644 --- a/runbot/templates/utils.xml +++ b/runbot/templates/utils.xml @@ -15,7 +15,7 @@ - + @@ -31,7 +31,7 @@ - + @@ -330,7 +330,7 @@ default - + killed @@ -365,12 +365,12 @@ Database selector - + Rebuild - + Kill diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index e55aab30a..3fc8cef24 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -744,7 +744,6 @@ def test_repo_gc_testing(self): bundle_b = self.env['runbot.bundle'].search([('name', '=', branch_b_name)]) bundle_b.last_batch._process() - build_b = bundle_b.last_batch.slot_ids[0].build_id # the two builds are starting tests on two different hosts @@ -754,8 +753,8 @@ def test_repo_gc_testing(self): # no room needed, verify that nobody got killed self.Runbot._gc_testing(host) - self.assertFalse(build_a.requested_action) - self.assertFalse(build_b.requested_action) + self.assertFalse(build_a.to_kill) + self.assertFalse(build_b.to_kill) # a new commit is pushed on branch_a self.push_commit(self.remote_odoo_dev, branch_a_name, 'new subject', sha='d0cad0ca') @@ -775,10 +774,10 @@ def test_repo_gc_testing(self): # no room needed, verify that nobody got killed self.Runbot._gc_testing(host) - self.assertFalse(build_a.requested_action) - self.assertFalse(build_b.requested_action) - self.assertFalse(build_a_last.requested_action) - self.assertFalse(children_b.requested_action) + self.assertFalse(build_a.to_kill) + self.assertFalse(build_b.to_kill) + self.assertFalse(build_a_last.to_kill) + self.assertFalse(children_b.to_kill) # now children_b starts on runbot_xxx children_b.write({'local_state': 'testing', 'host': host.name}) @@ -789,10 +788,10 @@ def test_repo_gc_testing(self): self.Runbot._gc_testing(host) # the killable build should have been marked to be killed - self.assertEqual(build_a.requested_action, 'deathrow') - self.assertFalse(build_b.requested_action) - self.assertFalse(build_a_last.requested_action) - self.assertFalse(children_b.requested_action) + self.assertTrue(build_a.to_kill) + self.assertFalse(build_b.to_kill) + self.assertFalse(build_a_last.to_kill) + self.assertFalse(children_b.to_kill) class TestGithubStatus(RunbotCase): diff --git a/runbot/views/build_views.xml b/runbot/views/build_views.xml index 25532541f..ce996e5b4 100644 --- a/runbot/views/build_views.xml +++ b/runbot/views/build_views.xml @@ -65,7 +65,7 @@ - + From fd3a243049c5f1f189e3e5b6e98492e4678e409a Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 17 Apr 2026 10:06:33 +0200 Subject: [PATCH 04/38] [FIX] fix badge page --- runbot/controllers/badge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runbot/controllers/badge.py b/runbot/controllers/badge.py index d0246cb21..662163682 100644 --- a/runbot/controllers/badge.py +++ b/runbot/controllers/badge.py @@ -45,7 +45,7 @@ def badge(self, name, repo_id=False, trigger_id=False, theme='default'): state = 'testing' else: state = 'failed' - if all(build.global_result == 'ok' for build in self): + if all(build.global_result == 'ok' for build in builds): state = 'success' etag = request.httprequest.headers.get('If-None-Match') From df908b1a945a9d9c6d6623db89f729bd871bde36 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 17 Apr 2026 10:13:41 +0200 Subject: [PATCH 05/38] [FIX] runbot: send message to correct host --- runbot/models/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index 690533389..12ecc007d 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1264,10 +1264,10 @@ def _ask_kill(self, message=None): pending.flush_recordset() # faster concurrent error or lock row values = [{ - 'host_id': build.host_id.id, - 'build_id': build_id, + 'host_id': b.host_id.id, + 'build_id': b.id, 'message': 'kill', - } for build_id in killable.ids] + } for b in killable] self.env['runbot.host.message'].sudo().create(values) From bdd8052925e59aa1839042fa49492e24b37d57ed Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 17 Apr 2026 15:45:22 +0200 Subject: [PATCH 06/38] [FIX] runbot: fix ko killed build status When a build is already ko, it should not be switched to killed This fix replicates the logic of the requested action processing. --- runbot/models/host.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/runbot/models/host.py b/runbot/models/host.py index 72d59c555..6d657d90f 100644 --- a/runbot/models/host.py +++ b/runbot/models/host.py @@ -364,7 +364,11 @@ def _process(self): for record in records: if record.message == 'kill': if record.build_id: - record.build_id._kill('killed') + build = record.build_id + result = None + if build.local_state != 'running' and build.global_result not in ('warn', 'ko'): + result = 'killed' + build._kill(result=result) self.env['runbot.runbot']._warning(f'Host {record.host_id.name} got an unexpected message {record.message}') self.unlink() return processed From 693aad85fb2f6d2df879224480a939009ab3345b Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 22 Apr 2026 12:22:16 +0200 Subject: [PATCH 07/38] [IMP] runbot: improve runbot build page Remove some not so needed elements, add links on all file looking paths --- runbot/models/build.py | 55 +++++++++++++++++- runbot/models/build_error.py | 1 - runbot/static/src/css/runbot.css | 30 +++++++++- runbot/templates/build.xml | 95 +++++++++++--------------------- runbot/tests/test_build.py | 26 +++++++++ 5 files changed, 141 insertions(+), 66 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index 12ecc007d..ef77696e8 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -14,12 +14,13 @@ from pathlib import Path from dateutil import parser +from markupsafe import Markup from psycopg2 import sql from psycopg2.extensions import TransactionRollbackError from odoo import api, fields, models from odoo.exceptions import ValidationError -from odoo.tools import file_open, file_path +from odoo.tools import file_open, file_path, html_escape from odoo.tools.safe_eval import safe_eval from ..common import ( @@ -1528,6 +1529,58 @@ def _get_color_class(self): return 'secondary' return 'default' + def _get_file_url(self, path, line=None): + repo_name = path.replace('/data/build/', '').split('/')[0] + for commit_link in self.params_id.commit_link_ids: + if commit_link.commit_id.repo_id.name == repo_name: + repo_base_url = commit_link.branch_id.remote_id.base_url + commit_hash = commit_link.commit_id.name + path = path.replace('/data/build/%s/' % repo_name, '') + url = f'https://{repo_base_url}/blob/{commit_hash}/{path}' + if line: + url = f'{url}#L{line}' + return url + return '' + + def _format_message(self, log): + text = log.message + if not "\n" in text and 'in: /data/build/' in text: + parts = text.split('in: /data/build/') + text = parts[0] + url = f'http://{self.host}/runbot/static/build/{self.dest}/{parts[-1]}' + template = Markup('%s') + return template % (url, text) + text = text.strip('\n') + text = html_escape(text) + + def get_link(match): + path = match.group(1) + line = match.group(2) + url = self._get_file_url(path, line) + if url: + if line: + return Markup('%s", line %s') % (url, path, line) + return Markup('%s') % (url, path) + return match.group(0) + regex = r''' + (/data/build/[\w\-\./]+\.(?:py|xml|js|css)) # Path in /data/build ending with a common extension + (?: + \&\#34;,\sline\s(\d+) # Optional line number (escaped quote) + )? + ''' + text = Markup(re.sub(regex, get_link, text, flags=re.VERBOSE)) + + return text + + def _log_details(self, log): + title = f"Logger: {log.name}\nFunc: {log.func}" + test_data = log.metadata.dict.get('test') + if test_data: + title += '\n' + for test_line in test_data: + title += f'\n{test_line}: {test_data[test_line]}' + return title + def _github_status(self): """Notify github of failed/successful builds""" for build in self: diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index b165914d6..87197ccfc 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -215,7 +215,6 @@ class BuildError(models.Model): _inherit = ('mail.thread', 'mail.activity.mixin', 'runbot.build.error.seen.mixin') _mail_post_access = 'read' - name = fields.Char("Name") active = fields.Boolean('Open (not fixed)', default=True, tracking=True) description = fields.Text("Description", store=True, compute='_compute_description') diff --git a/runbot/static/src/css/runbot.css b/runbot/static/src/css/runbot.css index c275824a5..c49d822fa 100644 --- a/runbot/static/src/css/runbot.css +++ b/runbot/static/src/css/runbot.css @@ -185,7 +185,7 @@ a.slots_infos:hover { } .separator { - border-top: 2px solid #666; + border-top: 0.2em solid #666; } body, .table { @@ -427,3 +427,31 @@ code { .hide-success tr.bg-success-subtle { display: none; } + +.pre { + display: block; + font-family: SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; + white-space: pre; + overflow: auto; + font-size: 0.875em; + margin:0; + padding:0; + margin-top: 0.2em; + border: none; + +} + +.subtle_link { + color: var(--bs-body-color); + text-decoration: underline; +} + +.table-condensed .log-server td, .table-condensed .log-details td { + padding-top: 0; + padding-bottom: 0; + border: none; +} + +.log-details td { + padding-left: 20px; +} diff --git a/runbot/templates/build.xml b/runbot/templates/build.xml index 3c701df09..9f57f309c 100644 --- a/runbot/templates/build.xml +++ b/runbot/templates/build.xml @@ -268,25 +268,27 @@
    - + - - - + - + - - + @@ -295,53 +297,21 @@ - + diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 3fc8cef24..2cf84f65e 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -705,6 +705,32 @@ def test_build_cmd_faketime(self): second_child_cmd = second_child._cmd(py_version=3) self.assertIn('faketime "2024-02-04 04:42 UTC" python3 odoo/server.py', str(second_child_cmd)) + def test_format_message(self): + def get_log(message): + return self.env['ir.logging'].create({ + 'build_id': build.id, + 'type': 'server', + 'message': message, + 'level': 'INFO', + 'name': 'odoo.addons.web.tests.test_web', + 'path': '', + 'func': '', + 'line': '', + }) + build = self.Build.create({ + 'params_id': self.server_params.id, + 'description': 'A nice **description** with a link to odoo.com', + }) + self.assertEqual( + build._format_message(get_log('File "/data/build/odoo/addons/web/tests/test_web.py", line 42, in test_web')), + 'File "/data/build/odoo/addons/web/tests/test_web.py", line 42, in test_web' + ) + + self.assertEqual( + build._format_message(get_log('File "/data/build/odoo/addons/web/tests/test_web.py", in test_web')), + 'File "/data/build/odoo/addons/web/tests/test_web.py", in test_web' + ) + class TestGc(RunbotCaseMinimalSetup): def test_repo_gc_testing(self): From 02f9f1094f05060e5139688b9815eb3bdc550213 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 23 Apr 2026 09:16:03 +0200 Subject: [PATCH 08/38] [FIX] runbot: replace broken error module order --- runbot/controllers/frontend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index fb55664ed..2a3ef69e5 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -455,8 +455,8 @@ def build_errors(self, sort=None, page=1, limit=20, **kwargs): 'build_count asc': 'Number seen: Low to High', 'responsible asc': 'Assignee: A - Z', 'responsible desc': 'Assignee: Z - A', - 'module_name asc': 'Module name: A - Z', - 'module_name desc': 'Module name: Z -A', + 'team_id asc': 'Team', + 'name asc': 'Name', } sort_order = sort if sort in sort_order_choices else 'last_seen_date desc' From a64f1580976c95c970acc6a9bf3ca45de35786f5 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 23 Apr 2026 13:17:34 +0200 Subject: [PATCH 09/38] [FIX] runbot: fix batch redirection for children --- runbot/controllers/frontend.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 2a3ef69e5..6735fe586 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -291,18 +291,17 @@ def build_operations(self, build_id, operation, **post): '/runbot/batch//build/', ], type='http', auth="public", website=True, sitemap=False) def build(self, build_id, search=None, from_batch=None, **post): - """Events/Logs""" - + build = request.env['runbot.build'].browse(build_id) if from_batch: from_batch = request.env['runbot.batch'].browse(int(from_batch)) - if build_id not in from_batch.with_context(active_test=False).slot_ids.build_id.ids: + if build.top_parent not in from_batch.with_context(active_test=False).slot_ids.build_id and build.create_batch_id != from_batch: # the url may have been forged replacing the build id, redirect to hide the batch return werkzeug.utils.redirect('/runbot/build/%s' % build_id) from_batch = from_batch.with_context(batch=from_batch) Build = request.env['runbot.build'].with_context(batch=from_batch) - build = Build.browse([build_id])[0] + build = Build.browse(build_id) if not build.exists(): return request.not_found() siblings = (build.parent_id.children_ids if build.parent_id else from_batch.slot_ids.build_id if from_batch else build).sorted('id') From 6d46249dc7ce0223ad2ccf3b2ae565161cd1fe8a Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 29 Apr 2026 12:38:57 +0200 Subject: [PATCH 10/38] [IMP] runbot: improve test-tags display and edition On an error, tests tags are joined by a comma and displayed in a single line. This makes it hard to read and edit when there are many tags, especially if they have parameters. This commit changes the separator to a newline, making it easier to read and edit the tags. In the future, it may also make the parsing more robust as we can easily split on \n, but the validation is still a good idea. This commit also adds a check on the format of the test tags to prevent saving invalid tag and hopefully complelty preventing saving tags that would break all builds. While on it, fixing the impacted error domain for parametric tags since right now they would match all errors ignoring the parameters making this feature pretty much unusable. --- runbot/common.py | 38 +++++++++++++++++--- runbot/models/build_config.py | 42 ++++++++++++++-------- runbot/models/build_error.py | 57 ++++++++++++++++++++---------- runbot/tests/test_build_error.py | 41 ++++++++++++++++++--- runbot/views/build_error_views.xml | 2 +- 5 files changed, 136 insertions(+), 44 deletions(-) diff --git a/runbot/common.py b/runbot/common.py index 23938c84e..fe34c2d44 100644 --- a/runbot/common.py +++ b/runbot/common.py @@ -329,9 +329,35 @@ class TestTagsParser: (?:\[(.*)\])? # parameters $''', re.VERBOSE) # [-][tag][/module][:class][.method][[params]] - def __init__(self, test_tags): - parts = re.split(r',(?![^\[]*\])', test_tags) # split on all comma not inside [] (not followed by ]) + def __init__(self, test_tags, keep_escape=True): + parts = [''] + bracket_level = 0 + escape_next = False + for char in test_tags: + if char == ',' and bracket_level == 0: + parts.append('') + continue + + if char == '\\': + if not escape_next: + escape_next = True + if keep_escape: + parts[-1] += '\\' # not as the TagsSelector, we keep the escape character + continue + elif char == '[': + if not escape_next: + bracket_level += 1 + elif char == ']': + if not escape_next: + bracket_level -= 1 + elif not keep_escape and escape_next: # the previous \ was not escaping anything, put it back + parts[-1] += '\\' + + escape_next = False + parts[-1] += char + filter_specs = [t.strip() for t in parts if t.strip()] + self.filter_specs = filter_specs self.exclude = set() self.include = set() self.parameters = OrderedSet() @@ -339,8 +365,7 @@ def __init__(self, test_tags): for filter_spec in filter_specs: match = self.filter_spec_re.match(filter_spec) if not match: - _logger.error('Invalid tag %s', filter_spec) - continue + raise ValueError('Invalid tag %s' % filter_spec) sign, tag, file_path, module, klass, method, parameters = match.groups() is_include = sign != '-' @@ -369,6 +394,7 @@ def __init__(self, test_tags): def test_tags_to_search_domain(self, exclude_error_id=None): search_domains = [] + params_by_spec = dict(self.parameters) for include in self.include: _, test_module, test_class, test_method, file_path = include module_path = file_path or ((test_module or '') + '%') @@ -376,6 +402,10 @@ def test_tags_to_search_domain(self, exclude_error_id=None): test_method = test_method or '%' search_pattern = f'{module_path}:{test_class}.{test_method}' tag_domain = [('canonical_tags', 'like', f'{search_pattern}')] + params = params_by_spec.get(include) + if params: + _sign, parameters = params + tag_domain.append(('canonical_tags', 'like', f'%[{parameters}%]%')) if exclude_error_id: tag_domain.append(('id', '!=', exclude_error_id)) search_domains.append(tag_domain) diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 9c3a5ccee..ac34d1ae0 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -1,35 +1,46 @@ import base64 +import fnmatch import glob import json import logging -import fnmatch -import psutil import re import shlex import time -from unidiff import PatchSet -from ..common import now, grep, time2str, rfind, s2human, os, RunbotException, ReProxy, markdown_escape -from ..container import docker_get_gateway_ip, Command -from odoo import models, fields, api, tools -from odoo.exceptions import UserError, ValidationError -from odoo.tools.misc import file_open -from odoo.tools.safe_eval import safe_eval, test_python_expr, _SAFE_OPCODES, to_opcodes -# adding some additionnal optcode to safe_eval. This is not 100% needed and won't be done in standard but will help -# to simplify some python step by wraping the content in a function to allow return statement and get closer to other -# steps +import psutil +from unidiff import VERSION, PatchSet, patch +from odoo import api, fields, models, tools +from odoo.exceptions import UserError, ValidationError +from odoo.tools.misc import file_open +from odoo.tools.safe_eval import _SAFE_OPCODES, safe_eval, test_python_expr, to_opcodes + +from ..common import ( + ReProxy, + RunbotException, + TestTagsParser, + grep, + markdown_escape, + now, + os, + rfind, + s2human, + time2str, +) +from ..container import Command, docker_get_gateway_ip # There is an issue in unidiff 0.7.3 fixed in 0.7.4 # https://github.com/matiasb/python-unidiff/commit/a3faffc54e5aacaee3ded4565c534482d5cc3465 # Since the unidiff packaged version in noble is 0.7.3 # patching it looks like the easiest solution -from unidiff import patch, VERSION if VERSION == '0.7.3': patch.RE_DIFF_GIT_DELETED_FILE = re.compile(r'^deleted file mode \d+$') patch.RE_DIFF_GIT_NEW_FILE = re.compile(r'^new file mode \d+$') +# adding some additionnal optcode to safe_eval. This is not 100% needed and won't be done in standard but will help +# to simplify some python step by wraping the content in a function to allow return statement and get closer to other +# steps _SAFE_OPCODES |= set(to_opcodes(['LOAD_DEREF', 'STORE_DEREF', 'LOAD_CLOSURE', 'MAKE_CELL', 'COPY_FREE_VARS'])) @@ -426,7 +437,7 @@ class ConfigStep(models.Model): paths_to_omit = fields.Char('Paths to omit from coverage', tracking=True) flamegraph = fields.Boolean('Allow Flamegraph', default=False, tracking=True) test_enable = fields.Boolean('Test enable', default=True, tracking=True) - test_tags = fields.Char('Test tags', help="comma separated list of test tags", tracking=True) + test_tags = fields.Char('Test tags', help="new line (or comma) separated list of test tags", tracking=True) enable_auto_tags = fields.Boolean('Allow auto tag', default=True, tracking=True) sub_command = fields.Char('Subcommand', tracking=True) extra_params = fields.Char('Extra cmd args', tracking=True) @@ -637,6 +648,7 @@ def _make_python_ctx(self, build): 'json_loads': json.loads, 'PatchSet': PatchSet, 'markdown_escape': markdown_escape, + 'TestTagsParser': TestTagsParser, } def _run_python(self, build, force=False): @@ -774,7 +786,7 @@ def _run_install_odoo(self, build, config_data=None): test_tags_in_extra = '--test-tags' in extra_params if (test_enable or test_tags) and "--test-tags" in available_options and not test_tags_in_extra: - test_tags = [t.strip() for t in (test_tags or '').split(',')] + test_tags = [t.strip() for t in TestTagsParser(test_tags or '').filter_specs] if enable_auto_tags and not config_data.get('disable_auto_tags', False): if grep(config_path, "[/module][:class]"): auto_tags = self.env['runbot.build.error']._disabling_tags(build) diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index 87197ccfc..711a88667 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -9,16 +9,15 @@ from dateutil import rrule from dateutil.relativedelta import relativedelta from markupsafe import Markup - from werkzeug.urls import url_join from odoo import api, fields, models from odoo.exceptions import AccessError, UserError, ValidationError -from odoo.tools import SQL, lazy, ormcache from odoo.fields import Domain +from odoo.tools import SQL, lazy, ormcache +from ..common import TestTagsParser, transactioncache from ..fields import JsonDictField -from ..common import transactioncache, TestTagsParser _logger = logging.getLogger(__name__) @@ -242,8 +241,8 @@ class BuildError(models.Model): breaking_bundle_url = fields.Char('Breaking bundle url', related='breaking_bundle_id.frontend_url') breaking_pr_date = fields.Datetime('Breaking date', related="breaking_pr_id.close_date", help="Date of the merge of the first pr") - test_tags = fields.Char(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True) - canonical_tags = fields.Char('Canonical tag', compute='_compute_canonical_tags', store=True) + test_tags = fields.Text(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True) + canonical_tags = fields.Text('Canonical tag', compute='_compute_canonical_tags', store=True) tags_match_count = fields.Integer('Nb errors matching the test_tags', compute='_compute_tags_match_count') tags_min_version_excluded_id = fields.Many2one('runbot.version', 'Tag min version (excluded)') tags_min_version_id = fields.Many2one('runbot.version', 'Tags Min version', compute="_compute_tags_min_version_id", inverse="_inverse_tags_min_version_id", help="Minimal version where the test tags will be applied.", tracking=True) @@ -288,7 +287,7 @@ def _inverse_tags_min_version_id(self): def _compute_canonical_tags(self): for record in self: canonical_tags = sorted(set(record.error_content_ids.filtered('canonical_tag').mapped('canonical_tag'))) - record.canonical_tags = ','.join(canonical_tags) + record.canonical_tags = '\n'.join(canonical_tags) @api.depends('tags_min_version_id') def _compute_tags_min_version_id(self): @@ -488,16 +487,20 @@ def _compute_tags_match_count(self): for record in self: record.tags_match_count = 0 if record.test_tags: - tags_parser = TestTagsParser(record.test_tags) - search_domain = tags_parser.test_tags_to_search_domain(exclude_error_id=record.id) - if search_domain: - record.tags_match_count = self.env['runbot.build.error'].with_context(active_test=True).search_count(search_domain) + try: + tags_parser = TestTagsParser(record.test_tags.replace('\n', ',')) + search_domain = tags_parser.test_tags_to_search_domain(exclude_error_id=record.id or record.id.origin) + if search_domain: + record.tags_match_count = self.env['runbot.build.error'].with_context(active_test=True).search_count(search_domain) + except Exception as e: # noqa: BLE001 + record.tags_match_count = -1 + _logger.warning("Error while computing tags_match_count for error %s with test_tags %s: %s", record.id, record.test_tags, e) def action_view_impacted_by_tag(self): self.ensure_one() if not self.test_tags: return - tags_parser = TestTagsParser(self.test_tags) + tags_parser = TestTagsParser(self.test_tags.replace('\n', ',')) return { 'type': 'ir.actions.act_window', 'views': [(False, 'list'), (False, 'form')], @@ -510,8 +513,15 @@ def action_view_impacted_by_tag(self): @api.constrains('test_tags') def _check_test_tags(self): for build_error in self: - if build_error.test_tags and '-' in build_error.test_tags: - raise ValidationError('Build error test_tags should not be negated') + if build_error.test_tags: + try: + test_tags = build_error.test_tags.replace('\n', ',') + tags_parser = TestTagsParser(test_tags) + tags_parser = TestTagsParser(test_tags, keep_escape=False) + except Exception as e: # noqa: BLE001 + raise ValidationError(f'Invalid test_tags format: {e}') + if tags_parser.exclude or any(params[0] == '-' for p, params in tags_parser.parameters): + raise ValidationError('Build error test_tags should not be negated') @api.onchange('test_tags') def _onchange_test_tags(self): @@ -586,12 +596,12 @@ def _merge(self, others): error.sudo().test_tags = previous_error.test_tags previous_error.sudo().test_tags = False elif self.env.su: - test_tags = error.test_tags.split(',') - previous_error - for tag in previous_error.test_tags.split(','): + test_tags = TestTagsParser(error.test_tags.replace('\n', ',')).filter_specs + previous_error_tags = TestTagsParser(previous_error.test_tags.replace('\n', ',')).filter_specs + for tag in previous_error_tags: if tag not in test_tags: test_tags.append(tag) - error.test_tags = ','.join(test_tags) + error.test_tags = '\n'.join(test_tags) previous_error.test_tags = False for field in fields_to_merge + fields_to_copy: if previous_error[field]: @@ -622,7 +632,16 @@ def filter_tags(e): return True test_tag_list = self.search([('test_tags', '!=', False)]).filtered(filter_tags).mapped('test_tags') - return [test_tag for error_tags in test_tag_list for test_tag in (error_tags).split(',')] + parsed_test_tags = [] + for error_tags in test_tag_list: + try: + # we cannot rely only on '\n' since old test-tags or user defined ones could be comma separated + error_tags = error_tags.replace('\n', ',') + tags_parser = TestTagsParser(error_tags) + parsed_test_tags.extend(tags_parser.filter_specs) + except Exception as e: # noqa: BLE001 + _logger.warning('Error while parsing test_tags for error with id %s: %s', self.id, e) + return parsed_test_tags @api.model def _disabling_tags(self, build_id=False): @@ -853,7 +872,7 @@ class BuildErrorContent(models.Model): breaking_pr_id = fields.Many2one(related='error_id.breaking_pr_id') fixing_pr_alive = fields.Boolean(related='error_id.fixing_pr_alive') fixing_pr_url = fields.Char(related='error_id.fixing_pr_url') - test_tags = fields.Char(related='error_id.test_tags') + test_tags = fields.Text(related='error_id.test_tags') tags_min_version_id = fields.Many2one(related='error_id.tags_min_version_id') tags_max_version_id = fields.Many2one(related='error_id.tags_max_version_id') diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py index 24e9ab6a2..3525871cb 100644 --- a/runbot/tests/test_build_error.py +++ b/runbot/tests/test_build_error.py @@ -266,7 +266,7 @@ def test_relink_simple(self): build_b = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) error_content_b = self.BuildErrorContent.create({'content': 'foo bar'}) error_b = error_content_b.error_id - error_b.test_tags = 'footag' + error_b.test_tags = 'footag[@test, comma]\nfootag2[@test, comma],footag3[@test, comma]' self.BuildErrorLink.create({'build_id': build_b.id, 'error_content_id': error_content_b.id}) self.assertEqual(self.BuildErrorContent.search([('fingerprint', '=', error_content_a.fingerprint)]), error_content_a | error_content_b) @@ -275,7 +275,7 @@ def test_relink_simple(self): self.assertFalse(error_b.error_content_ids) self.assertTrue(error_a.active, 'The merged error without test tags should have been deactivated') - self.assertEqual(error_a.test_tags, 'footag', 'Tags should have been transfered from b to a') + self.assertEqual(error_a.test_tags, 'footag[@test, comma]\nfootag2[@test, comma],footag3[@test, comma]', 'Tags should have been transfered from b to a') self.assertFalse(error_b.active, 'The merged error with test tags should remain active') self.assertIn(build_a, error_content_a.build_ids) self.assertIn(build_b, error_content_a.build_ids) @@ -284,9 +284,9 @@ def test_relink_simple(self): tagged_error_content = self.BuildErrorContent.create({'content': 'foo bar'}) tagged_error = tagged_error_content.error_id - tagged_error.test_tags = 'bartag' + tagged_error.test_tags = 'bartag[@test, comma]\nbartag2[@test, comma],bartag3[@test, comma]' (error_content_a | tagged_error_content)._relink() - self.assertEqual(error_a.test_tags, 'footag,bartag') + self.assertEqual(error_a.test_tags, 'footag[@test, comma]\nfootag2[@test, comma]\nfootag3[@test, comma]\nbartag[@test, comma]\nbartag2[@test, comma]\nbartag3[@test, comma]') self.assertTrue(error_a.active) self.assertFalse(tagged_error.active) @@ -869,7 +869,38 @@ def test_error_content_multiple_canonical_tags(self): self.assertEqual(error_content_2.canonical_tag, '/web/tests/test_file.py:TestUi.TestUi') self.assertNotEqual(error_content_1, error_content_2) self.assertEqual(error_content_1.error_id, error_content_2.error_id) - self.assertEqual(error_content_1.error_id.canonical_tags, '/base/tests/test_file.py:TestUi.TestUi,/web/tests/test_file.py:TestUi.TestUi') + self.assertEqual(error_content_1.error_id.canonical_tags, '/base/tests/test_file.py:TestUi.TestUi\n/web/tests/test_file.py:TestUi.TestUi') + error_content_1.error_id.test_tags = error_content_1.error_id.canonical_tags + self.assertEqual(error_content_1.error_id._disabling_tags(), [ + '-/base/tests/test_file.py:TestUi.TestUi', + '-/web/tests/test_file.py:TestUi.TestUi', + ]) + error_content_3 = self.env['runbot.build.error.content'].create({ + 'content': 'Tour foobar_tour failed at step step_14 in mode mode', + 'metadata': {'test': {'canonical_tag': '/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma\\[\\] and brackets]'}}, + }) + self.assertEqual(error_content_3.canonical_tag, '/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma\\[\\] and brackets]') + self.assertNotEqual(error_content_1, error_content_2) + self.assertEqual(error_content_1.error_id, error_content_2.error_id) + self.assertEqual(error_content_1.error_id.canonical_tags, """/base/tests/test_file.py:TestUi.TestUi\n/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma\\[\\] and brackets]\n/web/tests/test_file.py:TestUi.TestUi""") + error_content_1.error_id.test_tags = error_content_1.error_id.canonical_tags + self.assertEqual(error_content_1.error_id._disabling_tags(), [ + '-/base/tests/test_file.py:TestUi.TestUi', + '-/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma\\[\\] and brackets]', + '-/web/tests/test_file.py:TestUi.TestUi', + ], "disabling tags must keep the escaping and correct format") + error_content_1.error_id.test_tags = error_content_1.error_id.test_tags.replace('\n', ',') + self.assertEqual(error_content_1.error_id._disabling_tags(), [ + '-/base/tests/test_file.py:TestUi.TestUi', + '-/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma\\[\\] and brackets]', + '-/web/tests/test_file.py:TestUi.TestUi', + ], "_disabling_tags shoudl also work fine with comma separted tags") + error_content_1.error_id.test_tags = error_content_1.error_id.test_tags.replace('\\', '') + self.assertEqual(error_content_1.error_id._disabling_tags(), [ + '-/base/tests/test_file.py:TestUi.TestUi', + '-/web/tests/test_file.py:TestJs.test_unit_desktop[@web/some , comma[] and brackets]', + '-/web/tests/test_file.py:TestUi.TestUi', + ]) class TestCodeOwner(RunbotCase): diff --git a/runbot/views/build_error_views.xml b/runbot/views/build_error_views.xml index cee4d2d8e..6eec4d12e 100644 --- a/runbot/views/build_error_views.xml +++ b/runbot/views/build_error_views.xml @@ -66,7 +66,7 @@ - +
    From eee3037b59c7e5f53eba4d546cd2642f8b212146 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Mon, 4 May 2026 10:00:26 +0200 Subject: [PATCH 11/38] [FIX] runbot: better support newid --- runbot/models/build_error.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index 711a88667..00bd5e9b7 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -429,10 +429,10 @@ def _compute_unique_qualifiers(self): @api.depends('common_qualifiers') def _compute_similar_ids(self): for record in self: - if record.common_qualifiers: + if record.common_qualifiers and (record.id or record.id.origin): query = SQL( r"""SELECT id FROM runbot_build_error WHERE id != %s AND common_qualifiers @> %s""", - record.id, + record.id or record.id.origin, json.dumps(record.common_qualifiers.dict), ) self.env.cr.execute(query) @@ -443,10 +443,10 @@ def _compute_similar_ids(self): @api.depends('common_qualifiers') def _compute_similar_content_ids(self): for record in self: - if record.common_qualifiers: + if record.common_qualifiers and (record.id or record.id.origin): query = SQL( r"""SELECT id FROM runbot_build_error_content WHERE error_id != %s AND qualifiers @> %s""", - record.id, + record.id or record.id.origin, json.dumps(record.common_qualifiers.dict), ) self.env.cr.execute(query) @@ -457,10 +457,10 @@ def _compute_similar_content_ids(self): @api.depends('common_qualifiers') def _compute_analogous_ids(self): for record in self: - if record.common_qualifiers: + if record.common_qualifiers and (record.id or record.id.origin): query = SQL( r"""SELECT id FROM runbot_build_error WHERE id != %s AND unique_qualifiers @> %s""", - record.id, + record.id or record.id.origin, json.dumps(record.unique_qualifiers.dict), ) self.env.cr.execute(query) @@ -471,10 +471,10 @@ def _compute_analogous_ids(self): @api.depends('common_qualifiers') def _compute_analogous_content_ids(self): for record in self: - if record.common_qualifiers: + if record.common_qualifiers and (record.id or record.id.origin): query = SQL( r"""SELECT id FROM runbot_build_error_content WHERE error_id != %s AND qualifiers @> %s""", - record.id, + record.id or record.id.origin, json.dumps(record.unique_qualifiers.dict), ) self.env.cr.execute(query) @@ -1010,10 +1010,10 @@ def _compute_error_display_id(self): def _compute_similar_ids(self): """error contents having the exactly the same qualifiers""" for record in self: - if record.qualifiers: + if record.qualifiers and (record.id or record.id.origin): query = SQL( r"""SELECT id FROM runbot_build_error_content WHERE id != %s AND qualifiers @> %s AND qualifiers <@ %s""", - record.id, + record.id or record.id.origin, json.dumps(record.qualifiers.dict), json.dumps(record.qualifiers.dict), ) From 039f373eee092bef1d31ca08fd3fa8c71798d1ae Mon Sep 17 00:00:00 2001 From: "Sylvio Poliart (sypol)" Date: Thu, 30 Apr 2026 16:00:09 +0200 Subject: [PATCH 12/38] [IMP] runbot: prefer successful build for similar build quick result When reusing a similar build as a quick result, first look for a build with global_result == 'ok'. Fall back to the existing logic (any done build not skipped/killed) only if no successful build is found. Without this, a previously failed build could be used to shortcut subsequent similar builds, propagating the failure without letting them run. --- runbot/models/build_config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index ac34d1ae0..8371b3f4b 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -1039,7 +1039,9 @@ def get_reference_builds_for_versions(versions): ) if self.allow_similar_build_quick_result: - existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed')), None) + existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.global_result == 'ok'), None) + if not existing_done_build: + existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed')), None) if existing_done_build: child._log('', 'A similar [build](%s) has been found, marking as done directly', existing_done_build.build_url, log_type='markdown') child.local_state = 'done' From c3cbbe25db22d14509c6678c175d27c7769f9b01 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Wed, 22 Apr 2026 16:26:05 +0200 Subject: [PATCH 13/38] [IMP] runbot: detect and list duplicate breaking PR on build errors When a breaking PR is assigned to multiple build errors, a stat button now appears showing the count of duplicates. Clicking it opens a list of errors sharing the same breaking PR, allowing the user to review and potentially merge them. --- runbot/models/build_error.py | 33 ++++++++++++++++++++++++++ runbot/tests/test_build_error.py | 38 ++++++++++++++++++++++++++++++ runbot/views/build_error_views.xml | 3 +++ 3 files changed, 74 insertions(+) diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index 00bd5e9b7..470e64774 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -240,6 +240,7 @@ class BuildError(models.Model): breaking_bundle_id = fields.Many2one('runbot.bundle', 'Breaking bundle', tracking=True, help="Bundle that introduced the error", related='breaking_pr_id.bundle_id') breaking_bundle_url = fields.Char('Breaking bundle url', related='breaking_bundle_id.frontend_url') breaking_pr_date = fields.Datetime('Breaking date', related="breaking_pr_id.close_date", help="Date of the merge of the first pr") + duplicate_breaking_pr_count = fields.Integer('Same Breaking PR', compute='_compute_duplicate_breaking_pr_count', help='Other errors with same breaking PR') test_tags = fields.Text(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True) canonical_tags = fields.Text('Canonical tag', compute='_compute_canonical_tags', store=True) @@ -320,6 +321,25 @@ def _compute_fixing_bundle_id(self): for record in self: record.fixing_bundle_id = record.fixing_pr_id.bundle_id if record.fixing_pr_id else False + @api.depends('breaking_pr_id') + def _compute_duplicate_breaking_pr_count(self): + breaking_counts = self.env["runbot.build.error"]._read_group( + domain=[ + ("breaking_pr_id", "in", self.breaking_pr_id.ids), + ("active", "=", True), + ], + groupby=["breaking_pr_id"], + aggregates=["id:count"], + having=[('id:count', '>', 1)], + ) + + count_by_pr = {pr_count[0]: pr_count[1] for pr_count in breaking_counts} + + for record in self: + # remove 1 to not count the current error + record.duplicate_breaking_pr_count = count_by_pr.get(record.breaking_pr_id, 1) - 1 + + @api.depends('error_content_ids.version_ids') def _compute_version_ids(self): for record in self: @@ -719,6 +739,19 @@ def action_view_analogous_qualified_contents(self): 'name': 'Similary Qualified Contents' } + def action_view_duplicate_breaking_pr(self): + self.ensure_one() + return { + 'type': 'ir.actions.act_window', + 'res_model': 'runbot.build.error', + 'domain': [ + ('breaking_pr_id', '=', self.breaking_pr_id.id), + ('active', '=', True), + ], + 'view_mode': 'list,form', + 'name': 'Errors with same breaking PR', + } + @api.depends('manual_team_id', 'auto_team_id') def _compute_team_id(self): for error in self: diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py index 3525871cb..0217e3182 100644 --- a/runbot/tests/test_build_error.py +++ b/runbot/tests/test_build_error.py @@ -219,6 +219,44 @@ def test_merge_pr_ids(self): self.assertEqual(error_a.fixing_pr_id, self.dev_pr) self.assertEqual(error_a.breaking_pr_id, self.dev_pr) + def test_duplicate_breaking_pr(self): + pr_branch = self.Branch.create({ + 'name': '242', + 'is_pr': True, + 'alive': True, + 'remote_id': self.remote_odoo.id, + 'target_branch_name': self.branch_odoo.name, + 'pull_head_name': f'{self.remote_odoo.owner}:{self.dev_branch.name}', + }) + + error_a = self.BuildError.create({'content': 'error A', 'breaking_pr_id': pr_branch.id}) + error_b = self.BuildError.create({'content': 'error B', 'breaking_pr_id': pr_branch.id}) + error_c = self.BuildError.create({'content': 'error C', 'breaking_pr_id': pr_branch.id}) + error_d = self.BuildError.create({'content': 'error D'}) + + # Test compute count (excludes self) + self.assertEqual(error_a.duplicate_breaking_pr_count, 2) + self.assertEqual(error_b.duplicate_breaking_pr_count, 2) + self.assertEqual(error_c.duplicate_breaking_pr_count, 2) + self.assertEqual(error_d.duplicate_breaking_pr_count, 0) + + # Test action returns all errors including self + action = error_a.action_view_duplicate_breaking_pr() + self.assertEqual(action['type'], 'ir.actions.act_window') + self.assertEqual(action['res_model'], 'runbot.build.error') + errors_in_action = self.BuildError.search(action['domain']) + self.assertIn(error_a, errors_in_action) + self.assertEqual(len(errors_in_action), 3) + + # Test count update on deactivation + error_b.active = False + error_a.invalidate_recordset(['duplicate_breaking_pr_count']) + self.assertEqual(error_a.duplicate_breaking_pr_count, 1) + + # Test count update on breaking PR removal + error_a.breaking_pr_id = False + self.assertEqual(error_a.duplicate_breaking_pr_count, 0) + def test_relink_contents(self): build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) error_content_a = self.BuildErrorContent.create({'content': 'foo bar'}) diff --git a/runbot/views/build_error_views.xml b/runbot/views/build_error_views.xml index 6eec4d12e..e521b5eb4 100644 --- a/runbot/views/build_error_views.xml +++ b/runbot/views/build_error_views.xml @@ -10,6 +10,9 @@ +
    -
    -
    - -
    -
    - - -
    -
    - -
    - - - - -
    -
    - -
    - - - - -
    -
    - -
    - -
    - -
    -
    - - - - -

    -
    - -
    - -
    +
    +
    + + + + +
    + +

    + From 0b4fc5c96d87e78a6063c29d85168c226c902bf2 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 8 May 2026 11:38:20 +0200 Subject: [PATCH 20/38] [FIX] runbot: cpu_limit can be None --- runbot/models/build_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 5b804f444..85ff7af8d 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -597,7 +597,7 @@ def _run_step(self, build, **kwargs): docker_params['cpu_limit'] = min(self.cpu_limit, max_timeout) config_data = {**kwargs.get('config_data', {}), **build.params_id.config_data} - if config_data.get('cpu_limit_factor'): + if docker_params['cpu_limit'] and config_data.get('cpu_limit_factor'): docker_params['cpu_limit'] = int(docker_params['cpu_limit'] * min(float(config_data['cpu_limit_factor']), 2)) container_cpus = float(self.container_cpus or self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_containers_cpus', 0)) @@ -849,7 +849,7 @@ def _run_install_odoo(self, build, config_data=None): if config_data.get('coverage_test_context', self.coverage_test_context): env_variables.append("COVERAGE_DYNAMIC_CONTEXT=test_function") - cpu_limit = None + cpu_limit = self.cpu_limit if config_data.get('cpu_limit'): cpu_limit = min(self.cpu_limit, int(config_data['cpu_limit'])) return dict(cmd=cmd, ro_volumes=exports, cpu_limit=cpu_limit, env_variables=env_variables) From 93b26dc0734f6353cf7917b1ff6b795a073f0314 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Thu, 7 May 2026 10:55:07 +0200 Subject: [PATCH 21/38] [IMP] runbot: replace keep_running by gc_running_date The `keep_running` boolean field on BuildResult was a simple flag to prevent builds from being automatically killed. The drawback is that sometimes those builds are forgotten and stays alive forever. This commit replaces it with a `gc_running_date` field. Now, if a date is set, the build can only be killed by _gc_running after that date. --- runbot/models/build.py | 2 +- runbot/models/runbot.py | 2 +- runbot/views/build_views.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index 4b9a03b56..dea856f12 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -361,7 +361,7 @@ class BuildResult(models.Model): build_url = fields.Char('Build url', compute='_compute_build_url', store=False) build_error_link_ids = fields.One2many('runbot.build.error.link', 'build_id') build_error_ids = fields.Many2many('runbot.build.error', compute='_compute_build_error_ids', string='Errors') - keep_running = fields.Boolean('Keep running', help='Keep running', index=True) + gc_running_date = fields.Date('GC Running Date', help='Running build cannot be killed before this date', index='btree_not_null') log_counter = fields.Integer('Log Lines counter', default=100) slot_ids = fields.One2many('runbot.batch.slot', 'build_id') diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index b470e1df9..195efe4a7 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -104,7 +104,7 @@ def _get_builds_to_init(self, host): def _gc_running(self, host): running_max = host._get_running_max() Build = self.env['runbot.build'] - cannot_be_killed_ids = host._get_builds([('keep_running', '=', True)]).ids + cannot_be_killed_ids = host._get_builds([('gc_running_date', '>', fields.Date.today())]).ids sticky_bundles = self.env['runbot.bundle'].search([('sticky', '=', True), ('project_id.keep_sticky_running', '=', True)]) cannot_be_killed_ids += [ build.id diff --git a/runbot/views/build_views.xml b/runbot/views/build_views.xml index ce996e5b4..71a1b49e5 100644 --- a/runbot/views/build_views.xml +++ b/runbot/views/build_views.xml @@ -82,7 +82,7 @@ - + From 77f76552d1d3cf1200b35f44ad37c6f1bababda2 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 27 May 2026 09:09:58 +0200 Subject: [PATCH 22/38] [IMP] runbot: add support for fetching tokens from https urls Fetch using ssh are slow those days, adding support for fetch using https with token --- runbot/models/repo.py | 9 +++++++++ runbot/templates/git.xml | 2 +- runbot_builder/builder.py | 5 +++-- runbot_builder/leader.py | 12 ++++++++---- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 175721603..64661f834 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -264,6 +264,11 @@ def _compute_remote_name(self): for remote in self: remote.remote_name = sanitize(remote.short_name) + def _get_fetch_url(self): + if not self.name.startswith('https://') or not self.token: + return self.name + return self.name.replace('https://', 'https://%s@' % self.token, 1) + def create(self, values_list): remote = super().create(values_list) if not remote.repo_id.main_remote_id: @@ -685,6 +690,9 @@ def _update_batches(self, force=False, ignore=None): def _update_git_config(self): """ Update repo git config file """ + if not self: + return + _logger.info('Updating git config for %s repos', len(self)) for repo in self: if repo.mode == 'disabled': _logger.info(f'skipping disabled repo {repo.name}') @@ -698,6 +706,7 @@ def _update_git_config(self): _logger.info('Config updated for repo %s' % repo.name) else: _logger.info('Repo not cloned, skiping config update for %s' % repo.name) + return max(self.mapped('write_date')) def _git_init(self): """ Clone the remote repo if needed """ diff --git a/runbot/templates/git.xml b/runbot/templates/git.xml index f69633e9e..8d8968f81 100644 --- a/runbot/templates/git.xml +++ b/runbot/templates/git.xml @@ -7,7 +7,7 @@ bare = true [remote ""] - url = + url = fetch = +refs/heads/*:refs//heads/* fetch = +refs/pull/*/head:refs//pull/* diff --git a/runbot_builder/builder.py b/runbot_builder/builder.py index 90637ee64..5ca51d0c8 100755 --- a/runbot_builder/builder.py +++ b/runbot_builder/builder.py @@ -13,6 +13,7 @@ class BuilderClient(RunbotClient): def on_start(self): + self.last_update = datetime(1970, 1, 1) self.last_docker_updates = None if self.host.is_builder: builds_path = self.env['runbot.runbot']._path('build') @@ -30,6 +31,8 @@ def loop_turn(self): self.host._docker_update_images() self.env.cr.commit() if self.host.is_builder: + self.last_update = self.env['runbot.repo'].search([('write_date', '>', self.last_update)])._update_git_config() + self.env.cr.commit() if self.count == 1: # cleanup at second iteration self.env['runbot.runbot']._source_cleanup() self.env.cr.commit() @@ -39,8 +42,6 @@ def loop_turn(self): self.env.cr.commit() self.host._set_psql_conn_count() self.env.cr.commit() - self.env['runbot.repo']._update_git_config() - self.env.cr.commit() self.git_gc() self.env.cr.commit() return self.env['runbot.runbot']._scheduler_loop_turn(self.host) diff --git a/runbot_builder/leader.py b/runbot_builder/leader.py index c39cb8ea5..106d83eb1 100755 --- a/runbot_builder/leader.py +++ b/runbot_builder/leader.py @@ -1,23 +1,27 @@ #!/usr/bin/python3 from tools import RunbotClient, run import logging -import time + +from datetime import datetime + _logger = logging.getLogger(__name__) -class LeaderClient(RunbotClient): # Conductor, Director, Main, Maestro, Lead +class LeaderClient(RunbotClient): def __init__(self, env): self.pull_info_failures = {} + self.last_update = datetime(1970, 1, 1) super().__init__(env) def loop_turn(self): if not self.host.is_leader: _logger.warning('Leader client is not a leader host, skipping loop_turn') return 10 + + self.last_update = self.env['runbot.repo'].search([('write_date', '>', self.last_update)])._update_git_config() + self.env.cr.commit() if self.count == 0: - self.env['runbot.repo']._update_git_config() - self.env.cr.commit() self.git_gc() self.env.cr.commit() From 88559c587635dd2c18c169a3861695e88af8b56e Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 27 May 2026 11:28:04 +0200 Subject: [PATCH 23/38] [IMP] runbot: only fetch commits When runbot fetch a commit, it will try to fetch the complete repo first The main reason is that the other commit, branches, bob, .... could be needed later, so making one call to github looks more efficient. With the increasing number of dev branches and workers, the fetch becomes a problem and can be slow, mainly when no fetch was done for a while, and the repo has a tons of pending updates. This pr tries another approch to only fetch the single requested commit, which could be faster. --- runbot/models/commit.py | 2 +- runbot/models/repo.py | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/runbot/models/commit.py b/runbot/models/commit.py index 13fa32bf4..b0aba479a 100644 --- a/runbot/models/commit.py +++ b/runbot/models/commit.py @@ -106,7 +106,7 @@ def _export(self, build): export_path = self._source_path() if os.path.isdir(export_path): - _logger.info('git export: exporting to %s (already exists)', export_path) + _logger.debug('git export: exporting to %s (already exists)', export_path) return export_path _logger.info('git export: exporting to %s (new)', export_path) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 64661f834..77b8c6dd9 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -524,17 +524,14 @@ def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False): def _fetch(self, sha): if not self._hash_exists(sha): - self._update(force=True) + for remote in self.remote_ids: + try: + self._git(['fetch', remote.remote_name, sha]) + break + except subprocess.CalledProcessError: + pass if not self._hash_exists(sha): - for remote in self.remote_ids: - try: - self._git(['fetch', remote.remote_name, sha]) - _logger.info('Success fetching specific head %s on %s', sha, remote) - break - except subprocess.CalledProcessError: - pass - if not self._hash_exists(sha): - raise RunbotException("Commit %s is unreachable. Did you force push the branch?" % sha) + raise RunbotException("Commit %s is unreachable, most likely because it is not attached to any branch anymore" % sha) def _hash_exists(self, commit_hash): """ Verify that a commit hash exists in the repo """ From 882bcbdfa50c6551cce26f90fddee99eaa50c932 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Wed, 27 May 2026 14:22:30 +0200 Subject: [PATCH 24/38] [IMP] runbot: kill hanging git child process When fetching from GitHub, the git process sometimes hangs for a long time regardless of whether https or ssh is used. Investigation showed that the last child process of the git command is always the one hanging. Killing that subprocess allows git to retry the operation without data loss. This commit monitors the git process and kills its hanging child process on timeout. --- runbot/models/repo.py | 32 +++++++++++++++++++++++------ runbot/tests/test_repo.py | 42 ++++++++++++++++++++------------------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 77b8c6dd9..51f6a9c8c 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -6,6 +6,7 @@ import re import subprocess import time +import psutil import requests import markupsafe import shlex @@ -512,12 +513,31 @@ def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False): cmd = self._get_git_command(cmd, errors) if not quiet: _logger.info("git command: %s", shlex.join(cmd)) - kwargs = {'stderr': subprocess.STDOUT} - if input_data is not None: - if isinstance(input_data, str): - input_data = input_data.encode('utf-8') - kwargs['input'] = input_data - output = subprocess.check_output(cmd, **kwargs) + + if input_data is not None and isinstance(input_data, str): + input_data = input_data.encode('utf-8') + + stdin = subprocess.PIPE if input_data is not None else None + process = subprocess.Popen(cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + + fetch_timeout = int(self.env['ir.config_parameter'].get_param('runbot.runbot_fetch_timeout', default=30)) + while True: + try: + output, _ = process.communicate(input=input_data, timeout=fetch_timeout) + break + except subprocess.TimeoutExpired: + try: + parent = psutil.Process(process.pid) + for child in parent.children(recursive=True): + if 'git-upload-pack' in str(child.cmdline()) and child.status() == psutil.STATUS_SLEEPING: + child.kill() + _logger.info("Killed sleeping git subprocess (pid: %s)", child.pid) + except psutil.NoSuchProcess: + pass + + if process.returncode: + raise subprocess.CalledProcessError(process.returncode, cmd, output=output) + if raw: return output return output.decode(errors=errors) diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index cb8195a05..d4c40fcae 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -383,26 +383,28 @@ def mock_git_helper(self, repo, cmd, input_data=None, raw=False): class TestIdentityFile(RunbotCase): - def check_output_helper(self): - """Helper that returns a mock for repo._git()""" - def mock_check_output(cmd, *args, **kwargs): - expected_option = r'-c core.sshCommand=ssh -i \/.+\/\.ssh\/fake_identity' - git_cmd = ' '.join(cmd) - self.assertTrue(re.search(expected_option, git_cmd), '%s did not match %s' % (git_cmd, expected_option)) - return Mock() - - return mock_check_output - - def test_identity_file(self): - """test that the identity file is used in git command""" - - self.patcher_objects['git_patcher'].stop() - self.start_patcher('check_output_patcher', 'odoo.addons.runbot.models.repo.subprocess.check_output', new=self.check_output_helper()) - - self.repo_odoo.identity_file = 'fake_identity' - - with mute_logger("odoo.addons.runbot.models.repo"): - self.repo_odoo._update_fetch_cmd() + def popen_helper(self): + """Helper that returns a mock for repo._git()""" + def mock_popen(cmd, *args, **kwargs): + expected_option = r'-c core.sshCommand=ssh -i \/.+\/\.ssh\/fake_identity' + git_cmd = ' '.join(cmd) + self.assertTrue(re.search(expected_option, git_cmd), '%s did not match %s' % (git_cmd, expected_option)) + popen_mock = Mock() + attrs = {"communicate.return_value": (b"", b"")} + popen_mock.configure_mock(**attrs) + return popen_mock + + return mock_popen + + def test_identity_file(self): + """test that the identity file is used in git command""" + + self.patcher_objects['git_patcher'].stop() + self.start_patcher('popen_patcher', 'odoo.addons.runbot.models.repo.subprocess.Popen', new=self.popen_helper()) + self.repo_odoo.identity_file = 'fake_identity' + + with mute_logger("odoo.addons.runbot.models.repo"): + self.repo_odoo._update_fetch_cmd() class TestRepoScheduler(RunbotCase): From bc209f9fe5afc40e738bee12959fdc7e855c9455 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 29 May 2026 11:39:51 +0200 Subject: [PATCH 25/38] [FIX] runbot: retry in case of killed fetch Since #1418 when a fetch is too long it can be killed to prevent staying on a slow fetch This works well with the _update_fetch_cmd since it will retry multiple times, but not with the commit._fetch that won't retry at all. If a commit takes more than 30 second to fetch the build will fail in this case. Adding a retry inside the _git in case of killed fetch and increase the timeout. --- runbot/models/repo.py | 44 ++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 51f6a9c8c..0534e44df 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -511,30 +511,36 @@ def _get_git_command(self, cmd, errors='strict'): def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False): cmd = self._get_git_command(cmd, errors) - if not quiet: - _logger.info("git command: %s", shlex.join(cmd)) - if input_data is not None and isinstance(input_data, str): input_data = input_data.encode('utf-8') - stdin = subprocess.PIPE if input_data is not None else None - process = subprocess.Popen(cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - fetch_timeout = int(self.env['ir.config_parameter'].get_param('runbot.runbot_fetch_timeout', default=30)) - while True: - try: - output, _ = process.communicate(input=input_data, timeout=fetch_timeout) - break - except subprocess.TimeoutExpired: - try: - parent = psutil.Process(process.pid) - for child in parent.children(recursive=True): - if 'git-upload-pack' in str(child.cmdline()) and child.status() == psutil.STATUS_SLEEPING: - child.kill() - _logger.info("Killed sleeping git subprocess (pid: %s)", child.pid) - except psutil.NoSuchProcess: - pass + for i in range(3): # retry in case of timeout + if not quiet: + _logger.info("git command: %s", shlex.join(cmd)) + killed_fetch = False + stdin = subprocess.PIPE if input_data is not None else None + process = subprocess.Popen(cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + + while True: + try: + output, _ = process.communicate(input=input_data, timeout=fetch_timeout) + break + except subprocess.TimeoutExpired: + try: + parent = psutil.Process(process.pid) + for child in parent.children(recursive=True): + if 'git-upload-pack' in str(child.cmdline()) and child.status() == psutil.STATUS_SLEEPING: + child.kill() + _logger.info("Killed sleeping git subprocess (pid: %s)", child.pid) + killed_fetch = True + fetch_timeout *= 10 # increase the timeout for the next try + except psutil.NoSuchProcess: + pass + + if not killed_fetch: + break if process.returncode: raise subprocess.CalledProcessError(process.returncode, cmd, output=output) From 3b0d623973e6758263feb02c670e692c499c08c5 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 29 May 2026 14:26:00 +0200 Subject: [PATCH 26/38] [FIX] runbot: add missing gitinit Since we don't call update anymore, it is possible that the repo is not initialized yet. --- runbot/models/repo.py | 1 + 1 file changed, 1 insertion(+) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 0534e44df..32c12acb6 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -549,6 +549,7 @@ def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False): return output.decode(errors=errors) def _fetch(self, sha): + self._git_init() if not self._hash_exists(sha): for remote in self.remote_ids: try: From 751986da58fbf72d4c7f7e671e1d5aaad1609fe6 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Fri, 22 May 2026 17:53:20 +0200 Subject: [PATCH 27/38] [IMP] runbot: add a nocache toggle to docker build When building Dockerfiles, it happens that even if nothing changed in the Dockerfile itself, the image should be rebuilt. e.g.: depending on when an image is built, an `apt-get install foobartool` may give a newer version of `foobartool`. --- runbot/container.py | 9 +++++---- runbot/models/docker.py | 6 +++++- runbot/views/dockerfile_views.xml | 3 +++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/runbot/container.py b/runbot/container.py index f52456c1e..2fe340d9d 100644 --- a/runbot/container.py +++ b/runbot/container.py @@ -108,21 +108,22 @@ def get_config(self, starting_config=''): return res.read() -def docker_build(build_dir, image_tag, pull=False): - return _docker_build(build_dir, image_tag, pull) +def docker_build(build_dir, image_tag, pull=False, nocache=False): + return _docker_build(build_dir, image_tag, pull, nocache) -def _docker_build(build_dir, image_tag, pull=False): +def _docker_build(build_dir, image_tag, pull=False, nocache=False): """Build the docker image :param build_dir: the build directory that contains Dockerfile. :param image_tag: name used to tag the resulting docker image + :param nocache: bypass Docker layer cache when True :return: dict """ with DockerManager(image_tag) as dm: last_step = None dm.result['success'] = False # waiting for an image_id - for chunk in dm.consume(dm.docker_client.api.build(path=build_dir, tag=image_tag, rm=True, pull=pull)): + for chunk in dm.consume(dm.docker_client.api.build(path=build_dir, tag=image_tag, rm=True, pull=pull, nocache=nocache)): if 'stream' in chunk: stream = chunk['stream'] if stream.startswith('Step '): diff --git a/runbot/models/docker.py b/runbot/models/docker.py index 547a3c6e0..611435443 100644 --- a/runbot/models/docker.py +++ b/runbot/models/docker.py @@ -145,6 +145,7 @@ class Dockerfile(models.Model): dockerfile = fields.Text(compute='_compute_dockerfile', recursive=True, tracking=True) in_error = fields.Boolean('In error', help='The last build failed.', default=False) to_build = fields.Boolean('To Build', help='Build Dockerfile. Check this when the Dockerfile is ready.', default=True) + nocache = fields.Boolean('No Cache', help='Force a full rebuild on next build, bypassing the Docker layer cache. Automatically reset to False after the build.', copy=False) always_pull = fields.Boolean('Always pull', help='Always Pull on the hosts, not only at the use time', default=False, tracking=True, copy=False) version_ids = fields.One2many('runbot.version', 'dockerfile_id', string='Versions') description = fields.Text('Description') @@ -383,7 +384,7 @@ def _build(self, host=None): content = self._get_cached_content(docker_build_path) with open(self.env['runbot.runbot']._path('docker', tag_dir, 'Dockerfile'), 'w', encoding="utf-8") as Dockerfile: Dockerfile.write(content) - result = docker_build(docker_build_path, self.image_future_tag, self.pull_on_build) + result = docker_build(docker_build_path, self.image_future_tag, self.pull_on_build, self.nocache) duration = result['duration'] msg = result['msg'] success = image_id = result.get('image_id') @@ -427,6 +428,9 @@ def clean_output(output): message = f'Build failure, check results for more info ({result.summary})' self.message_post(body=message) _logger.error(message) + + if self.nocache: + self.nocache = False return image_id diff --git a/runbot/views/dockerfile_views.xml b/runbot/views/dockerfile_views.xml index dab232b01..27d45ac5e 100644 --- a/runbot/views/dockerfile_views.xml +++ b/runbot/views/dockerfile_views.xml @@ -23,6 +23,7 @@ + @@ -107,6 +108,8 @@ + + From 64acffdb7b83da1b9d43fe6fb0a1899377b218c1 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 20 May 2026 11:00:07 +0200 Subject: [PATCH 28/38] [FIX] runbot: warning should not be emited for kill message --- runbot/models/host.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runbot/models/host.py b/runbot/models/host.py index 6d657d90f..6b9055ef5 100644 --- a/runbot/models/host.py +++ b/runbot/models/host.py @@ -369,6 +369,7 @@ def _process(self): if build.local_state != 'running' and build.global_result not in ('warn', 'ko'): result = 'killed' build._kill(result=result) - self.env['runbot.runbot']._warning(f'Host {record.host_id.name} got an unexpected message {record.message}') + else: + self.env['runbot.runbot']._warning(f'Host {record.host_id.name} got an unexpected message {record.message}') self.unlink() return processed From 218539d89d5fe99ff67bc49b871d477f118204a9 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 8 May 2026 12:04:06 +0200 Subject: [PATCH 29/38] [FIX] runbot: restore cpu_limit Since dynamic config were introduced, a Falsy value was given as cpu limit bypassing the step limit and max limit in run_odoo_odoo This is fixed by setting the step limit fist in run_odoo_odoo The limit is also set on the docker but improprely used to kill the build manually. To avoid having to compute it twice the same way, the limit is now stored on the build and the same value is used to gc the build. We still have a fallback on the step limit, so both values needs to be Falsy in order to disable the limit, and provide a default waiting for the build.cpu_limit to be set on all builds. --- runbot/models/build.py | 6 ++++-- runbot/models/build_config.py | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index dea856f12..def701191 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -317,7 +317,7 @@ class BuildResult(models.Model): active_step = fields.Many2one('runbot.build.config.step', 'Active step') job = fields.Char('Active step display name', compute='_compute_job') dynamic_active_step_index = fields.Integer('Dynamic active step index') - + cpu_limit = fields.Integer('CPU limit for the current running docker') job_start = fields.Datetime('Job start') job_end = fields.Datetime('Job end') build_start = fields.Datetime('Build start') @@ -886,7 +886,8 @@ def _schedule(self): else: _docker_state = docker_state(build._get_docker_name(), build._path()) if _docker_state == 'RUNNING': - timeout = min(build.active_step.cpu_limit, int(icp.get_param('runbot.runbot_timeout', default=10000))) + build_limit = build.cpu_limit or build.active_step.cpu_limit + timeout = min(build_limit, int(icp.get_param('runbot.runbot_timeout', default=10000))) if build.local_state != 'running' and build.job_time > timeout: build.active_step._make_stats(build) build._log('_schedule', '%s time exceeded (%ss)' % (build.active_step._get_display_name(self) if build.active_step else "?", build.job_time)) @@ -1064,6 +1065,7 @@ def _docker_run(self, step, cmd=None, ro_volumes=None, env_variables=None, **kwa self.env.flush_all() env_variables = env_variables or [] env_variables.append('ODOO_RUNBOT=1') + self.cpu_limit = kwargs.get('cpu_limit') def start_docker(): docker_run( cmd=cmd, diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 85ff7af8d..22dbe68bf 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -598,7 +598,7 @@ def _run_step(self, build, **kwargs): config_data = {**kwargs.get('config_data', {}), **build.params_id.config_data} if docker_params['cpu_limit'] and config_data.get('cpu_limit_factor'): - docker_params['cpu_limit'] = int(docker_params['cpu_limit'] * min(float(config_data['cpu_limit_factor']), 2)) + docker_params['cpu_limit'] = int(docker_params['cpu_limit'] * min(float(config_data['cpu_limit_factor']), 3)) container_cpus = float(self.container_cpus or self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_containers_cpus', 0)) if 'cpus' not in docker_params and container_cpus: @@ -1643,6 +1643,9 @@ def _run_dynamic(self, build): if 'extra_params' in current_step: config_data['extra_params'] = self._parse_dynamic_entry(current_step.get('extra_params'), build, dynamic_vars) + if 'cpu_limit' in current_step: + config_data['cpu_limit'] = int(current_step.get('cpu_limit')) + for key in ('screencast', 'demo_mode', 'enable_auto_tags'): if key in current_step: value = current_step[key] From ee2484063605b28e5b47eda771cdd4fb03da1e3f Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Thu, 9 Apr 2026 14:44:38 +0200 Subject: [PATCH 30/38] [IMP] runbot: handle main command exit status Since the runbot executes Docker containers non-blockingly, there is no direct way to capture the main command's exit status. With this commit the exit status code of the main command is written to a file. It also adds a new boolean field 'check_exit_status' on ConfigStep that enables checking the exit status. When enabled, the step reads the exit status from the dedicated file and sets the build result to 'ko' if the exit code is non-zero. This allows config steps to properly detect and handle command failures that may not produce error logs but still indicate a failed execution. --- runbot/models/build.py | 2 + runbot/models/build_config.py | 22 ++++++ runbot/tests/test_build_config_step.py | 105 ++++++++++++++++++++++++- 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index def701191..52c4ec2c9 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1052,6 +1052,8 @@ def _docker_run(self, step, cmd=None, ro_volumes=None, env_variables=None, **kwa starting_config = self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_default_odoorc') if isinstance(cmd, Command): rc_content = cmd.get_config(starting_config=starting_config) + if step.check_exit_status: + cmd.finals = [['echo', r'$?', '>', f'/data/build/logs/{step.sanitized_name(self)}_exit_status.txt']] + cmd.finals else: rc_content = starting_config self._write_file('.odoorc', rc_content) diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 22dbe68bf..500252ddf 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -479,6 +479,9 @@ class ConfigStep(models.Model): restore_download_db_suffix = fields.Char('Download db suffix') restore_rename_db_suffix = fields.Char('Rename db suffix') + # TODO change the default to True once we are sure that it works as expected + check_exit_status = fields.Boolean('Check exit status', default=False, help='Check exit status of the main command') + semgrep_category = fields.Many2one('runbot.checker_category', string='Semgrep Category', tracking=True) custom_link = fields.Char('Custom link for semgrep codes', tracking=True) disable_nosem = fields.Boolean('Disable nosem', default=False, tracking=True) @@ -1282,6 +1285,10 @@ def _make_results(self, build): if log_time: build.job_end = log_time + if self.check_exit_status and (exit_status := self._get_exit_status(build)) != 0: + build._log('_make_results', f'Main command exited with status code {exit_status}', level='ERROR') + build.local_result = 'ko' + if check_logs or expected_logs: self._make_custom_result(build, check_logs, expected_logs) elif active_job_type == 'python': @@ -1421,6 +1428,21 @@ def _get_checkers_result(self, build, checkers): return result return 'ok' + def _get_exit_status(self, build): + exit_status_filename = f'{self.sanitized_name(build)}_exit_status.txt' + if not os.path.exists(build._path(exit_status_filename)): + build._log('_make_tests_results', f'Exit status file "{exit_status_filename}" not found', level="ERROR") + return 1 + res = build._read_file(exit_status_filename) + if res: + try: + return int(res.strip('\n')) + except ValueError: + build._log('_make_tests_results', f'Status file "{exit_status_filename}" does not contain an integer', level="ERROR") + return -242 + build._log('_make_tests_results', f'Exception or file empty while reading status file "{exit_status_filename}"', level="ERROR") + return -241 + def _make_custom_result(self, build, enabled_checkers=None, expected_logs=None): build._log('run', 'Getting results for build %s' % build.dest) if build.local_result != 'ko': diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index a165b68b0..36b0d9f74 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -724,11 +724,14 @@ def test_dynamic_step_l10n_standalone(self): ) # 0.2 run standalone l10n script + self.config_step.check_exit_status = True self.docker_run_calls = [] build._schedule()() self.assertEqual(len(self.docker_run_calls), 1, "One docker run should have been called for install_all step") cmd = self.docker_run_calls[0][0] - self.assertEqual(cmd.build(), f'odoo/odoo/tests/test_module_operations.py -d {build.dest}-l10n --data-dir /data/build/datadir/ --addons-path odoo/addons,odoo/core/addons,enterprise --standalone all_l10n') + expected_cmd = f'odoo/odoo/tests/test_module_operations.py -d {build.dest}-l10n --data-dir /data/build/datadir/ --addons-path odoo/addons,odoo/core/addons,enterprise --standalone all_l10n' + expected_cmd += r' ; echo $? > /data/build/logs/running_standalone_exit_status.txt' + self.assertEqual(cmd.build(), expected_cmd) # 0.3. create post install builds build._schedule() @@ -1661,3 +1664,103 @@ def make_warn(build): mock_make_odoo_results.side_effect = make_warn config_step._make_results(build) self.assertEqual(build.local_result, 'warn') + + def test_check_exit_status_ok(self): + self.config_step.write({'check_exit_status': True}) + file_content = """ +Loading stuff +odoo.stuff.modules.loading: Modules loaded. +Some post install stuff +Initiating shutdown + """ + exit_status_content = "0\n" + + def mock_open_files(filename, mode='r', *args, **kwargs): + if filename.endswith('_exit_status.txt'): + return mock_open(read_data=exit_status_content)() + return mock_open(read_data=file_content)() + + with patch('builtins.open', mock_open_files): + with patch('os.path.exists', return_value=True): + self.config_step._make_results(self.build) + + self.assertEqual(self.build.local_result, 'ok') + + def test_check_exit_status_ko(self): + self.config_step.write({'check_exit_status': True}) + file_content = """ +Loading stuff +odoo.stuff.modules.loading: Modules loaded. +Some post install stuff +Initiating shutdown + """ + exit_status_content = "1\n" + + def mock_open_files(filename, mode='r', *args, **kwargs): + if filename.endswith('_exit_status.txt'): + return mock_open(read_data=exit_status_content)() + return mock_open(read_data=file_content)() + + with patch('builtins.open', mock_open_files): + with patch('os.path.exists', return_value=True): + self.config_step._make_results(self.build) + + self.assertEqual(self.build.local_result, 'ko') + self.assertIn(('ERROR', 'Main command exited with status code 1'), self.logs) + + def test_check_exit_status_file_empty(self): + self.config_step.write({'check_exit_status': True}) + file_content = """ +Loading stuff +odoo.stuff.modules.loading: Modules loaded. +Some post install stuff +Initiating shutdown + """ + exit_status_content = "" + + def mock_open_files(filename, mode='r', *args, **kwargs): + if filename.endswith('_exit_status.txt'): + return mock_open(read_data=exit_status_content)() + return mock_open(read_data=file_content)() + + with patch('builtins.open', mock_open_files): + with patch('os.path.exists', return_value=True): + self.config_step._make_results(self.build) + + self.assertEqual(self.build.local_result, 'ko') + self.assertIn(('ERROR', 'Exception or file empty while reading status file "all_exit_status.txt"'), self.logs) + self.assertIn(('ERROR', 'Main command exited with status code -241'), self.logs) + + def test_check_exit_status_file_non_integer(self): + self.config_step.write({'check_exit_status': True}) + file_content = """ +Loading stuff +odoo.stuff.modules.loading: Modules loaded. +Some post install stuff +Initiating shutdown + """ + exit_status_content = "d0d0caca" + + def mock_open_files(filename, mode='r', *args, **kwargs): + if filename.endswith('_exit_status.txt'): + return mock_open(read_data=exit_status_content)() + return mock_open(read_data=file_content)() + + with patch('builtins.open', mock_open_files): + with patch('os.path.exists', return_value=True): + self.config_step._make_results(self.build) + + self.assertEqual(self.build.local_result, 'ko') + self.assertIn(('ERROR', 'Status file "all_exit_status.txt" does not contain an integer'), self.logs) + self.assertIn(('ERROR', 'Main command exited with status code -242'), self.logs) + + def test_check_exit_status_file_not_found(self): + config_step = self.ConfigStep.create({ + 'name': 'test', + 'job_type': 'install_odoo', + 'check_exit_status': True, + }) + self.patchers['file_exist'].return_value = False + config_step._make_results(self.build) + self.assertEqual(self.build.local_result, 'ko') + self.assertIn(('ERROR', 'Exit status file "test_exit_status.txt" not found'), self.logs) From a3a2f2c92cead38e4f65bc79eff3a97ba727146d Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 20 Mar 2026 09:01:59 +0100 Subject: [PATCH 31/38] [IMP] runbot: allow to set a memory limit factor Some tests can fail randomly because they reach memory limit. Setting a slightly lower memory limit during the nightly will help to detect such tests and provide an easier way to fix them early. --- runbot/models/build.py | 4 +++- runbot/tests/test_build_config_step.py | 28 +++++++++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index 52c4ec2c9..8ba62d047 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1041,7 +1041,9 @@ def _docker_run(self, step, cmd=None, ro_volumes=None, env_variables=None, **kwa containers_memory_limit = self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_containers_memory', 0) if containers_memory_limit and 'memory' not in kwargs: - kwargs['memory'] = int(float(containers_memory_limit) * 1024 ** 3) + memory_limit_factor = float(self.params_id.config_data.get('memory_limit_factor', 1)) + containers_memory_limit = int(float(containers_memory_limit) * 1024 ** 3) * memory_limit_factor + kwargs['memory'] = containers_memory_limit self.docker_start = now() if self.job_start: diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 36b0d9f74..1e66d26fc 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -1375,22 +1375,15 @@ def test_network_can_be_enable(self, mock_checkout): 'job_type': 'install_odoo', }) - # by default, network is disabled - def first_docker_run(cmd, log_path, *args, **kwargs): - self.assertFalse(kwargs['network_enabled']) - - self.docker_run_patch = first_docker_run config_step._run_step(self.parent_build)() - - def second_docker_run(cmd, log_path, *args, **kwargs): - self.assertTrue(kwargs['network_enabled']) - - self.docker_run_patch = second_docker_run + self.assertFalse(self.docker_run_calls[0][3]['network_enabled']) parent_build_params = self.parent_build.params_id.copy({'config_data': {'network_enabled': True}}) parent_build = self.parent_build.copy({'params_id': parent_build_params.id}) config_step._run_step(parent_build)() + self.assertTrue(self.docker_run_calls[1][3]['network_enabled']) + @patch('odoo.addons.runbot.models.build.BuildResult._checkout') def test_run_python_networkcan_be_enabled(self, mock_checkout): @@ -1431,6 +1424,21 @@ def test_install_custom_parametric_tags(self, mock_checkout, parse_config): tags = self.get_test_tags(params) self.assertEqual(tags, '"-at_install,/web/foo.py:WebSuite.test_unit_desktop[@bar/test with spaces],-/web/bar.py[@snafu/other test with spaces]"') + @patch('odoo.addons.runbot.models.build.BuildResult._checkout') + def test_memory_limit(self, mock_checkout): + """ test that network can be disabled with config_data """ + config_step = self.ConfigStep.create({ + 'name': 'default', + 'job_type': 'install_odoo', + }) + self.env['ir.config_parameter'].sudo().set_param('runbot.runbot_containers_memory', 10) + config_step._run_step(self.parent_build)() + self.assertEqual(self.docker_run_calls[0][3]['memory'], 10 * 1024 ** 3) + + parent_build_params = self.parent_build.params_id.copy({'config_data': {'memory_limit_factor': 0.9}}) + parent_build = self.parent_build.copy({'params_id': parent_build_params.id}) + config_step._run_step(parent_build)() + self.assertEqual(self.docker_run_calls[1][3]['memory'], 9 * 1024 ** 3) class TestMakeResult(RunbotCase): def setUp(self): From ec7560baea9eacab34eb1f60ec2500bbfaae4fee Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 19 Mar 2026 13:58:44 +0100 Subject: [PATCH 32/38] [IMP] runbot: show cached ADD line in a comment --- runbot/models/docker.py | 1 + runbot/tests/test_dockerfile.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/runbot/models/docker.py b/runbot/models/docker.py index 611435443..d7115e3d5 100644 --- a/runbot/models/docker.py +++ b/runbot/models/docker.py @@ -352,6 +352,7 @@ def _get_cached_content(self, docker_build_path): destination = add_match.group('destination') # Use the destination name as hardlink name to avoid rebuild if file content is the same but not the url hardlink_name = re.sub(r'[^a-zA-Z0-9]', '_', destination) + lines[i] = f'# CACHED {lines[i + 1]}' lines[i + 1] = f'COPY {hardlink_name} {destination}' cache_file_path = cache_dir / filename if not cache_file_path.exists() or time.time() - cache_file_path.lstat().st_mtime > cache_duration: diff --git a/runbot/tests/test_dockerfile.py b/runbot/tests/test_dockerfile.py index bdb412d27..b04ea6fb2 100644 --- a/runbot/tests/test_dockerfile.py +++ b/runbot/tests/test_dockerfile.py @@ -204,7 +204,7 @@ def test_dockerfile_get_cached_content(self): expected_content = """# CacheAddTest FROM ubuntu:noble -# CACHE 60 +# CACHED ADD https://nowhere.example.org/nothing.txt /data/nothing.txt COPY _data_nothing_txt /data/nothing.txt @@ -270,7 +270,7 @@ def test_dockerfile_build_with_cached_content(self): expected_content = """# Cache Test FROM ubuntu:noble -# CACHE 60 +# CACHED ADD https://nowhere.example.org/nothing.txt /data/nothing.txt COPY _data_nothing_txt /data/nothing.txt From 8638a2670269f66f04f740f55e6949a414cc76ba Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Wed, 6 May 2026 11:41:43 +0200 Subject: [PATCH 33/38] [IMP] runbot: update test tags version bounds on manual parse When parsing build errors, it may happen that the error appears in a version lower or higher than the current test tags bounds. With this commit, when a build error log is manually parsed (via the web interface or the server action), the min and max version bounds of the test tags are updated accordingly to include the new builds versions. --- runbot/controllers/frontend.py | 2 +- runbot/data/build_parse.xml | 2 +- runbot/models/build.py | 4 +- runbot/models/build_error.py | 18 ++++- runbot/tests/test_build_error.py | 114 +++++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 5 deletions(-) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 6735fe586..181a2c184 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -670,7 +670,7 @@ def access_running(self, build_id, db_suffix=None, **kwargs): @route(['/runbot/parse_log/'], type='http', auth='user', sitemap=False) def parse_log(self, ir_log, **kwargs): - request.env['runbot.build.error']._parse_logs(ir_log) + request.env['runbot.build.error']._parse_logs(ir_log, update_tags=True) return werkzeug.utils.redirect('/runbot/build/%s' % ir_log.build_id.id) @route(['/runbot/bundle//triggers/'], type='http', auth='user', sitemap=False) diff --git a/runbot/data/build_parse.xml b/runbot/data/build_parse.xml index a164c660d..dd2279edb 100644 --- a/runbot/data/build_parse.xml +++ b/runbot/data/build_parse.xml @@ -6,7 +6,7 @@ ir.actions.server code - action = records._parse_logs() + action = records._parse_logs(update_tags=True) diff --git a/runbot/models/build.py b/runbot/models/build.py index 8ba62d047..d092f289a 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1492,12 +1492,12 @@ def _get_py_version(self): return '3' return '' - def _parse_logs(self): + def _parse_logs(self, update_tags=False): """ Parse build logs to classify errors """ # only parse logs from builds in error and not already scanned builds_to_scan = self.filtered(lambda b: b.local_result in ('ko', 'killed', 'warn') and not b.build_error_link_ids) ir_logs = builds_to_scan.log_ids.filtered(lambda l: l.level in ('ERROR', 'WARNING', 'CRITICAL')) - return self.env['runbot.build.error']._parse_logs(ir_logs) + return self.env['runbot.build.error']._parse_logs(ir_logs, update_tags=update_tags) def _is_file(self, file, mode='r'): file_path = self._path(file) diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index 470e64774..2d07236fd 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -549,6 +549,17 @@ def _onchange_test_tags(self): self.tags_min_version_id = min(self.version_ids, key=lambda rec: rec.number) self.tags_max_version_id = max(self.version_ids, key=lambda rec: rec.number) + def _update_version_tags(self): + for error in self: + if not (error.test_tags and error.version_ids): + continue + new_min = min(error.version_ids, key=lambda rec: rec.number) + new_max = max(error.version_ids, key=lambda rec: rec.number) + if not error.tags_min_version_id or new_min.number < error.tags_min_version_id.number: + error.tags_min_version_id = new_min + if not error.tags_max_version_id or new_max.number > error.tags_max_version_id.number: + error.tags_max_version_id = new_max + @api.onchange('customer') def _onchange_customer(self): if not self.responsible: @@ -789,7 +800,7 @@ def action_copy_canonical_tag(self): record._onchange_test_tags() @api.model - def _parse_logs(self, ir_logs): + def _parse_logs(self, ir_logs, update_tags=False): if not ir_logs: return None regexes = self.env['runbot.error.regex'].search([]) @@ -845,6 +856,11 @@ def _parse_logs(self, ir_logs): 'log_date': rec.create_date, }) + if update_tags: + errors_to_update = build_error_contents.error_id.filtered('test_tags') + for error in errors_to_update: + error._update_version_tags() + if build_error_contents: window_action = { "type": "ir.actions.act_window", diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py index 0217e3182..8d4a27ec6 100644 --- a/runbot/tests/test_build_error.py +++ b/runbot/tests/test_build_error.py @@ -257,6 +257,120 @@ def test_duplicate_breaking_pr(self): error_a.breaking_pr_id = False self.assertEqual(error_a.duplicate_breaking_pr_count, 0) + def test_onchange_test_tags(self): + error = self.BuildError.create({}) + error_content = self.BuildErrorContent.create({'content': 'very bad trip', 'error_id': error.id}) + build_13 = self.create_test_build({'local_result': 'ko'}) + self.BuildErrorLink.create({ + 'build_id': build_13.id, + 'error_content_id': error_content.id, + }) + self.assertEqual(error.tags_min_version_id.id, False) + self.assertEqual(error.tags_max_version_id.id, False) + + error.test_tags = '/account' + error._onchange_test_tags() + self.assertEqual(error.tags_min_version_id, self.version_13) + self.assertEqual(error.tags_max_version_id, self.version_13) + + version_14 = self.Version._get('14.0') + params_14 = self.create_params({'version_id': version_14.id}) + build_14 = self.create_test_build({'local_result': 'ko', 'params_id': params_14.id}) + self.BuildErrorLink.create({ + 'build_id': build_14.id, + 'error_content_id': error_content.id, + }) + error._onchange_test_tags() + self.assertEqual(error.tags_min_version_id, self.version_13) + self.assertEqual(error.tags_max_version_id, version_14) + + def test_parse_logs_updates_version_bounds(self): + build_13 = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) + log_data = { + 'name': 'test-parse-logs', + 'type': 'server', + 'path': 'runbot', + 'level': 'ERROR', + 'func': 'test_trip', + 'line': 242, + 'message': 'Error Very Bad Trip', + 'build_id': build_13.id, + } + log_13 = self.IrLog.create(log_data) + + action = self.BuildError._parse_logs(log_13, update_tags=True) + self.assertEqual(action.get('type'), 'ir.actions.act_window') + error_content = self.BuildErrorContent.browse(action.get('res_id')) + error = error_content.error_id + error.test_tags = '/account' + error._update_version_tags() + self.assertEqual(error.tags_min_version_id, self.version_13) + self.assertEqual(error.tags_max_version_id, self.version_13) + + # Scanning a newer build extends the max without updating the min + version_14 = self.Version._get('14.0') + params_14 = self.create_params({'version_id': version_14.id}) + build_14 = self.create_test_build({'local_result': 'ko', 'local_state': 'done', 'params_id': params_14.id}) + log_data.update({'build_id': build_14.id}) + log_14 = self.IrLog.create(log_data) + self.BuildError._parse_logs(log_14, update_tags=True) + self.assertIn(build_14, error.build_ids) + self.assertEqual(error.tags_min_version_id, self.version_13) + self.assertEqual(error.tags_max_version_id, version_14) + + def test_parse_logs_no_update_without_flag(self): + build_13 = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) + log_13 = self.IrLog.create({ + 'name': 'test-parse-logs', + 'type': 'server', + 'path': 'runbot', + 'level': 'ERROR', + 'func': 'test_trip', + 'line': 242, + 'message': 'Error Very Bad Trip', + 'build_id': build_13.id, + }) + action = self.BuildError._parse_logs(log_13) + self.assertEqual(action.get('type'), 'ir.actions.act_window') + error_content = self.BuildErrorContent.browse(action.get('res_id')) + error = error_content.error_id + error.test_tags = '/discuss' + self.assertFalse(error.tags_min_version_id) + self.assertFalse(error.tags_max_version_id) + + def test_update_version_tags_no_update_inside_bounds(self): + version_14 = self.Version._get('14.0') + version_16 = self.Version._get('16.0') + params_14 = self.create_params({'version_id': version_14.id}) + params_16 = self.create_params({'version_id': version_16.id}) + build_14 = self.create_test_build({'local_result': 'ko', 'params_id': params_14.id}) + build_16 = self.create_test_build({'local_result': 'ko', 'params_id': params_16.id}) + + error = self.BuildError.create({}) + error_content = self.BuildErrorContent.create({'content': 'inside bounds test', 'error_id': error.id}) + link_14 = self.BuildErrorLink.create({'build_id': build_14.id, 'error_content_id': error_content.id}) + link_16 = self.BuildErrorLink.create({'build_id': build_16.id, 'error_content_id': error_content.id}) + + error.test_tags = '/account' + error._update_version_tags() + self.assertEqual(error.tags_min_version_id, version_14) + self.assertEqual(error.tags_max_version_id, version_16) + + link_14.unlink() + self.assertNotIn(version_14, error.version_ids) + # There is only version 16.0 on the error now but if we update the tags, it should leave the min at 14.0 + error._update_version_tags() + self.assertEqual(error.tags_min_version_id, version_14) + self.assertEqual(error.tags_max_version_id, version_16) + + link_14 = self.BuildErrorLink.create({'build_id': build_14.id, 'error_content_id': error_content.id}) + link_16.unlink() + self.assertNotIn(version_16, error.version_ids) + # Now there is only version 14.0 on the error but if we update the tags, it should leave the max at 16.0 + error._update_version_tags() + self.assertEqual(error.tags_min_version_id, version_14) + self.assertEqual(error.tags_max_version_id, version_16) + def test_relink_contents(self): build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) error_content_a = self.BuildErrorContent.create({'content': 'foo bar'}) From 0df68d7165792ec450a63b0b890b05ceb9985668 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Tue, 9 Dec 2025 09:48:23 +0100 Subject: [PATCH 34/38] [IMP] runbot: add authors and teams on bundles With this commit, the authors involved in a bundle are computed. Authors are found based on github logins first, then from the ngram extracted from the bundle name. The teams are infered from the authors found. Finally an `Owning team` is automatically choosen (the first team of the teams) or can be manually set. --- runbot/models/bundle.py | 88 +++++++++++++++++++++++++++++------ runbot/tests/test_branch.py | 41 ++++++++++++---- runbot/views/bundle_views.xml | 6 ++- 3 files changed, 110 insertions(+), 25 deletions(-) diff --git a/runbot/models/bundle.py b/runbot/models/bundle.py index 3e8073e89..5d616b32a 100644 --- a/runbot/models/bundle.py +++ b/runbot/models/bundle.py @@ -1,8 +1,14 @@ import datetime import re - from collections import defaultdict -from odoo import models, fields, api, tools +from itertools import chain + +from odoo import api, fields, models, tools +from odoo.fields import Domain + + +VALID_BUNDLE_NAME_RE = re.compile(r'^.{3,6}-.*-.{2,5}$') +NGRAM_RE = re.compile(r'.+\(([a-z]{2,5})\)$') class Bundle(models.Model): @@ -55,7 +61,11 @@ class Bundle(models.Model): # extra_info description = fields.Char('Description', compute='_compute_description', store=True, readonly=False) tag_ids = fields.Many2many('runbot.bundle.tag', string='Tags') - team_id = fields.Many2one('runbot.team', compute='_compute_team_id', store=True, readonly=False) + author_ids = fields.Many2many('res.users', string='Involved Users', compute='_compute_author_ids', domain=[('share', '=', False)]) + team_ids = fields.Many2many('runbot.team', string='Involved Teams', compute='_compute_team_ids') + team_id = fields.Many2one('runbot.team', string='Owning Team', compute='_compute_team_id', inverse='_inverse_team_id', store=True, tracking=True) + manual_team_id = fields.Many2one('runbot.team', 'Manually set team') + auto_team_id = fields.Many2one('runbot.team', 'Automatically set team', compute='_compute_auto_team_id', readonly=True) priority_offset = fields.Integer("Priority offset", help="Offset in seconds to remove from the create date of a batch to define priority, positive value means higher priority, negative value means lower priority.") @@ -201,19 +211,68 @@ def _compute_all_trigger_custom_ids(self): parent_bundle = self.env['runbot.bundle'].search([('name', '=', targets.pop())]) bundle.all_trigger_custom_ids = parent_bundle.all_trigger_custom_ids - @api.depends('name') + @api.depends('name', 'branch_ids.pr_author', 'branch_ids.forwardport_of_id', 'branch_ids.forwardport_of_id.pr_author', 'branch_ids.is_pr') + def _compute_author_ids(self): + self.author_ids = self.env['res.users'].browse() + bundles = self.filtered(lambda b: not b.is_base and not b.is_staging) + + github_logins_by_bundle = {} + ngram_by_bundle = {} + for bundle in bundles: + github_logins = [] + for pr in bundle.branch_ids.filtered('is_pr'): + author = (pr.forwardport_of_id.pr_author if pr.forwardport_of_id else pr.pr_author) + if author not in github_logins: + github_logins.append(author) + github_logins_by_bundle[bundle] = github_logins + if VALID_BUNDLE_NAME_RE.match(bundle.name): + ngram = bundle.name.split('-')[-1].lower() + ngram_by_bundle[bundle] = ngram + + user_domains = [[('github_login', '=', author)] for author in set(chain.from_iterable(github_logins_by_bundle.values()))] + user_domains += [[('name', 'ilike', f'% ({ngram})')] for ngram in set(ngram_by_bundle.values())] + + user_domain = Domain.OR(user_domains) + user_domain = Domain.AND([user_domain, [('share', '=', False)]]) + users = self.env['res.users'].search(user_domain) + user_ids_by_github_login = {u.github_login: u.id for u in users if u.github_login} + user_ids_by_ngram = {} + for user in users: + ngrams = NGRAM_RE.findall(user.complete_name or '') + if ngrams: + user_ids_by_ngram[ngrams[0]] = user.id + + for bundle in bundles: + user_ids = [] + for github_logins in github_logins_by_bundle[bundle]: + user_id = user_ids_by_github_login.get(github_logins) + if user_id: + user_ids.append(user_id) + ngram = ngram_by_bundle.get(bundle) + if ngram: + user_id = user_ids_by_ngram.get(ngram) + if user_id: + user_ids.append(user_id) + + bundle.author_ids = user_ids + + @api.depends('author_ids') + def _compute_team_ids(self): + for bundle in self: + bundle.team_ids = bundle.author_ids.runbot_team_ids.filtered(lambda rec: rec.module_ownership_ids).sorted('id') + + @api.depends('manual_team_id', 'auto_team_id') def _compute_team_id(self): - ngram_re = re.compile(r'.+\((?P[a-z]{2,4})\)$') - team_by_ngram_project = dict() - for team in self.env['runbot.team'].search([('module_ownership_ids', '!=', False)]): - for user in team.user_ids: - if m := ngram_re.match(user.name.lower()): - team_by_ngram_project[m.group('ngram'), team.project_id] = team for bundle in self: - if bundle.is_base or not bundle.name: - continue - bundle_ngram = bundle.name.split('-')[-1].lower() - bundle.team_id = team_by_ngram_project.get((bundle_ngram, bundle.project_id)) + bundle.team_id = bundle.manual_team_id or bundle.auto_team_id + + @api.depends('name', 'team_ids', 'author_ids') + def _compute_auto_team_id(self): + for bundle in self: + bundle.auto_team_id = bundle.team_ids and bundle.team_ids[0] + + def _inverse_team_id(self): + self.manual_team_id = self.team_id @api.depends('branch_ids') def _compute_description(self): @@ -366,6 +425,7 @@ class BundleTag(models.Model): _name = "runbot.bundle.tag" _description = "Bundle tag" + _order = "id desc, name" name = fields.Char(string='Bundle Tag') bundle_ids = fields.Many2many('runbot.bundle', string='Bundles') diff --git a/runbot/tests/test_branch.py b/runbot/tests/test_branch.py index 304ce0d98..fc4880034 100644 --- a/runbot/tests/test_branch.py +++ b/runbot/tests/test_branch.py @@ -152,9 +152,9 @@ def test_relations_no_match(self): self.assertEqual(b.bundle_id.base_id.name, 'master') def test_relations_pr(self): - self.Branch.create({ + dev_branch = self.Branch.create({ 'remote_id': self.remote_odoo_dev.id, - 'name': 'master-test-tri', + 'name': 'master-test-tri-imp', 'is_pr': False, }) @@ -167,17 +167,19 @@ def test_relations_pr(self): 'login': 'Pr author' }, } - b = self.Branch.create({ + pr_branch = self.Branch.create({ 'remote_id': self.remote_odoo_dev.id, 'name': '100', 'is_pr': True, }) - self.assertEqual(b.bundle_id.name, 'master-test-tri-imp') - self.assertEqual(b.bundle_id.base_id.name, 'master') - self.assertEqual(b.bundle_id.previous_major_version_base_id.name, '13.0') - self.assertEqual(sorted(b.bundle_id.intermediate_version_base_ids.mapped('name')), ['saas-13.1', 'saas-13.2']) + bundle = pr_branch.bundle_id + self.assertEqual(bundle.name, 'master-test-tri-imp') + self.assertEqual(bundle.base_id.name, 'master') + self.assertEqual(bundle.previous_major_version_base_id.name, '13.0') + self.assertEqual(sorted(bundle.intermediate_version_base_ids.mapped('name')), ['saas-13.1', 'saas-13.2']) + self.assertIn(dev_branch, bundle.branch_ids) class TestBranchForbidden(RunbotCase): """Test that a branch matching the repo forbidden regex, goes to dummy bundle""" @@ -309,14 +311,16 @@ def test_bundle_team_attribution(self): self.stop_patcher('isfile') self.stop_patcher('isdir') # needed to create the user avatar create_context = {'no_reset_password': True, 'mail_create_nolog': True, 'mail_create_nosubscribe': True, 'mail_notrack': True} - test_user = new_test_user(self.env, login='testrunbot', name='testrunbot (tru)', context=create_context) + committer_user = new_test_user(self.env, login='testrunbot', name='testrunbot (tru)', email='trut@somewhere.com', context=create_context) + github_user = new_test_user(self.env, login='github_author', name='github author (gaut)', email='gaut@somewhere.com', context=create_context) + github_user.github_login = 'gaut_github' team = self.env['runbot.team'].create({ 'name': 'Test Team', 'project_id': self.project.id, }) - team.user_ids += test_user + team.user_ids += committer_user branch = self.Branch.create({ 'remote_id': self.remote_odoo_dev.id, @@ -332,6 +336,7 @@ def test_bundle_team_attribution(self): bundle = self.env['runbot.bundle'].search([('name', '=', branch.name)]) self.assertEqual(bundle.team_id, team) + self.assertEqual(bundle.author_ids, committer_user, 'The only involved author should be the one based on bundle ngram') # now test that a team can be manually set on a bundle other_team = self.env['runbot.team'].create({ @@ -341,3 +346,21 @@ def test_bundle_team_attribution(self): bundle.team_id = other_team self.assertEqual(bundle.team_id, other_team) + + self.patchers['github_patcher'].return_value = { + 'base': {'ref': 'saas-19.1'}, + 'head': {'label': 'dev:saas-19.1-test-tru', 'repo': {'full_name': 'dev/odoo'}}, + 'title': '[IMP] Title', + 'body': 'Body', + 'user': { + 'login': github_user.github_login, + }, + } + pr_branch = self.Branch.create({ + 'remote_id': self.remote_odoo_dev.id, + 'name': '100', + 'is_pr': True, + }) + + self.assertIn(pr_branch, bundle.branch_ids) + self.assertIn(github_user, bundle.author_ids) diff --git a/runbot/views/bundle_views.xml b/runbot/views/bundle_views.xml index ff76a3fb4..a8b4c8aae 100644 --- a/runbot/views/bundle_views.xml +++ b/runbot/views/bundle_views.xml @@ -55,12 +55,14 @@ + + + + - - From ba1b4e1923294267cbc4d2d27e46c6233135244f Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Wed, 18 Feb 2026 11:40:06 +0100 Subject: [PATCH 35/38] [IMP] runbot: limit the size of files to open --- runbot/common.py | 2 ++ runbot/models/build.py | 5 +++++ runbot/models/build_config.py | 5 +++++ runbot/models/build_stat_regex.py | 6 +++++- runbot/tests/test_build.py | 1 - runbot/tests/test_build_config_step.py | 10 ++++++++++ runbot/tests/test_build_stat.py | 1 + runbot/tests/test_upgrade.py | 1 + 8 files changed, 29 insertions(+), 2 deletions(-) diff --git a/runbot/common.py b/runbot/common.py index fe34c2d44..0740ce89e 100644 --- a/runbot/common.py +++ b/runbot/common.py @@ -19,6 +19,8 @@ from odoo.fields import Domain from odoo.tools.misc import DEFAULT_SERVER_DATETIME_FORMAT, file_open, html_escape, OrderedSet +DEFAULT_MAX_FILE_SIZE = 500 * 1024 * 1024 + _logger = logging.getLogger(__name__) dest_reg = re.compile(r'^\d{5,}-.+$') diff --git a/runbot/models/build.py b/runbot/models/build.py index d092f289a..a822f69c7 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -38,6 +38,7 @@ sanitize, tail, transactioncache, + DEFAULT_MAX_FILE_SIZE, ) from ..container import Command, docker_pull, docker_run, docker_state, docker_stop from ..fields import JsonDictField @@ -1505,6 +1506,10 @@ def _is_file(self, file, mode='r'): def _read_file(self, file, mode='r'): file_path = self._path(file) + max_log_file_size = int(self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_max_log_size', DEFAULT_MAX_FILE_SIZE)) + if os.path.getsize(file_path) > max_log_file_size: + self._log('readfile', f"File size exceeds {max_log_file_size} limit", level="ERROR") + return False try: with file_open(file_path, mode) as f: return f.read() diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 500252ddf..af90073a8 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -26,6 +26,7 @@ rfind, s2human, time2str, + DEFAULT_MAX_FILE_SIZE, ) from ..container import Command, docker_get_gateway_ip @@ -1346,6 +1347,10 @@ def _check_log(self, build): if not os.path.isfile(log_path): build._log('_make_tests_results', "Log file not found at the end of test job", level="ERROR") return 'ko' + max_log_file_size = int(self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_max_log_size', DEFAULT_MAX_FILE_SIZE)) + if os.path.getsize(log_path) > max_log_file_size: + build._log('_make_tests_results', f"Log file exceeds {max_log_file_size} limit", level="ERROR") + return 'ko' return 'ok' def _check_module_loaded(self, build): diff --git a/runbot/models/build_stat_regex.py b/runbot/models/build_stat_regex.py index e845cbad7..9b6b8747c 100644 --- a/runbot/models/build_stat_regex.py +++ b/runbot/models/build_stat_regex.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import logging -from ..common import os +from ..common import os, DEFAULT_MAX_FILE_SIZE import re from odoo import models, fields, api @@ -53,6 +53,10 @@ def _find_in_file(self, file_path): """ if not os.path.exists(file_path): return {} + max_log_file_size = int(self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_max_log_size', DEFAULT_MAX_FILE_SIZE)) + if os.path.getsize(file_path) > max_log_file_size: + _logger.warning("Log file '%s' exceeds %s limit", file_path, max_log_file_size) + return {} stats_matches = {} with file_open(file_path, "r") as log_file: data = log_file.read() diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 2cf84f65e..03e0cd831 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -7,7 +7,6 @@ from odoo.tests import tagged from odoo.exceptions import UserError, ValidationError from .common import RunbotCase, RunbotCaseMinimalSetup -from unittest.mock import MagicMock def rev_parse(repo, branch_name): diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 1e66d26fc..a91044f0a 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -1451,6 +1451,7 @@ def _log(build, func, message, level='INFO', log_type='runbot', path='runbot'): self.logs.append((level, message)) self.start_patcher('log_patcher', 'odoo.addons.runbot.models.build.BuildResult._log', new=_log) + self.start_patcher('get_size', 'odoo.addons.runbot.models.build_config.os.path.getsize', return_value=100) self.build = self.Build.create({ 'params_id': self.base_params.id, @@ -1772,3 +1773,12 @@ def test_check_exit_status_file_not_found(self): config_step._make_results(self.build) self.assertEqual(self.build.local_result, 'ko') self.assertIn(('ERROR', 'Exit status file "test_exit_status.txt" not found'), self.logs) + + def test_make_result_large_file(self): + custom_limit = 1024 + self.env['ir.config_parameter'].sudo().set_param('runbot.runbot_max_log_size', custom_limit) + self.patchers['get_size'].return_value = custom_limit + 1 + self.config_step._make_results(self.build) + self.assertEqual(str(self.build.job_end), '1970-01-01 02:00:00') + self.assertIn(('ERROR', 'Log file exceeds %s limit' % custom_limit), self.logs) + self.assertEqual(self.build.local_result, 'ko') diff --git a/runbot/tests/test_build_stat.py b/runbot/tests/test_build_stat.py index 6f8282585..5a5e616a7 100644 --- a/runbot/tests/test_build_stat.py +++ b/runbot/tests/test_build_stat.py @@ -9,6 +9,7 @@ class TestBuildStatRegex(RunbotCase): def setUp(self): super(TestBuildStatRegex, self).setUp() + self.start_patcher('get_size', 'odoo.addons.runbot.models.build_config.os.path.getsize', return_value=100) self.StatRegex = self.env["runbot.build.stat.regex"] self.ConfigStep = self.env["runbot.build.config.step"] self.BuildStat = self.env["runbot.build.stat"] diff --git a/runbot/tests/test_upgrade.py b/runbot/tests/test_upgrade.py index dbd7bb48d..e52ef5a8d 100644 --- a/runbot/tests/test_upgrade.py +++ b/runbot/tests/test_upgrade.py @@ -19,6 +19,7 @@ def setUp(self): def upgrade_flow_setup(self): self.start_patcher('find_patcher', 'odoo.addons.runbot.common.find', 0) + self.start_patcher('get_size', 'odoo.addons.runbot.models.build_config.os.path.getsize', return_value=100) self.additionnal_setup() self.master_bundle = self.branch_odoo.bundle_id From ffde0c7ab00750a111b7615e37716465475ad32e Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Mon, 1 Jun 2026 10:38:12 +0200 Subject: [PATCH 36/38] [FIX] runbot: add missing int cast --- runbot/models/build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index a822f69c7..ba08d8ee0 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1044,7 +1044,7 @@ def _docker_run(self, step, cmd=None, ro_volumes=None, env_variables=None, **kwa if containers_memory_limit and 'memory' not in kwargs: memory_limit_factor = float(self.params_id.config_data.get('memory_limit_factor', 1)) containers_memory_limit = int(float(containers_memory_limit) * 1024 ** 3) * memory_limit_factor - kwargs['memory'] = containers_memory_limit + kwargs['memory'] = int(containers_memory_limit) self.docker_start = now() if self.job_start: From 9beccf5e8f9a601f9f85065a29c27d614da92917 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 4 Jun 2026 14:19:35 +0200 Subject: [PATCH 37/38] [IMP] runbot: revamp the freeze page as a grouped table This commit improves the readability and usability of the freeze page : - use a more structured grouped table paradigm to provide more cohesive alignment and readability. - add collapse/expand teams' PRs. - add per team counters - add collapse/expand all - add tags/"done" filtering To support those changes, the TableGroup and TableFilter js components have been added, providing a generic addition on top of Bootstrap's classes. --- runbot/__manifest__.py | 3 + runbot/controllers/frontend.py | 7 +-- runbot/static/src/css/table_group.css | 28 +++++++++ runbot/static/src/js/table_filter.js | 36 +++++++++++ runbot/static/src/js/table_group.js | 59 +++++++++++++++++ runbot/templates/bundles_by_tag.xml | 91 +++++++++++++++++++-------- 6 files changed, 195 insertions(+), 29 deletions(-) create mode 100644 runbot/static/src/css/table_group.css create mode 100644 runbot/static/src/js/table_filter.js create mode 100644 runbot/static/src/js/table_group.js diff --git a/runbot/__manifest__.py b/runbot/__manifest__.py index b3589f8dc..c6923b221 100644 --- a/runbot/__manifest__.py +++ b/runbot/__manifest__.py @@ -80,11 +80,14 @@ 'web/static/lib/odoo_ui_icons/style.css', 'runbot/static/lib/bootstrap/css/bootstrap.css', 'runbot/static/lib/fontawesome/css/font-awesome.css', + 'runbot/static/src/css/table_group.css', 'runbot/static/src/css/runbot.css', 'runbot/static/src/js/polyfill_command_api.js', 'runbot/static/lib/jquery/jquery.js', 'runbot/static/lib/bootstrap/js/bootstrap.bundle.js', + 'runbot/static/src/js/table_filter.js', + 'runbot/static/src/js/table_group.js', 'runbot/static/src/js/runbot.js', ], }, diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 181a2c184..23e79eddd 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -908,19 +908,18 @@ def bundles_by_tag(self, bundle_tag_id=None, project=None, **kwargs): if not project and projects: project = projects[0] bundles_by_team = defaultdict(list) - nb_bundles = 0 nb_bundles_done = 0 - for bundle in self.env['runbot.bundle'].search([('tag_ids', 'in', bundle_tag_id.id)]): + bundles = self.env['runbot.bundle'].search([('tag_ids', 'in', bundle_tag_id.id)]) + for bundle in bundles: bundles_by_team[bundle.team_id.name or 'No Team Defined'].append(bundle) - nb_bundles += 1 bundle_prs = bundle.branch_ids.filtered(lambda rec: rec.is_pr) if any(bundle_prs) and not any(bundle_prs.mapped('alive')): nb_bundles_done += 1 qctx = { 'tag': bundle_tag_id, + 'bundles': bundles, 'bundles_by_team': bundles_by_team, - 'nb_bundles': nb_bundles, 'nb_bundles_done': nb_bundles_done, } return request.render('runbot.bundles_by_tag', qctx) diff --git a/runbot/static/src/css/table_group.css b/runbot/static/src/css/table_group.css new file mode 100644 index 000000000..545816125 --- /dev/null +++ b/runbot/static/src/css/table_group.css @@ -0,0 +1,28 @@ +.table-group-divider { + > tr > th { + cursor: pointer; + + &::before { + content: "\25BC"; + display: inline-block; + width: 1rem; + } + } +} + +.table-group-hidden { + > tr > th { + &::before { + content: "\25B6"; + font-size: x-small + } + } + + > tr:not(:has(> th)) { + display: none; + } +} + +.w-0 { + width: 0; +} diff --git a/runbot/static/src/js/table_filter.js b/runbot/static/src/js/table_filter.js new file mode 100644 index 000000000..2c6706b82 --- /dev/null +++ b/runbot/static/src/js/table_filter.js @@ -0,0 +1,36 @@ +// @odoo-module ignore + +class TableFilter { + static selector = ".table-filter"; + static filterRowSelector = "[data-toggle='filter-row']"; + + constructor(el) { + this.el = el; + for (const filter of this.filters) { + this.onFilter(filter); + filter.addEventListener("change", () => this.onFilter(filter)); + } + } + + get filters() { + return [...this.el.querySelectorAll(this.constructor.filterRowSelector)]; + } + + get rows() { + return [...this.el.querySelectorAll("tbody > tr:not(:has(th))")]; + } + + onFilter(filter) { + const [key, val] = filter.dataset.filter.split("=="); + const filteredRows = this.rows.filter((r) => r.matches([`tr:has([data-${key}^="${val}"])`])); + for (const row of filteredRows) { + row.classList.toggle("d-none", !filter.checked); + } + } +} + +document.addEventListener("DOMContentLoaded", () => { + for (const table of [...document.querySelectorAll(TableFilter.selector)]){ + new TableFilter(table); + } +}); diff --git a/runbot/static/src/js/table_group.js b/runbot/static/src/js/table_group.js new file mode 100644 index 000000000..fac9d619d --- /dev/null +++ b/runbot/static/src/js/table_group.js @@ -0,0 +1,59 @@ +// @odoo-module ignore + +class TableGroup { + static selector = ".table-group"; + static groupSelector = ".table-group-divider"; + static groupHeaderSelector = "tr:has(> th)"; + static groupRowSelector = "tr:not(:has(> th))"; + static collapseGroupsSelector = "[data-toggle='table-group-collapse']"; + static hiddenGroupClass = "table-group-hidden"; + + constructor(el) { + this.el = el; + this.expanded = !this.isAllCollapsed; + for (const group of this.groups) { + const header = this.groupHeader(group); + header.querySelector("th").addEventListener("click", () => this.toggleGroup(group)); + } + this.toggleCollapseText(); + this.collapseButton.addEventListener("click", () => this.toggleCollapse()); + } + + get groups() { + return [...this.el.querySelectorAll(this.constructor.groupSelector)]; + } + + get collapseButton() { + return this.el.querySelector(this.constructor.collapseGroupsSelector); + } + + get isAllCollapsed() { + return this.groups.every((group) => group.classList.contains(this.constructor.hiddenGroupClass)); + } + + groupHeader(group) { + return group.querySelector(this.constructor.groupHeaderSelector); + } + + toggleGroup(group, force = false) { + group.classList.toggle(this.constructor.hiddenGroupClass, force ? true : undefined); + } + + toggleCollapse() { + this.expanded = !this.expanded; + for (const group of this.groups) { + this.toggleGroup(group, !this.expanded); + } + this.toggleCollapseText(); + } + + toggleCollapseText() { + this.collapseButton.textContent = this.expanded ? "Collapse all" : "Expand all"; + } +} + +document.addEventListener("DOMContentLoaded", () => { + for (const table of [...document.querySelectorAll(TableGroup.selector)]){ + new TableGroup(table); + } +}); diff --git a/runbot/templates/bundles_by_tag.xml b/runbot/templates/bundles_by_tag.xml index e7a7834d9..7dc48d2dc 100644 --- a/runbot/templates/bundles_by_tag.xml +++ b/runbot/templates/bundles_by_tag.xml @@ -2,6 +2,7 @@ From a8b02c05a232ce0ed22eb8fbc8157808d525a474 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Fri, 5 Jun 2026 14:26:33 +0200 Subject: [PATCH 38/38] [IMP] runbot: cleanup build errors frontend page --- runbot/controllers/frontend.py | 1 + runbot/templates/build_error.xml | 152 ++++++++++++++++--------------- 2 files changed, 82 insertions(+), 71 deletions(-) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 23e79eddd..3b049d4bc 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -481,6 +481,7 @@ def build_errors(self, sort=None, page=1, limit=20, **kwargs): 'build_errors': build_errors, 'title': 'Build Errors', 'sort_order_choices': sort_order_choices, + 'sort_order': sort_order, 'page': page, 'pager': pager, } diff --git a/runbot/templates/build_error.xml b/runbot/templates/build_error.xml index 87ad07804..399f2c4b0 100644 --- a/runbot/templates/build_error.xml +++ b/runbot/templates/build_error.xml @@ -2,57 +2,70 @@
    DateDate (UTC) LevelType Message
    - -
    - + - + + + + + + + + - - - - Build # - - - - - - - - - - - - - : - + + + Build # + + + - - - - - - - - - - - - - - - - - - - - - -
    -
    + + +
    - + @@ -351,17 +321,16 @@ -
    +
    - This error is already . - - - - - () + + This error is already . + + + + () +