[IMP] runbot_merge: map PR commits to integrated commits

* when rebasing, store a map of rebased to source, that way it'll be
  possible to link cherry-picked forward ports to the originally
  integrated commit rather than just the one from the PR (which was
  likely not itself integrated as the straight merge mode is somewhat
  rare: as of 5600 PRs merged so far only 100 were straight merged)
* while at it, store the "merge head" of the PR (whether squashed,
  merged or rebased) and put *that* in the commit message

fixes #161
This commit is contained in:
xmo-odoo 2019-07-31 09:19:39 +02:00 committed by GitHub
parent 955b97a023
commit 85ac2e5d5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 16 deletions

View File

@ -172,7 +172,7 @@ class GH(object):
""" Rebase pr's commits on top of dest, updates dest unless ``reset``
is set.
Returns the hash of the rebased head.
Returns the hash of the rebased head and a map of all PR commits (to the PR they were rebased to)
"""
logger = _logger.getChild('rebase')
original_head = self.head(dest)
@ -189,6 +189,7 @@ class GH(object):
c['new_tree'] = self.merge(c['sha'], dest, tmp_msg)['tree']['sha']
prev = original_head
mapping = {}
for c in commits:
copy = self('post', 'git/commits', json={
'message': c['commit']['message'],
@ -198,7 +199,7 @@ class GH(object):
'committer': c['commit']['committer'],
}, check={409: exceptions.MergeError}).json()
logger.debug('copied %s to %s (parent: %s)', c['sha'], copy['sha'], prev)
prev = copy['sha']
prev = mapping[c['sha']] = copy['sha']
if reset:
self.set_ref(dest, original_head)
@ -209,7 +210,7 @@ class GH(object):
self._repo, pr, dest, reset, len(commits),
prev)
# prev is updated after each copy so it's the rebased PR head
return prev
return prev, mapping
# fetch various bits of issues / prs to load them
def pr(self, number):

View File

@ -494,6 +494,7 @@ class PullRequests(models.Model):
batch_id = fields.Many2one('runbot_merge.batch',compute='_compute_active_batch', store=True)
batch_ids = fields.Many2many('runbot_merge.batch')
staging_id = fields.Many2one(related='batch_id.staging_id', store=True)
commits_map = fields.Char(help="JSON-encoded mapping of PR commits to actually integrated commits. The integration head (either a merge commit or the PR's topmost) is mapped from the 'empty' pr commit (the key is an empty string, because you can't put a null key in json maps).", default='{}')
link_warned = fields.Boolean(
default=False, help="Whether we've already warned that this (ready)"
@ -961,12 +962,16 @@ class PullRequests(models.Model):
# on top of target
msg = self._build_merge_message(commits[-1]['commit']['message'])
commits[-1]['commit']['message'] = msg
return gh.rebase(self.number, target, commits=commits)
head, mapping = gh.rebase(self.number, target, commits=commits)
self.commits_map = json.dumps({**mapping, '': head})
return head
def _stage_rebase_merge(self, gh, target, commits):
msg = self._build_merge_message(self.message)
h = gh.rebase(self.number, target, reset=True, commits=commits)
return gh.merge(h, target, msg)['sha']
h, mapping = gh.rebase(self.number, target, reset=True, commits=commits)
merge_head = gh.merge(h, target, msg)['sha']
self.commits_map = json.dumps({**mapping, '': merge_head})
return merge_head
def _stage_merge(self, gh, target, commits):
pr_head = commits[-1] # oldest to newest
@ -981,6 +986,7 @@ class PullRequests(models.Model):
if len(merge) == 1:
[base_commit] = merge
commits_map = {c['sha']: c['sha'] for c in commits}
if base_commit:
# replicate pr_head with base_commit replaced by
# the current head
@ -996,11 +1002,18 @@ class PullRequests(models.Model):
'parents': new_parents,
}).json()
gh.set_ref(target, copy['sha'])
# merge commit *and old PR head* map to the pr head replica
commits_map[''] = commits_map[pr_head['sha']] = copy['sha']
self.commits_map = json.dumps(commits_map)
return copy['sha']
else:
# otherwise do a regular merge
msg = self._build_merge_message(self.message)
return gh.merge(self.head, target, msg)['sha']
merge_head = gh.merge(self.head, target, msg)['sha']
# and the merge commit is the normal merge head
commits_map[''] = merge_head
self.commits_map = json.dumps(commits_map)
return merge_head
# state changes on reviews
RPLUS = {
@ -1363,7 +1376,7 @@ class Stagings(models.Model):
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': pr.repository.id,
'pull_request': pr.number,
'message': "Merged, thanks!",
'message': "Merged at %s, thanks!" % json.loads(pr.commits_map)[''],
'close': True,
})
finally:

