diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 71edb676..40c52093 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -642,36 +642,28 @@ class PullRequests(models.Model): for pr in self: owner, _ = pr.repository.fp_remote_target.split('/', 1) source = pr.source_id or pr - message = source.message - if message: - message += '\n\n' - else: - message = '' root = pr._get_root() - message += '\n'.join( + + message = source.message + '\n\n' + '\n'.join( "Forward-Port-Of: %s" % p.display_name for p in root | source ) title, body = re.match(r'(?P[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups() - title = '[FW]' + title - if not body: - body = None - self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE') - url = 'https://api.github.com/repos/{}/pulls'.format(pr.repository.name) - pr_data = { - 'base': target.name, 'head': '%s:%s' % (owner, new_branch), - 'title': title, 'body': body - } - r = gh.post(url, json=pr_data) + r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={ + 'base': target.name, + 'head': f'{owner}:{new_branch}', + 'title': '[FW]' + title, + 'body': body + }) if not r.ok: _logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name) # delete all the branches this should automatically close the # PRs if we've created any. Using the API here is probably # simpler than going through the working copies for repo in self.mapped('repository'): - d = gh.delete('https://api.github.com/repos/{}/git/refs/heads/{}'.format(repo.fp_remote_target, new_branch)) + d = gh.delete(f'https://api.github.com/repos/{repo.fp_remote_target}/git/refs/heads/{new_branch}') if d.ok: _logger.info("Deleting %s:%s=success", repo.fp_remote_target, new_branch) else: diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index a5e2a0e5..f765bee7 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -1418,6 +1418,11 @@ class PullRequests(models.Model): if gh_name: self.reviewed_by.name = gh_name + # update pr message in case an update was missed + msg = f'{prdict["title"]}\n\n{prdict.get("body") or ""}'.strip() + if self.message != msg: + self.message = msg + # NOTE: lost merge v merge/copy distinction (head being # a merge commit reused instead of being re-merged) return method, getattr(self, '_stage_' + method.replace('-', '_'))( diff --git a/runbot_merge/tests/test_oddities.py b/runbot_merge/tests/test_oddities.py index b2b2f984..5514563e 100644 --- a/runbot_merge/tests/test_oddities.py +++ b/runbot_merge/tests/test_oddities.py @@ -1,3 +1,6 @@ +from utils import Commit, to_pr + + def test_partner_merge(env): p_src = env['res.partner'].create({ 'name': 'kfhsf', @@ -60,3 +63,37 @@ def test_name_search(env): assert PRs.name_search('repo') == [pr2, pr0, pr1] assert PRs.name_search('repo#1959') == [pr1] + +def test_message_desync(env, project, make_repo, users, setreviewers, config): + """If the PR message gets desync'd (github misses sending an update), the + merge message should still match what's on github rather than what's in the + db + """ + repo = make_repo('repo') + 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') + + [c] = repo.make_commits('master', Commit('whee', tree={'b': '2'})) + pr = repo.make_pr(title='title', body='body', target='master', head=c) + repo.post_status(c, 'success', 'status') + env.run_crons() + + pr_id = to_pr(env, pr) + assert pr_id.message == 'title\n\nbody' + pr_id.message = "xxx" + + with repo: + pr.post_comment('hansen merge r+', config['role_reviewer']['token']) + env.run_crons() + + st = repo.commit('staging.master') + assert st.message.startswith('title\n\nbody'),\ + "the stored PR message should have been ignored when staging" + assert st.parents == [m, c], "check the staging's ancestry is the right one"