[FIX] runbot_merge: attempt to stage PRs on disabled branches

Normally opening a PR against a disabled branch is like opening a PR
against a branch which is not configured at all: the PR id ignored
entirely.

However if the PR already exists then the state of the branch isn't
normally checked when interacting with the branch, and it is possible
to trigger its staging, at which point the staging itself will crash:
on a project the branches are `active_test=False` so they're all
visible in the form, but when repos go search()-ing for the branch
they won't find it and will blow up.

Solution: only try staging on branches which are active. Fixes
odoo/runbot#408. Also do the same for checking stagings.

And while at it, fix #409 by wrapping each checking or staging into a
try/except and a savepoint. This way if a staging blows up it should
move on to the next branch instead of getting stuck.
This commit is contained in:
Xavier Morel 2020-10-01 17:37:07 +02:00
parent c93ce7c2c2
commit 2fafd5419a
2 changed files with 66 additions and 9 deletions

View File

@ -66,18 +66,32 @@ class Project(models.Model):
)
def _check_stagings(self, commit=False):
for staging in self.search([]).mapped('branch_ids.active_staging_id'):
staging.check_status()
if commit:
self.env.cr.commit()
def _create_stagings(self, commit=False):
for branch in self.search([]).mapped('branch_ids'):
if not branch.active_staging_id:
branch.try_staging()
for branch in self.search([]).mapped('branch_ids').filtered('active'):
staging = branch.active_staging_id
if not staging:
continue
try:
with self.env.cr.savepoint():
staging.check_status()
except Exception:
_logger.exception("Failed to check staging for branch %r (staging %s)",
branch.name, staging)
else:
if commit:
self.env.cr.commit()
def _create_stagings(self, commit=False):
for branch in self.search([]).mapped('branch_ids').filtered('active'):
if not branch.active_staging_id:
try:
with self.env.cr.savepoint():
branch.try_staging()
except Exception:
_logger.exception("Failed to create staging for branch %r", branch.name)
else:
if commit:
self.env.cr.commit()
def _send_feedback(self):
Repos = self.env['runbot_merge.repository']
ghs = {}

View File

@ -88,3 +88,46 @@ def test_override(env, project, make_repo, users, setreviewers, config):
'target_url': comments[-1]['html_url'],
'description': 'Overridden by @{}'.format(users['other']),
}}
def test_disabled_branch(env, project, make_repo, users, setreviewers, config):
""" PRs to disabled branches are ignored, but what if the PR exists *before*
the branch is disabled?
"""
repo = make_repo('repo')
project.branch_ids.sequence = 0
project.write({'branch_ids': [
(0, 0, {'name': 'other', 'sequence': 1}),
(0, 0, {'name': 'other2', 'sequence': 2}),
]})
repo_id = env['runbot_merge.repository'].create({
'project_id': project.id,
'name': repo.name,
'status_ids': [(0, 0, {'context': 'status'})]
})
setreviewers(*project.repo_ids)
with repo:
[m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')
[ot] = repo.make_commits(m, Commit('other', tree={'b': '1'}), ref='heads/other')
repo.make_commits(m, Commit('other2', tree={'c': '1'}), ref='heads/other2')
[c] = repo.make_commits(ot, Commit('wheee', tree={'b': '2'}))
pr = repo.make_pr(title="title", body='body', target='other', head=c)
repo.post_status(c, 'success', 'status')
pr_id = env['runbot_merge.pull_requests'].search([
('repository', '=', repo_id.id),
('number', '=', pr.number),
])
# disable branch "other"
project.branch_ids.filtered(lambda b: b.name == 'other').active = False
# r+ the PR
with repo:
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
# nothing should happen, the PR should be unstaged forever, maybe?
assert pr_id.state == 'ready'
assert not pr_id.staging_id