2025-02-06 20:33:40 +07:00
|
|
|
from utils import Commit, make_basic, to_pr, seen
|
2019-10-15 13:54:25 +07:00
|
|
|
|
|
|
|
|
|
|
|
def test_single_updated(env, config, make_repo):
|
|
|
|
""" Given co-dependent PRs getting merged, one of them being modified should
|
|
|
|
lead to a restart of the merge & forward port process.
|
|
|
|
|
|
|
|
See test_update_pr for a simpler (single-PR) version
|
|
|
|
"""
|
2025-02-06 20:33:40 +07:00
|
|
|
r1, _ = make_basic(env, config, make_repo, reponame='repo-1', statuses='default')
|
|
|
|
r2, _ = make_basic(env, config, make_repo, reponame='repo-2', statuses='default')
|
2019-10-15 13:54:25 +07:00
|
|
|
|
|
|
|
with r1:
|
|
|
|
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
|
|
|
|
pr1 = r1.make_pr(target='a', head='aref')
|
2025-02-06 20:33:40 +07:00
|
|
|
r1.post_status('aref', 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
with r2:
|
|
|
|
r2.make_commits('a', Commit('2', tree={'2': '0'}), ref='heads/aref')
|
|
|
|
pr2 = r2.make_pr(target='a', head='aref')
|
2025-02-06 20:33:40 +07:00
|
|
|
r2.post_status('aref', 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
with r1, r2:
|
2025-02-06 20:33:40 +07:00
|
|
|
r1.post_status('staging.a', 'success')
|
|
|
|
r2.post_status('staging.a', 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr1_id, pr11_id, pr2_id, pr21_id = pr_ids = env['runbot_merge.pull_requests'].search([]).sorted('display_name')
|
|
|
|
assert pr1_id.number == pr1.number
|
|
|
|
assert pr2_id.number == pr2.number
|
|
|
|
assert pr1_id.state == pr2_id.state == 'merged'
|
|
|
|
|
|
|
|
assert pr11_id.parent_id == pr1_id
|
|
|
|
assert pr11_id.repository.name == pr1_id.repository.name == r1.name
|
|
|
|
|
|
|
|
assert pr21_id.parent_id == pr2_id
|
|
|
|
assert pr21_id.repository.name == pr2_id.repository.name == r2.name
|
|
|
|
|
|
|
|
assert pr11_id.target.name == pr21_id.target.name == 'b'
|
|
|
|
|
|
|
|
# don't even bother faking CI failure, straight update pr21_id
|
|
|
|
repo, ref = r2.get_pr(pr21_id.number).branch
|
|
|
|
with repo:
|
|
|
|
repo.make_commits(
|
2025-02-06 18:35:55 +07:00
|
|
|
r2.commit(pr21_id.target.name).id,
|
2019-10-15 13:54:25 +07:00
|
|
|
Commit('Whops', tree={'2': '1'}),
|
|
|
|
ref='heads/' + ref,
|
|
|
|
make=False
|
|
|
|
)
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert not pr21_id.parent_id
|
|
|
|
|
|
|
|
with r1, r2:
|
2025-02-06 20:33:40 +07:00
|
|
|
r1.post_status(pr11_id.head, 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
r1.get_pr(pr11_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
|
2025-02-06 20:33:40 +07:00
|
|
|
r2.post_status(pr21_id.head, 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
r2.get_pr(pr21_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
prs_again = env['runbot_merge.pull_requests'].search([])
|
|
|
|
assert prs_again == pr_ids,\
|
|
|
|
"should not have created FP PRs as we're now in a detached (iso new PR) state " \
|
|
|
|
"(%s)" % prs_again.mapped('display_name')
|
|
|
|
|
|
|
|
with r1, r2:
|
2025-02-06 20:33:40 +07:00
|
|
|
r1.post_status('staging.b', 'success')
|
|
|
|
r2.post_status('staging.b', 'success')
|
2019-10-15 13:54:25 +07:00
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
new_prs = env['runbot_merge.pull_requests'].search([]).sorted('display_name') - pr_ids
|
|
|
|
assert len(new_prs) == 2, "should have created the new FP PRs"
|
|
|
|
pr12_id, pr22_id = new_prs
|
|
|
|
|
|
|
|
assert pr12_id.source_id == pr1_id
|
|
|
|
assert pr12_id.parent_id == pr11_id
|
|
|
|
|
|
|
|
assert pr22_id.source_id == pr2_id
|
2020-11-17 21:21:21 +07:00
|
|
|
assert pr22_id.parent_id == pr21_id
|
2024-02-20 17:24:35 +07:00
|
|
|
|
|
|
|
def test_closing_during_fp(env, config, make_repo, users):
|
|
|
|
""" Closing a PR after it's been ported once should not port it further, but
|
|
|
|
the rest of the batch should carry on
|
|
|
|
"""
|
2025-02-06 20:33:40 +07:00
|
|
|
r1, _ = make_basic(env, config, make_repo, statuses='default')
|
|
|
|
r2, _ = make_basic(env, config, make_repo, statuses='default')
|
2024-02-20 17:24:35 +07:00
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
|
|
|
|
pr1 = r1.make_pr(target='a', head='aref')
|
|
|
|
r1.post_status('aref', 'success')
|
|
|
|
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
|
|
|
|
r2.make_commits('a', Commit('2', tree={'2': '0'}), ref='heads/aref')
|
|
|
|
pr2 = r2.make_pr(target='a', head='aref')
|
|
|
|
r2.post_status('aref', 'success')
|
|
|
|
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.a', 'success')
|
|
|
|
r2.post_status('staging.a', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr1_id = to_pr(env, pr1)
|
|
|
|
[pr1_1_id] = pr1_id.forwardport_ids
|
|
|
|
pr2_id = to_pr(env, pr2)
|
|
|
|
[pr2_1_id] = pr2_id.forwardport_ids
|
|
|
|
|
|
|
|
with r1:
|
|
|
|
r1.get_pr(pr1_1_id.number).close(config['role_user']['token'])
|
|
|
|
|
|
|
|
with r2:
|
|
|
|
r2.post_status(pr2_1_id.head, 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert env['runbot_merge.pull_requests'].search_count([]) == 5,\
|
|
|
|
"only one of the forward ports should be ported"
|
|
|
|
assert not env['runbot_merge.pull_requests'].search([('parent_id', '=', pr1_1_id.id)]),\
|
|
|
|
"the closed PR should not be ported"
|
2024-04-03 17:08:04 +07:00
|
|
|
assert env['runbot_merge.pull_requests'].search([('source_id', '=', pr1_id.id)]) == pr1_1_id,\
|
|
|
|
"the closed PR should not be ported"
|
|
|
|
|
|
|
|
r1_b_head = r1.commit("b")
|
|
|
|
with r2:
|
|
|
|
r2.get_pr(pr2_1_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
assert not pr2_1_id.blocked
|
|
|
|
assert not pr2_1_id.batch_id.blocked
|
|
|
|
st = pr2_1_id.staging_id
|
|
|
|
assert st
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.b', 'success')
|
|
|
|
r2.post_status('staging.b', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
assert st.state == 'success'
|
|
|
|
|
|
|
|
assert r1_b_head.id == r1.commit("b").id, \
|
|
|
|
"r1:b's head should not have been touched"
|
2024-02-20 17:24:35 +07:00
|
|
|
|
|
|
|
def test_add_pr_during_fp(env, config, make_repo, users):
|
|
|
|
""" It should be possible to add new PRs to an FP batch
|
|
|
|
"""
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
r1, _ = make_basic(env, config, make_repo, statuses="default")
|
|
|
|
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
|
2024-02-20 17:24:35 +07:00
|
|
|
# needs a "d" branch
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
env['runbot_merge.project'].search([]).write({
|
2024-02-20 17:24:35 +07:00
|
|
|
'branch_ids': [(0, 0, {'name': 'd', 'sequence': 40})],
|
|
|
|
})
|
|
|
|
with r1, r2:
|
|
|
|
r1.make_ref("heads/d", r1.commit("c").id)
|
|
|
|
r2.make_ref("heads/d", r2.commit("c").id)
|
|
|
|
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
with r1:
|
2024-02-20 17:24:35 +07:00
|
|
|
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
|
|
|
|
pr1_a = r1.make_pr(target='a', head='aref')
|
|
|
|
r1.post_status('aref', 'success')
|
|
|
|
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.a', 'success')
|
|
|
|
r2.post_status('staging.a', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr1_a_id = to_pr(env, pr1_a)
|
|
|
|
[pr1_b_id] = pr1_a_id.forwardport_ids
|
|
|
|
|
|
|
|
with r2, fork2:
|
2025-02-06 18:35:55 +07:00
|
|
|
fork2.make_commits(r2.commit('b').id, Commit('2', tree={'2': '0'}), ref=f'heads/{pr1_b_id.refname}')
|
2024-02-20 17:24:35 +07:00
|
|
|
pr2_b = r2.make_pr(title="B", target='b', head=f'{fork2.owner}:{pr1_b_id.refname}')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr2_b_id = to_pr(env, pr2_b)
|
|
|
|
|
|
|
|
assert not pr1_b_id.staging_id
|
|
|
|
assert not pr2_b_id.staging_id
|
|
|
|
assert pr1_b_id.batch_id == pr2_b_id.batch_id
|
|
|
|
assert pr1_b_id.state == "opened",\
|
|
|
|
"implicit approval from forward port should have been canceled"
|
|
|
|
batch = pr2_b_id.batch_id
|
|
|
|
|
|
|
|
with r1:
|
|
|
|
r1.post_status(pr1_b_id.head, 'success')
|
|
|
|
r1.get_pr(pr1_b_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert batch.blocked
|
|
|
|
assert pr1_b_id.blocked
|
|
|
|
|
|
|
|
with r2:
|
|
|
|
r2.post_status(pr2_b.head, "success")
|
|
|
|
pr2_b.post_comment("hansen r+", config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert not batch.blocked
|
|
|
|
assert pr1_b_id.staging_id and pr1_b_id.staging_id == pr2_b_id.staging_id
|
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.b', 'success')
|
|
|
|
r2.post_status('staging.b', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
def find_child(pr):
|
|
|
|
return env['runbot_merge.pull_requests'].search([
|
|
|
|
('parent_id', '=', pr.id),
|
|
|
|
])
|
|
|
|
pr1_c_id = find_child(pr1_b_id)
|
|
|
|
assert pr1_c_id
|
|
|
|
pr2_c_id = find_child(pr2_b_id)
|
|
|
|
assert pr2_c_id
|
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status(pr1_c_id.head, 'success')
|
|
|
|
r2.post_status(pr2_c_id.head, 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert find_child(pr1_c_id)
|
|
|
|
assert find_child(pr2_c_id)
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
|
|
|
def test_add_to_forward_ported(env, config, make_repo, users):
|
|
|
|
"""Add a new branch to an intermediate step of a fw *sequence*, either
|
|
|
|
because skipci or because all the intermediate CI succeeded
|
|
|
|
"""
|
|
|
|
# region setup
|
|
|
|
r1, _ = make_basic(env, config, make_repo, statuses="default")
|
|
|
|
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
project = env['runbot_merge.project'].search([])
|
|
|
|
project.write({
|
|
|
|
'branch_ids': [(0, 0, {'name': 'd', 'sequence': 40})],
|
|
|
|
})
|
|
|
|
with r1, r2:
|
|
|
|
r1.make_commits('c', Commit('1111', tree={'g': 'a'}), ref='heads/d')
|
|
|
|
r2.make_commits('c', Commit('1111', tree={'g': 'a'}), ref='heads/d')
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
|
|
|
with r1:
|
|
|
|
r1.make_commits('a', Commit('a', tree={'a': 'a'}), ref="heads/pr1")
|
|
|
|
pr1_a = r1.make_pr(target="a", head="pr1")
|
|
|
|
r1.post_status(pr1_a.head, 'success')
|
|
|
|
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.a', 'success')
|
|
|
|
r2.post_status('staging.a', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
# region port forward
|
|
|
|
pr1_a_id = to_pr(env, pr1_a)
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
for branch in 'bcd':
|
|
|
|
next_pr = env['runbot_merge.pull_requests'].search([
|
|
|
|
('source_id', '=', pr1_a_id.id),
|
|
|
|
('target.name', '=', branch),
|
|
|
|
])
|
|
|
|
assert next_pr
|
|
|
|
with r1:
|
|
|
|
r1.post_status(next_pr.head, 'success')
|
|
|
|
env.run_crons()
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
# endregion
|
|
|
|
# endregion
|
|
|
|
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
pr1_b_id, pr1_c_id, pr1_d_id = pr1_a_id.forwardport_ids[::-1]
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
# new PR must be in fork for labels to actually match
|
|
|
|
with r2, fork2:
|
|
|
|
# branch in fork has no owner prefix, but HEAD for cross-repo PR does
|
2025-02-06 18:35:55 +07:00
|
|
|
fork2.make_commits(r2.commit("b").id, Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}')
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
pr2_b = r2.make_pr(title="b", target="b", head=pr1_b_id.label)
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr2_b_id = to_pr(env, pr2_b)
|
|
|
|
assert pr2_b_id.batch_id == pr1_b_id.batch_id
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
assert len(pr2_b_id.forwardport_ids) == 2, \
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
"since the batch is already forward ported, the new PR should" \
|
|
|
|
" immediately be forward ported to match"
|
|
|
|
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
assert pr2_b_id.forwardport_ids.mapped('label') == (pr1_d_id | pr1_c_id).mapped('label')
|
|
|
|
|
|
|
|
assert not (b := env['forwardport.batches'].search([])),\
|
|
|
|
f"there should not be any more forward porting, found {b.read(['source', 'pr_id'])}"
|
|
|
|
known_set = pr1_a_id | pr1_a_id.forwardport_ids | pr2_b_id | pr2_b_id.forwardport_ids
|
|
|
|
assert not (p := (env['runbot_merge.pull_requests'].search([]) - known_set)), \
|
|
|
|
"there should not be any PR outside of the sequences we created, "\
|
|
|
|
f"found {p.read(['source_id', 'parent_id', 'display_name'])}"
|
|
|
|
|
|
|
|
with r2:
|
|
|
|
for pr_id in pr2_b_id | pr2_b_id.forwardport_ids:
|
|
|
|
r2.post_status(pr_id.head, 'success')
|
|
|
|
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
with r1, r2:
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
r1.get_pr(pr1_a_id.forwardport_ids[0].number)\
|
|
|
|
.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
r2.get_pr(pr2_b_id.forwardport_ids[0].number) \
|
|
|
|
.post_comment('hansen r+', config['role_reviewer']['token'])
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
with r1, r2:
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
for branch in 'bcd':
|
|
|
|
r1.post_status(f'staging.{branch}', 'success')
|
|
|
|
r2.post_status(f'staging.{branch}', 'success')
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
env.run_crons()
|
|
|
|
|
[FIX] *: double forwardport when adding a PR to an existing batch
This is a bit of an odd case which was only noticed because of
persistent forwardport.batches, which ended up having a ton of related
traceback in the logs (the mergebot kept trying to create forward
ports from Jan 27th to Feb 10th, thankfully the errors happened in git
so did not seem to eat through our API rate limiting).
The issue was triggered by the addition of odoo/enterprise#77876 to
odoo/odoo#194818. This triggered a completion job which led to the
creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so
good.
Except the bit of code responsible for creating completion jobs only
checked if the PR was being added to a batch with a descendant. That
is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not
odoo/enterprise#77880 because that's the end of the line). As a
result, those triggered 3 more completion jobs, which kept failing in
a loop because they tried pushing different commits to their
next-siblings (without forcing, leading git to reject the non-ff push,
hurray).
A completion request should only be triggered by the addition of a
new *source* (a PR without a source) to an existing batch with
descendants, so add that to the check. This requires updating
`_from_json` to create PRs in a single step (rather than one step to
create based on github's data, and an other one for the hierarchical
tracking) as we need the source to be set during `create` not as a
post-action.
Although there was a test which could have triggered this issue, the
test only had 3 branches so was not long enough to trigger the issue:
- Initial PR 1 on branch A merged then forward-ported to B and C.
- Sibling PR 2 added to the batch in B.
- Completed to C.
- Ending there as C(1) has no descendant batch, leading to no further
completion request.
Adding a 4th branch did surface / show the issue by providing space
for a new completion request from the creation of C(2). Interestingly
even though I the test harness attempts to run all triggered crons to
completion there can be misses, so the test can fail in two different
ways (being now checked for):
- there's a leftover forwardport.batch after we've created all our
forwardports
- there's an extra PR targeting D, descending from C(2)
- in almost every case there's also a traceback in the logs, which
does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
|
|
|
assert not env['runbot_merge.pull_requests'].search([]) - known_set, \
|
|
|
|
"there should not be any PR outside of the sequences we created"
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
|
|
|
def test_add_to_forward_port_conflict(env, config, make_repo, users):
|
|
|
|
"""If a PR is added to an existing forward port sequence, and it causes
|
|
|
|
conflicts when forward ported, it should be treated similarly to an *update*
|
|
|
|
causing a conflict: the PR is still created, but it's set in conflict.
|
|
|
|
"""
|
|
|
|
# region setup
|
|
|
|
r1, _ = make_basic(env, config, make_repo, statuses="default")
|
|
|
|
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
|
|
|
|
project = env['runbot_merge.project'].search([])
|
|
|
|
with r2:
|
|
|
|
r2.make_commits(
|
|
|
|
"c",
|
|
|
|
Commit("C-onflict", tree={"b": "X"}),
|
|
|
|
ref="heads/c"
|
|
|
|
)
|
|
|
|
|
|
|
|
with r1:
|
|
|
|
r1.make_commits('a', Commit('a', tree={'a': 'a'}), ref="heads/pr1")
|
|
|
|
pr1_a = r1.make_pr(target="a", head="pr1")
|
|
|
|
r1.post_status(pr1_a.head, 'success')
|
|
|
|
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.a', 'success')
|
|
|
|
r2.post_status('staging.a', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
# region port forward
|
|
|
|
pr1_a_id = to_pr(env, pr1_a)
|
|
|
|
pr1_b_id = pr1_a_id.forwardport_ids
|
|
|
|
assert pr1_b_id
|
|
|
|
with r1:
|
|
|
|
r1.post_status(pr1_b_id.head, 'success')
|
|
|
|
env.run_crons()
|
|
|
|
pr1_c_id = pr1_a_id.forwardport_ids - pr1_b_id
|
|
|
|
assert pr1_c_id
|
|
|
|
# endregion
|
|
|
|
# endregion
|
|
|
|
|
|
|
|
# new PR must be in fork for labels to actually match
|
|
|
|
with r2, fork2:
|
|
|
|
# branch in fork has no owner prefix, but HEAD for cross-repo PR does
|
2025-02-06 18:35:55 +07:00
|
|
|
fork2.make_commits(r2.commit("b").id, Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}')
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
pr2_b = r2.make_pr(title="b", target="b", head=pr1_b_id.label)
|
|
|
|
r2.post_status(pr2_b.head, 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
pr2_b_id = to_pr(env, pr2_b)
|
|
|
|
assert pr2_b_id.batch_id == pr1_b_id.batch_id
|
|
|
|
pr2_c_id = pr2_b_id.forwardport_ids
|
|
|
|
assert len(pr2_c_id) == 1, \
|
|
|
|
"since the batch is already forward ported, the new PR should" \
|
|
|
|
" immediately be forward ported to match"
|
|
|
|
assert pr2_c_id.label == pr1_c_id.label
|
|
|
|
assert not pr2_c_id.parent_id, "conflict -> should be detached"
|
|
|
|
assert pr2_c_id.detach_reason
|
|
|
|
|
|
|
|
pr2_a = r1.get_pr(pr1_b_id.number)
|
|
|
|
with r1, r2:
|
|
|
|
pr2_a.post_comment('hansen r+', config['role_reviewer']['token'])
|
|
|
|
pr2_b.post_comment("hansen r+", config['role_reviewer']['token'])
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
with r1, r2:
|
|
|
|
r1.post_status('staging.b', 'success')
|
|
|
|
r2.post_status('staging.b', 'success')
|
|
|
|
env.run_crons()
|
|
|
|
|
|
|
|
assert pr1_b_id.state == 'merged'
|
|
|
|
assert pr2_b_id.state == 'merged'
|
|
|
|
|
|
|
|
pr2_c = r2.get_pr(pr2_c_id.number)
|
|
|
|
assert pr2_c.comments == [
|
|
|
|
seen(env, pr2_c, users),
|
|
|
|
# should have conflicts
|
[CHG] forwardport: perform forward porting without working copies
The goal is to reduce maintenance and odd disk interactions &
concurrency issues, by not creating concurrent clones, not having to
push forks back in the repository, etc... it also removes the need to
cleanup "scratch" working copies though that looks not to have been an
issue in a while.
The work is done on isolated objects without using or mutating refs,
so even concurrent work should not be a problem.
This turns out to not be any more verbose (less so if anything) than
using `cherry-pick`, as that is not really designed for scripted /
non-interactive use, or for squashing commits thereafter. Working
directly with trees and commits is quite a bit cleaner even without a
ton of helpers.
Much of the credit goes to Julia Evans for [their investigation of
3-way merges as the underpinnings of cherry-picking][3-way merge],
this would have been a lot more difficult if I'd had to rediscover the
merge-base trick independently.
A few things have been changed by this:
- The old trace/stderr from cherrypick has disappeared as it's
generated by cherrypick, but for a non-interactive use it's kinda
useless anyway so I probably should have looked into removing it
earlier (I think the main use was investigation of the inflateinit
issue).
- Error on emptied commits has to be hand-rolled as `merge-tree`
couldn't care less, this is not hard but is a bit annoying.
- `merge-tree`'s conflict information only references raw commits,
which makes sense, but requires updating a bunch of tests. Then
again so does the fact that it *usually* doesn't send anything to
stderr, so that's usually disappearing.
Conveniently `merge-tree` merges the conflict marker directly in the
files / tree so we don't have to mess about moving them back out of
the repository and into the working copy as I assume cherry-pick does,
which means we don't have to try and commit them back in ether. That
is a huge part of the gain over faffing about with the working copy.
Fixes #847
[3-way merge]: https://jvns.ca/blog/2023/11/10/how-cherry-pick-and-revert-work/
2024-07-05 18:32:02 +07:00
|
|
|
(users['user'], """@{user} cherrypicking of pull request {previous.display_name} failed.
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
|
|
|
stdout:
|
|
|
|
```
|
|
|
|
Auto-merging b
|
2024-06-04 17:15:29 +07:00
|
|
|
CONFLICT (add/add): Merge conflict in b
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
|
|
|
```
|
|
|
|
|
2024-06-04 17:15:29 +07:00
|
|
|
Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
2024-06-04 17:15:29 +07:00
|
|
|
In the former case, you may want to edit this PR message as well.
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
2024-06-04 17:15:29 +07:00
|
|
|
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}.
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
|
2024-06-04 17:15:29 +07:00
|
|
|
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
[CHG] forwardport: perform forward porting without working copies
The goal is to reduce maintenance and odd disk interactions &
concurrency issues, by not creating concurrent clones, not having to
push forks back in the repository, etc... it also removes the need to
cleanup "scratch" working copies though that looks not to have been an
issue in a while.
The work is done on isolated objects without using or mutating refs,
so even concurrent work should not be a problem.
This turns out to not be any more verbose (less so if anything) than
using `cherry-pick`, as that is not really designed for scripted /
non-interactive use, or for squashing commits thereafter. Working
directly with trees and commits is quite a bit cleaner even without a
ton of helpers.
Much of the credit goes to Julia Evans for [their investigation of
3-way merges as the underpinnings of cherry-picking][3-way merge],
this would have been a lot more difficult if I'd had to rediscover the
merge-base trick independently.
A few things have been changed by this:
- The old trace/stderr from cherrypick has disappeared as it's
generated by cherrypick, but for a non-interactive use it's kinda
useless anyway so I probably should have looked into removing it
earlier (I think the main use was investigation of the inflateinit
issue).
- Error on emptied commits has to be hand-rolled as `merge-tree`
couldn't care less, this is not hard but is a bit annoying.
- `merge-tree`'s conflict information only references raw commits,
which makes sense, but requires updating a bunch of tests. Then
again so does the fact that it *usually* doesn't send anything to
stderr, so that's usually disappearing.
Conveniently `merge-tree` merges the conflict marker directly in the
files / tree so we don't have to mess about moving them back out of
the repository and into the working copy as I assume cherry-pick does,
which means we don't have to try and commit them back in ether. That
is a huge part of the gain over faffing about with the working copy.
Fixes #847
[3-way merge]: https://jvns.ca/blog/2023/11/10/how-cherry-pick-and-revert-work/
2024-07-05 18:32:02 +07:00
|
|
|
""".format(project=project, previous=pr2_b_id, **users))
|
[IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).
There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?
Options are:
- don't do anything, such additions don't get ported, this is
incongruous and unexpected as by default PRs are forward-ported, and
if the batch wasn't an intermediate (but e.g. a conflict) it
probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
need its own limit) but it means further batches may get
unexpectedly merged (or at least retied) without the additional PR
even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
harder or impossible to configure but it makes the *batch sequence*
more consistent
We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-03-29 15:16:51 +07:00
|
|
|
]
|