NextERP/runbot
5
0
Fork 0
mirror of https://github.com/odoo/runbot.git synced 2025-03-17 00:15:47 +07:00
Code Issues Actions Packages Projects Releases Wiki Activity
15.0-fix-stats-xdo
runbot/forwardport/models/forwardport.py

252 lines
10 KiB
Python
Raw Permalink Normal View History

[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# -*- coding: utf-8 -*-
import logging
[IMP] forwardport: mitigate cat-file not finding commit on updates Fix #457 hopefully: I didn't manage to repro / create a test for. It looks like in some cases during the update process the PR ref lags behind the branch itself. This means `forwardport.updates` creates a new commit, pushes it, then on the next iteration updates the local cache, tries to find the commit we just pushed... and that fails. I can only assume this is because when there's enough load on the github side the update to the `info/refs` pseudo-file can fall behind (it's now 4MB and holding nearly 65k refs). So cheat: take the commit we just pushed to the dev remote and... immediately push it to the local cache under a dummy branch, which we delete. Since we only gc "1 day ago" this should not vacuum.
2021-03-01 18:38:23 +07:00
import uuid
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
from contextlib import ExitStack
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
from datetime import datetime
from dateutil import relativedelta
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
from odoo import fields, models
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
from odoo.addons.runbot_merge.github import GH
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
# how long a merged PR survives
MERGE_AGE = relativedelta.relativedelta(weeks=2)
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
_logger = logging.getLogger(__name__)
class Queue:
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
limit = 100
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
def _process_item(self):
raise NotImplementedError
def _process(self):
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
for b in self.search(self._search_domain(), order='create_date, id', limit=self.limit):
[IMP] forwardport: processing queue reliability The queue would get items to process one at a time, process, commit, and go to the next. However this is an issue if one of the item fails systematically for some reason (aka it's not just a transient failure): the cron fails, then restarts at the exact same point, and fails again with the same issue, leading to following items never getting processed. Fix by getting all the queue contents at once, processing them one by one and "skipping" any item which fails (leaving it in place so it can get re-processed later). That way, even if an item causes issues, the rest of the queue gets processed normally. The interruption was an issue following odoo/enterprise#5670 not getting properly updated in the backend (backend didn't get notified of the last two updates / force-push to the PR, so it was trying to forward-port a commit which didn't exist - and failing).
2019-10-10 13:41:33 +07:00
try:
[FIX] forwardport: queue reliability changes Previous version would break if _process_item itself committed which was bad
2019-10-11 14:02:50 +07:00
b._process_item()
[IMP] forwardport: processing queue reliability The queue would get items to process one at a time, process, commit, and go to the next. However this is an issue if one of the item fails systematically for some reason (aka it's not just a transient failure): the cron fails, then restarts at the exact same point, and fails again with the same issue, leading to following items never getting processed. Fix by getting all the queue contents at once, processing them one by one and "skipping" any item which fails (leaving it in place so it can get re-processed later). That way, even if an item causes issues, the rest of the queue gets processed normally. The interruption was an issue following odoo/enterprise#5670 not getting properly updated in the backend (backend didn't get notified of the last two updates / force-push to the PR, so it was trying to forward-port a commit which didn't exist - and failing).
2019-10-10 13:41:33 +07:00
b.unlink()
self.env.cr.commit()
except Exception:
_logger.exception("Error while processing %s, skipping", b)
[FIX] forwardport: queue reliability changes Previous version would break if _process_item itself committed which was bad
2019-10-11 14:02:50 +07:00
self.env.cr.rollback()
self.clear_caches()
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
def _search_domain(self):
return []
[FIX] forwardport: don't forwardport freeze PRs The freeze wizard has support for merging freeze / release PRs on each of the newly created branches. But since this would be done by, well, merging, those PRs would get forward-ported to master, and would have to be closed there. This creates additional work for the freeze master, and noise / parasitic PRs. Obviously it's possible for the freeze master to set some nonsense `up to` (nonsense because the "real" limit doesn't exist yet at that point), but really it never makes any sense to forward port release PRs, so the wizard should do it.
2022-02-08 16:11:57 +07:00
class ForwardPortTasks(models.Model, Queue):
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
_name = 'forwardport.batches'
_description = 'batches which got merged and are candidates for forward-porting'
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
limit = 10
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
batch_id = fields.Many2one('runbot_merge.batch', required=True)
source = fields.Selection([
('merge', 'Merge'),
('fp', 'Forward Port Followup'),
[ADD] forwardport: special handling of adding branches to projects If a new branch is added to a project, there's an issue with *ongoing* forward ports (forward ports which were not merged before the branch was forked from an existing one): the new branch gets "skipped" and might be missing some fixes until those are noticed and backported. This commit hooks into updating projects to try and see if the update consists of adding a branch inside the sequence, in which case it tries to find the FP sequences to update and queues up new "intermediate" forward ports to insert into the existing sequences. Note: had to increase the cron socket limit to 2mn as 1mn blew up the big staging cron in the test (after all the forward-port PRs are approved). Fixes #262 [FIX]
2020-01-27 21:39:25 +07:00
('insert', 'New branch port')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
], required=True)
def _process_item(self):
batch = self.batch_id
newbatch = batch.prs._port_forward()
[ADD] forwardport: special handling of adding branches to projects If a new branch is added to a project, there's an issue with *ongoing* forward ports (forward ports which were not merged before the branch was forked from an existing one): the new branch gets "skipped" and might be missing some fixes until those are noticed and backported. This commit hooks into updating projects to try and see if the update consists of adding a branch inside the sequence, in which case it tries to find the FP sequences to update and queues up new "intermediate" forward ports to insert into the existing sequences. Note: had to increase the cron socket limit to 2mn as 1mn blew up the big staging cron in the test (after all the forward-port PRs are approved). Fixes #262 [FIX]
2020-01-27 21:39:25 +07:00
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
if newbatch:
_logger.info(
"Processing %s (from %s): %s (%s) -> %s (%s)",
self.id, self.source,
batch, batch.prs,
newbatch, newbatch.prs,
)
[FIX] forwardport: chain crash on insert in forward-port cron On two of the freezes, thereafter the logs showed serial crashes in the forwardport cron when trying to find the insertion point for a new forward-port. The first time was not really diagnosed, the second time the cause was found to be a retargeted PR which led to a failure of the "insertion" forward port, which did not take that possibility in account (it assumed -- sensibly I believed -- that an intermediate FP following a branch insertion would always succeed, sadly the malevolent universe had other plans). So only insert the new forward port inside its sequence (if necessary) if the forward port actually succeeded, otherwise ignore it. Fixes #551
2022-02-10 19:42:32 +07:00
# insert new batch in ancestry sequence unless conflict (= no parent)
if self.source == 'insert':
for pr in newbatch.prs:
if not pr.parent_id:
break
newchild = pr.search([
('parent_id', '=', pr.parent_id.id),
('id', '!=', pr.id),
])
if newchild:
newchild.parent_id = pr.id
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
else: # reached end of seq (or batch is empty)
# FIXME: or configuration is fucky so doesn't want to FP (maybe should error and retry?)
_logger.info(
"Processing %s (from %s): %s (%s) -> end of the sequence",
self.id, self.source,
batch, batch.prs
)
batch.active = False
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
CONFLICT_TEMPLATE = "WARNING: the latest change ({previous.head}) triggered " \
"a conflict when updating the next forward-port " \
"({next.display_name}), and has been ignored.\n\n" \
"You will need to update this pull request differently, " \
"or fix the issue by hand on {next.display_name}."
CHILD_CONFLICT = "WARNING: the update of {previous.display_name} to " \
"{previous.head} has caused a conflict in this pull request, " \
"data may have been lost."
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
class UpdateQueue(models.Model, Queue):
_name = 'forwardport.updates'
_description = 'if a forward-port PR gets updated & has followups (cherrypick succeeded) the followups need to be updated as well'
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
limit = 10
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
original_root = fields.Many2one('runbot_merge.pull_requests')
new_root = fields.Many2one('runbot_merge.pull_requests')
def _process_item(self):
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
Feedback = self.env['runbot_merge.pull_requests.feedback']
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
previous = self.new_root
with ExitStack() as s:
for child in self.new_root._iter_descendants():
[FIX] forwardport: possible race in forwardport followup update Normally when a forwardport is updated the forward-bot cascades the update onto its followups (if it has any), but takes care to keep the followups attached as they were not updated "by hand". In the case of odoo/odoo#77677 however that did not work and the followup PRs got detached. Looking at the logs, it becomes flagrant that there was a race condition: either Git took a long time to respond to the push, or there was an IO slowdown which led to the "local update" taking a very long time. Either way this allowed the "synchronize" webhook from github to arrive before the current transaction was committed, rolling back said transaction and making the forwardbot assume this was a "real" sync and detach the followup from its parent. Locking the PR row up-front ought fix the issue, and also move the local update to before having pushed: the "extra" commits in the local cache don't matter too much even if pushing to github fails, they'll be cleaned up by a GC eventually. Also migrate the `-f` on push to `--force-if-includes` in order to avoid possible race conditions on the push (since we're not fetching the current branch, use the full-syntax explicit CAS form, that's exactly what we're looking for anyway). Fixes #541 (hopefully)
2021-10-19 15:30:55 +07:00
self.env.cr.execute("""
SELECT id
FROM runbot_merge_pull_requests
WHERE id = %s
FOR UPDATE NOWAIT
""", [child.id])
[FIX] forwardport: PR update through a closed PR Fixes #328
2020-02-19 18:12:02 +07:00
_logger.info(
"Re-port %s from %s (changed root %s -> %s)",
child.display_name,
previous.display_name,
self.original_root.display_name,
self.new_root.display_name
)
if child.state in ('closed', 'merged'):
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
Feedback.create({
[FIX] forwardport: PR update through a closed PR Fixes #328
2020-02-19 18:12:02 +07:00
'repository': child.repository.id,
'pull_request': child.number,
'message': "Ancestor PR %s has been updated but this PR"
" is %s and can't be updated to match."
"\n\n"
"You may want or need to manually update any"
" followup PR." % (
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
self.new_root.display_name,
child.state,
)
[FIX] forwardport: PR update through a closed PR Fixes #328
2020-02-19 18:12:02 +07:00
})
return
[IMP] *: add draft support to mergebot, kinda * Remove the forwardport creating PRs in draft, that was mostly to avoid codeowners triggering but we've removed the github one and hand-rolled it, so not a concern anymore. * Prevent merging `draft` PRs, the mergebot rejects approval on draft PRs and insults people. TBD (maybe): try to create *conflicting* forward-port PRs in draft so it's clearer they need to be *fixed*? Issue of not being able to do that on all private repositories remains so~~ Fixes #500
2021-08-11 16:36:35 +07:00
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
conflicts, working_copy = previous._create_fp_branch(
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
child.target, child.refname, s)
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
if conflicts:
[IMP] forwardport: provide clearer picture of conflicts On conflicts in multi-commit PRs developers sometimes get confused as to what happened why. If a conflict occurs and the source pull request had multiple commits, list all the source commits and show which one broke. Related to #505
2021-08-11 19:08:18 +07:00
_, out, err, _ = conflicts
[IMP] forwardport: handling of updates causing conflicts on followups If a PR is updated and has extent forward-ports, those forwardports get updated automatically ("followup"). However there is an issue if the udpate causes a conflict in the followup: the conflict gets silently pushed, and may fairly easily get merged if it occurs in an area which the CI doesn't cover. It's unclear what the policy really should be for this issue, and there is no real way to *block* a pull request at the moment (save by putting it in error at the mergebot level I guess?), so for now clearly notify the user on both the modified PR and the followup, with a comment on both. We may want to revisit this eventually. Fixes #467
2021-07-27 14:17:18 +07:00
Feedback.create({
'repository': previous.repository.id,
'pull_request': previous.number,
'message': CONFLICT_TEMPLATE.format(
previous=previous,
next=child
)
})
Feedback.create({
'repository': child.repository.id,
'pull_request': child.number,
'message': CHILD_CONFLICT.format(previous=previous, next=child)\
+ (f'\n\nstdout:\n```\n{out.strip()}\n```' if out.strip() else '')
+ (f'\n\nstderr:\n```\n{err.strip()}\n```' if err.strip() else '')
})
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
new_head = working_copy.stdout().rev_parse(child.refname).stdout.decode().strip()
[FIX] forwardport: completely update PR data when forwarding updates The process did properly update the state, but not the squash state. It's somewhat unclear whether the state should be fully reset and require reapproval though. Maybe only the validation should be reset? The CI will eventually run and either succeed (re-validating) or fail (devalidating, hopefully) but I'm not entirely sure this is correct.
2021-02-26 15:45:44 +07:00
commits_count = int(working_copy.stdout().rev_list(
f'{child.target.name}..{child.refname}',
count=True
).stdout.decode().strip())
[FIX] forwardport: possible race in forwardport followup update Normally when a forwardport is updated the forward-bot cascades the update onto its followups (if it has any), but takes care to keep the followups attached as they were not updated "by hand". In the case of odoo/odoo#77677 however that did not work and the followup PRs got detached. Looking at the logs, it becomes flagrant that there was a race condition: either Git took a long time to respond to the push, or there was an IO slowdown which led to the "local update" taking a very long time. Either way this allowed the "synchronize" webhook from github to arrive before the current transaction was committed, rolling back said transaction and making the forwardbot assume this was a "real" sync and detach the followup from its parent. Locking the PR row up-front ought fix the issue, and also move the local update to before having pushed: the "extra" commits in the local cache don't matter too much even if pushing to github fails, they'll be cleaned up by a GC eventually. Also migrate the `-f` on push to `--force-if-includes` in order to avoid possible race conditions on the push (since we're not fetching the current branch, use the full-syntax explicit CAS form, that's exactly what we're looking for anyway). Fixes #541 (hopefully)
2021-10-19 15:30:55 +07:00
old_head = child.head
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# update child's head to the head we're going to push
[FIX] forwardport: completely update PR data when forwarding updates The process did properly update the state, but not the squash state. It's somewhat unclear whether the state should be fully reset and require reapproval though. Maybe only the validation should be reset? The CI will eventually run and either succeed (re-validating) or fail (devalidating, hopefully) but I'm not entirely sure this is correct.
2021-02-26 15:45:44 +07:00
child.with_context(ignore_head_update=True).write({
'head': new_head,
# 'state': 'opened',
'squash': commits_count == 1,
})
[FIX] forwardport: possible race in forwardport followup update Normally when a forwardport is updated the forward-bot cascades the update onto its followups (if it has any), but takes care to keep the followups attached as they were not updated "by hand". In the case of odoo/odoo#77677 however that did not work and the followup PRs got detached. Looking at the logs, it becomes flagrant that there was a race condition: either Git took a long time to respond to the push, or there was an IO slowdown which led to the "local update" taking a very long time. Either way this allowed the "synchronize" webhook from github to arrive before the current transaction was committed, rolling back said transaction and making the forwardbot assume this was a "real" sync and detach the followup from its parent. Locking the PR row up-front ought fix the issue, and also move the local update to before having pushed: the "extra" commits in the local cache don't matter too much even if pushing to github fails, they'll be cleaned up by a GC eventually. Also migrate the `-f` on push to `--force-if-includes` in order to avoid possible race conditions on the push (since we're not fetching the current branch, use the full-syntax explicit CAS form, that's exactly what we're looking for anyway). Fixes #541 (hopefully)
2021-10-19 15:30:55 +07:00
# push the new head to the local cache: in some cases github
# doesn't propagate revisions fast enough so on the next loop we
# can't find the revision we just pushed
[IMP] forwardport: mitigate cat-file not finding commit on updates Fix #457 hopefully: I didn't manage to repro / create a test for. It looks like in some cases during the update process the PR ref lags behind the branch itself. This means `forwardport.updates` creates a new commit, pushes it, then on the next iteration updates the local cache, tries to find the commit we just pushed... and that fails. I can only assume this is because when there's enough load on the github side the update to the `info/refs` pseudo-file can fall behind (it's now 4MB and holding nearly 65k refs). So cheat: take the commit we just pushed to the dev remote and... immediately push it to the local cache under a dummy branch, which we delete. Since we only gc "1 day ago" this should not vacuum.
2021-03-01 18:38:23 +07:00
dummy_branch = str(uuid.uuid4())
ref = previous._get_local_directory()
working_copy.push(ref._directory, f'{new_head}:refs/heads/{dummy_branch}')
ref.branch('--delete', '--force', dummy_branch)
[FIX] forwardport: possible race in forwardport followup update Normally when a forwardport is updated the forward-bot cascades the update onto its followups (if it has any), but takes care to keep the followups attached as they were not updated "by hand". In the case of odoo/odoo#77677 however that did not work and the followup PRs got detached. Looking at the logs, it becomes flagrant that there was a race condition: either Git took a long time to respond to the push, or there was an IO slowdown which led to the "local update" taking a very long time. Either way this allowed the "synchronize" webhook from github to arrive before the current transaction was committed, rolling back said transaction and making the forwardbot assume this was a "real" sync and detach the followup from its parent. Locking the PR row up-front ought fix the issue, and also move the local update to before having pushed: the "extra" commits in the local cache don't matter too much even if pushing to github fails, they'll be cleaned up by a GC eventually. Also migrate the `-f` on push to `--force-if-includes` in order to avoid possible race conditions on the push (since we're not fetching the current branch, use the full-syntax explicit CAS form, that's exactly what we're looking for anyway). Fixes #541 (hopefully)
2021-10-19 15:30:55 +07:00
# then update the child's branch to the new head
working_copy.push(f'--force-with-lease={child.refname}:{old_head}',
'target', child.refname)
[IMP] forwardport: mitigate cat-file not finding commit on updates Fix #457 hopefully: I didn't manage to repro / create a test for. It looks like in some cases during the update process the PR ref lags behind the branch itself. This means `forwardport.updates` creates a new commit, pushes it, then on the next iteration updates the local cache, tries to find the commit we just pushed... and that fails. I can only assume this is because when there's enough load on the github side the update to the `info/refs` pseudo-file can fall behind (it's now 4MB and holding nearly 65k refs). So cheat: take the commit we just pushed to the dev remote and... immediately push it to the local cache under a dummy branch, which we delete. Since we only gc "1 day ago" this should not vacuum.
2021-03-01 18:38:23 +07:00
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# committing here means github could technically trigger its
# webhook before sending a response, but committing before
# would mean we can update the PR in database but fail to
# update on github, which is probably worse?
# alternatively we can commit, push, and rollback if the push
# fails
# FIXME: handle failures (especially on non-first update)
self.env.cr.commit()
previous = child
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
_deleter = _logger.getChild('deleter')
class DeleteBranches(models.Model, Queue):
_name = 'forwardport.branch_remover'
_description = "Removes branches of merged PRs"
pr_id = fields.Many2one('runbot_merge.pull_requests')
def _search_domain(self):
cutoff = self.env.context.get('forwardport_merged_before') \
or fields.Datetime.to_string(datetime.now() - MERGE_AGE)
[IMP] forwardport: reliability of PR reminders The exponential backoff offsets from the write_date of the children PRs, however it doesn't reset, so the offsetting gets bumped up way more than originally expected or designed if the child PRs are under active development for some reason. Fix this by adding a field to specifically record the date of merge of a PR, and check that feature against the backoff offset. This should provide more regular and reliable backoff. Fixes #369
2020-05-20 17:42:45 +07:00
return [('pr_id.merge_date', '<', cutoff)]
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
def _process_item(self):
_deleter.info(
"PR %s: checking deletion of linked branch %s",
self.pr_id.display_name,
self.pr_id.label
)
if self.pr_id.state != 'merged':
_deleter.info('✘ PR is not "merged" (got %s)', self.pr_id.state)
return
repository = self.pr_id.repository
fp_remote = repository.fp_remote_target
if not fp_remote:
_deleter.info('✘ no forward-port target')
return
repo_owner, repo_name = fp_remote.split('/')
owner, branch = self.pr_id.label.split(':')
if repo_owner != owner:
_deleter.info('✘ PR owner != FP target owner (%s)', repo_owner)
return # probably don't have access to arbitrary repos
[FIX] forwardport: the fix it's a per-call parameter not a per-instance one
2019-10-18 13:11:27 +07:00
github = GH(token=repository.project_id.fp_github_token, repo=fp_remote)
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
refurl = 'git/refs/heads/' + branch
[FIX] forwardport: the fix it's a per-call parameter not a per-instance one
2019-10-18 13:11:27 +07:00
ref = github('get', refurl, check=False)
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
if ref.status_code != 200:
_deleter.info("✘ branch already deleted (%s)", ref.json())
return
ref = ref.json()
[FIX] forwardport: apparently git/refs/heads can fuzzy-match If the ref we asked for does not exist, github apparently decides to fall-back to prefix-matching. So if we're trying to delete already-deleted branch A and someone called their branch A-x we're going to get it as a result. Thankfully they were apparently smart enough to return a list even if there's only a single fuzzy match. So if we get a list (instead of a dict) as response to git/refs/heads assume the branch was already deleted as if we got a 404.
2019-10-18 16:21:52 +07:00
if isinstance(ref, list):
_deleter.info(
"✘ got a fuzzy match (%s), branch probably deleted",
', '.join(r['ref'] for r in ref)
)
[FIX] forwardport: the fix Of course I forgot the most relevant bit
2019-10-18 17:01:47 +07:00
return
[FIX] forwardport: apparently git/refs/heads can fuzzy-match If the ref we asked for does not exist, github apparently decides to fall-back to prefix-matching. So if we're trying to delete already-deleted branch A and someone called their branch A-x we're going to get it as a result. Thankfully they were apparently smart enough to return a list even if there's only a single fuzzy match. So if we get a list (instead of a dict) as response to git/refs/heads assume the branch was already deleted as if we got a 404.
2019-10-18 16:21:52 +07:00
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
if ref['object']['sha'] != self.pr_id.head:
_deleter.info(
"✘ branch %s head mismatch, expected %s, got %s",
self.pr_id.label,
self.pr_id.head,
ref['object']['sha']
)
return
[FIX] forwardport: the fix it's a per-call parameter not a per-instance one
2019-10-18 13:11:27 +07:00
r = github('delete', refurl, check=False)
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
assert r.status_code == 204, \
"Tried to delete branch %s of %s, got %s" % (
branch, self.pr_id.display_name,
r.json()
)
_deleter.info('✔ deleted branch %s of PR %s', self.pr_id.label, self.pr_id.display_name)
Reference in New Issue Copy Permalink