View File

@ -975,11 +975,16 @@ class TestMergeMethod:
repo.post_status(staging.id, 'success', 'legal/cla')
repo.post_status(staging.id, 'success', 'ci/runbot')
run_crons(env)
assert env['runbot_merge.pull_requests'].search([
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).state == 'merged'
])
assert pr.state == 'merged'
assert prx.state == 'closed'
assert json.loads(pr.commits_map) == {
c1: actual_sha,
'': actual_sha,
}, "for a squash, the one PR commit should be mapped to the one rebased commit"
def test_pr_update_to_many_commits(self, repo, env):
"""
@ -1157,16 +1162,26 @@ class TestMergeMethod:
repo.post_status('heads/staging.master', 'success', 'ci/runbot')
run_crons(env)
assert env['runbot_merge.pull_requests'].search([
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
]).state == 'merged'
])
assert pr.state == 'merged'
# check that the dummy commit is not in the final master
master = log_to_node(repo.log('heads/master'))
assert master == merge_head
final_tree = repo.read_tree(repo.commit('heads/master'))
head = repo.commit('heads/master')
final_tree = repo.read_tree(head)
assert final_tree == {'m': b'2', 'b': b'1'}, "sanity check of final tree"
r1 = repo.commit(head.parents[1])
r0 = repo.commit(r1.parents[0])
assert json.loads(pr.commits_map) == {
b0: r0.id,
b1: r1.id,
'': head.id,
}
assert r0.parents == [m2]
def test_pr_rebase_ff(self, repo, env, users):
""" test result on rebase-merge
@ -1220,17 +1235,28 @@ class TestMergeMethod:
repo.post_status('heads/staging.master', 'success', 'ci/runbot')
run_crons(env)
assert env['runbot_merge.pull_requests'].search([
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
]).state == 'merged'
])
assert pr.state == 'merged'
# check that the dummy commit is not in the final master
master = log_to_node(repo.log('heads/master'))
assert master == nb1
final_tree = repo.read_tree(repo.commit('heads/master'))
head = repo.commit('heads/master')
final_tree = repo.read_tree(head)
assert final_tree == {'m': b'2', 'b': b'1'}, "sanity check of final tree"
m1 = head
m0 = repo.commit(m1.parents[0])
assert json.loads(pr.commits_map) == {
'': m1.id, # merge commit
b1: m1.id, # second PR's commit
b0: m0.id, # first PR's commit
}
assert m0.parents == [m2], "can't hurt to check the parent of our root commit"
@pytest.mark.skip(reason="what do if the PR contains merge commits???")
def test_pr_contains_merges(self, repo, env):
pass
@ -1272,6 +1298,14 @@ class TestMergeMethod:
expected = node('gibberish\n\nblahblah\n\ncloses {}#{}'
'\n\nSigned-off-by: {}'.format(repo.name, prx.number, reviewer), m, c0)
assert log_to_node(repo.log('heads/master')), expected
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert json.loads(pr.commits_map) == {
prx.head: prx.head,
'': master.id
}
def test_unrebase_emptymessage(self, repo, env, users):
""" When merging between master branches (e.g. forward port), the PR