NextERP/runbot
5
0
Fork 0
mirror of https://github.com/odoo/runbot.git synced 2025-03-17 08:25:46 +07:00
Code Issues Actions Packages Projects Releases Wiki Activity
17.0-api-xdo-moc
runbot/forwardport/models/project.py

1318 lines
56 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 -*-
"""
Technically could be independent from mergebot but would require a lot of
duplicate work e.g. keeping track of statuses (including on commits which
might not be in PRs yet), handling co-dependent PRs, ...
However extending the mergebot also leads to messiness: fpbot should have
its own user / feedback / API keys, mergebot and fpbot both have branch
ordering but for mergebot it's completely cosmetics, being slaved to mergebot
means PR creation is trickier (as mergebot assumes opened event will always
lead to PR creation but fpbot wants to attach meaning to the PR when setting
it up), ...
"""
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
import ast
[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
import base64
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
import collections
[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
import contextlib
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
import datetime
[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
import itertools
import json
import logging
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
import operator
[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
import os
import pathlib
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
import re
[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
import subprocess
import tempfile
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
import typing
[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
[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
import dateutil.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
import requests
[FIX] forwardport: relax limits on all git operations Apparently it's not just GC which causes trouble, on the new configuration anyway.
2022-12-12 13:40:39 +07:00
import resource
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
from odoo import _, models, fields, api
[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
from odoo.osv import expression
[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.exceptions import UserError
[FIX] forwardport: correct batching of intermediate PRs When inserting a new branch, if there are extant forward ports sequences spanning over the insertion, new forward ports must be created for the new branch. This was the case, *however* one of the cases was handled incorrectly: PR batches (PRs to different repositories staged together) must be forward-ported as batches. However when creating the "insert" batches, these PRs would be split apart, leading to either inconsistent states or an inability to merge the new forward ports (if they are co-dependent which is a common case). This is because the handler would look for all the relevant PRs to forward ports, but it would fail to group them by label thereafter. Fix that, create the `insert` batches correctly, and augment the relevant test with an additional repository to test this case. No gh issue was created, but this problem was reported on odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by hand in the runbot and mergebot, but on github the labels remain visibly different, one having the slug ZCwE while the other hasO7Pr). It's possible that the problem had occurred previously and not been noticed (or reported), though it's also possible this issue had never occurred before: for the latest freeze (16.1) there were 18 forward ports spanning the insertion, but of them only 2 were the same batch.
2023-01-19 16:49:16 +07:00
from odoo.tools.misc import topological_sort, groupby
[REM] forwardport: fp_sequence field It's almost certainly not useful, save as a minor convenience for tests: decorrelating the branch sequence and the fp sequence seems like it would only be extremely confusing, and on the mergebot all the fp_sequence values are set to the default while the sequence values are set to something useful and sensible (kinda). Fixes #584
2022-12-08 16:42:12 +07:00
from odoo.tools.sql import reverse_order
[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.tools.appdirs import user_cache_dir
from odoo.addons.runbot_merge import utils
[FIX] forwardport: don't re-approve approved PRs during an fw-bot r+ When using the forwardport's shortcut, the bot would not skip already-approved PRs leading to a warning from the mergebot that the PR was already approved (out of nowhere which was weird). During the walk to the ancestors, skip any PR which is not approvable (either already approved or in a state where that makes no sense e.g. closed). Fixes #543
2021-10-06 19:42:54 +07:00
from odoo.addons.runbot_merge.models.pull_requests import RPLUS
[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
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
footer = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n'
[IMP] forwardport: on conflict note previous FP can be r+'d Closes #294
2020-03-12 14:33:15 +07:00
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
DEFAULT_DELTA = dateutil.relativedelta.relativedelta(days=3)
[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('odoo.addons.forwardport')
class Project(models.Model):
_inherit = 'runbot_merge.project'
fp_github_token = fields.Char()
fp_github_name = fields.Char(store=True, compute="_compute_git_identity")
fp_github_email = fields.Char(store=True, compute="_compute_git_identity")
def _find_commands(self, comment):
if self.env.context.get('without_forward_port'):
return super()._find_commands(comment)
return re.findall(
'^\s*[@|#]?{}:? (.*)$'.format(self.fp_github_name),
comment, re.MULTILINE | re.IGNORECASE
) + super()._find_commands(comment)
# technically the email could change at any moment...
@api.depends('fp_github_token')
def _compute_git_identity(self):
s = requests.Session()
for project in self:
if not project.fp_github_token:
continue
r0 = s.get('https://api.github.com/user', headers={
'Authorization': 'token %s' % project.fp_github_token
})
if 'user:email' not in set(re.split(r',\s*', r0.headers['x-oauth-scopes'])):
raise UserError(_("The forward-port github token needs the user:email scope to fetch the bot's identity."))
r1 = s.get('https://api.github.com/user/emails', headers={
'Authorization': 'token %s' % project.fp_github_token
})
if not (r0.ok and r1.ok):
[IMP] runbot_merge, forwardport: update some logging Downgrade an error and a warning to info, and upgrade two warnings to error. Point is to improve logic of log levels & sentry visibility. Fixes #258
2021-01-13 21:48:39 +07:00
_logger.error("Failed to fetch bot information for project %s: %s", project.name, (r0.text or r0.content) if not r0.ok else (r1.text or r1.content))
[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
continue
project.fp_github_name = r0.json()['login']
project.fp_github_email = next((
entry['email']
for entry in r1.json()
if entry['primary']
), None)
if not project.fp_github_email:
raise UserError(_("The forward-port bot needs a primary email set up."))
[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
def write(self, vals):
# check on branches both active and inactive so disabling branches doesn't
# make it look like the sequence changed.
self_ = self.with_context(active_test=False)
[FIX] *: fix forward port inserter if last branch is disabled In case where the last branch (before the branch being frozen) is disabled, the forwardport inserter screws up, and fails to correctly create the intermediate forwardports from the new branch. Also when disabling a branch, if there are FW PRs which target that branch and have not been forward-ported further, automatically forward-port them as if the branch had been disabled when they were created, this should limit data loss and confusion. Also change the message set on PRs when disabling a branch: because of user conflicts in test setup, the message about a branch being disabled would close the PRs, which would then orphan the followup, leading to unexpected / inconsistent behaviour. Fixes #665
2022-11-10 21:55:21 +07:00
previously_active_branches = {project: project.branch_ids.filtered('active') for project in self_}
[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
branches_before = {project: project._forward_port_ordered() for project in self_}
r = super().write(vals)
[FIX] *: fix forward port inserter if last branch is disabled In case where the last branch (before the branch being frozen) is disabled, the forwardport inserter screws up, and fails to correctly create the intermediate forwardports from the new branch. Also when disabling a branch, if there are FW PRs which target that branch and have not been forward-ported further, automatically forward-port them as if the branch had been disabled when they were created, this should limit data loss and confusion. Also change the message set on PRs when disabling a branch: because of user conflicts in test setup, the message about a branch being disabled would close the PRs, which would then orphan the followup, leading to unexpected / inconsistent behaviour. Fixes #665
2022-11-10 21:55:21 +07:00
self_._followup_prs(previously_active_branches)
self_._insert_intermediate_prs(branches_before)
return r
def _followup_prs(self, previously_active_branches):
"""If a branch has been disabled and had PRs without a followup (e.g.
because no CI or CI failed), create followup, as if the branch had been
originally disabled (and thus skipped over)
"""
PRs = self.env['runbot_merge.pull_requests']
for p in self:
actives = previously_active_branches[p]
for deactivated in p.branch_ids.filtered(lambda b: not b.active) & actives:
# if a PR targets a deactivated branch, and that's not its limit,
# and it doesn't have a child (e.g. CI failed), enqueue a forward
# port as if the now deactivated branch had been skipped over (which
# is the normal fw behaviour)
extant = PRs.search([
('target', '=', deactivated.id),
('source_id.limit_id', '!=', deactivated.id),
('state', 'not in', ('closed', 'merged')),
])
for p in extant.with_context(force_fw=True):
next_target = p.source_id._find_next_target(p)
# should not happen since we already filtered out limits
if not next_target:
continue
# check if it has a descendant in the next branch, if so skip
if PRs.search_count([
('source_id', '=', p.source_id.id),
('target', '=', next_target.id)
]):
continue
# otherwise enqueue a followup
p._schedule_fp_followup()
def _insert_intermediate_prs(self, branches_before):
"""If new branches have been added to the sequence inbetween existing
branches (mostly a freeze inserted before the main branch), fill in
forward-ports for existing sequences
"""
Branches = self.env['runbot_merge.branch']
for p in self:
[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
# check if the branches sequence has been modified
bbefore = branches_before[p]
bafter = p._forward_port_ordered()
if bafter.ids == bbefore.ids:
continue
[IMP] forwardport: add logging to branch reinsertion When a new branch is created between other branches, the process to try and look for forward-port PRs to create to "fill in" currently has no logging so it's very difficult to figure out why it decides not to do something. Add some logging to the process to try and better understand what happens. Fixes #441
2021-01-12 18:54:35 +07:00
logger = _logger.getChild('project').getChild(p.name)
logger.debug("branches updated %s -> %s", bbefore, bafter)
[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
# if it's just that a branch was inserted at the end forwardport
# should keep on keeping normally
if bafter.ids[:-1] == bbefore.ids:
continue
if bafter <= bbefore:
raise UserError("Branches can not be reordered or removed after saving.")
# Last possibility: branch was inserted but not at end, get all
# branches before and all branches after
before = new = after = Branches
for b in bafter:
if b in bbefore:
if new:
after += b
else:
before += b
else:
if new:
raise UserError("Inserting multiple branches at the same time is not supported")
new = b
[IMP] forwardport: add logging to branch reinsertion When a new branch is created between other branches, the process to try and look for forward-port PRs to create to "fill in" currently has no logging so it's very difficult to figure out why it decides not to do something. Add some logging to the process to try and better understand what happens. Fixes #441
2021-01-12 18:54:35 +07:00
logger.debug('before: %s new: %s after: %s', before.ids, new.ids, after.ids)
[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
# find all FPs whose ancestry spans the insertion
leaves = self.env['runbot_merge.pull_requests'].search([
('state', 'not in', ['closed', 'merged']),
('target', 'in', after.ids),
('source_id.target', 'in', before.ids),
])
# get all PRs just preceding the insertion point which either are
# sources of the above or have the same source
candidates = self.env['runbot_merge.pull_requests'].search([
('target', '=', before[-1].id),
'|', ('id', 'in', leaves.mapped('source_id').ids),
('source_id', 'in', leaves.mapped('source_id').ids),
])
[IMP] forwardport: add logging to branch reinsertion When a new branch is created between other branches, the process to try and look for forward-port PRs to create to "fill in" currently has no logging so it's very difficult to figure out why it decides not to do something. Add some logging to the process to try and better understand what happens. Fixes #441
2021-01-12 18:54:35 +07:00
logger.debug("\nPRs spanning new: %s\nto port: %s", leaves, candidates)
[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
# enqueue the creation of a new forward-port based on our candidates
[FIX] forwardport: correct batching of intermediate PRs When inserting a new branch, if there are extant forward ports sequences spanning over the insertion, new forward ports must be created for the new branch. This was the case, *however* one of the cases was handled incorrectly: PR batches (PRs to different repositories staged together) must be forward-ported as batches. However when creating the "insert" batches, these PRs would be split apart, leading to either inconsistent states or an inability to merge the new forward ports (if they are co-dependent which is a common case). This is because the handler would look for all the relevant PRs to forward ports, but it would fail to group them by label thereafter. Fix that, create the `insert` batches correctly, and augment the relevant test with an additional repository to test this case. No gh issue was created, but this problem was reported on odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by hand in the runbot and mergebot, but on github the labels remain visibly different, one having the slug ZCwE while the other hasO7Pr). It's possible that the problem had occurred previously and not been noticed (or reported), though it's also possible this issue had never occurred before: for the latest freeze (16.1) there were 18 forward ports spanning the insertion, but of them only 2 were the same batch.
2023-01-19 16:49:16 +07:00
# but it should only create a single step and needs to stitch back
[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
# the parents linked list, so it has a special type
[FIX] forwardport: correct batching of intermediate PRs When inserting a new branch, if there are extant forward ports sequences spanning over the insertion, new forward ports must be created for the new branch. This was the case, *however* one of the cases was handled incorrectly: PR batches (PRs to different repositories staged together) must be forward-ported as batches. However when creating the "insert" batches, these PRs would be split apart, leading to either inconsistent states or an inability to merge the new forward ports (if they are co-dependent which is a common case). This is because the handler would look for all the relevant PRs to forward ports, but it would fail to group them by label thereafter. Fix that, create the `insert` batches correctly, and augment the relevant test with an additional repository to test this case. No gh issue was created, but this problem was reported on odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by hand in the runbot and mergebot, but on github the labels remain visibly different, one having the slug ZCwE while the other hasO7Pr). It's possible that the problem had occurred previously and not been noticed (or reported), though it's also possible this issue had never occurred before: for the latest freeze (16.1) there were 18 forward ports spanning the insertion, but of them only 2 were the same batch.
2023-01-19 16:49:16 +07:00
for _, cs in groupby(candidates, key=lambda p: p.label):
[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
self.env['forwardport.batches'].create({
'batch_id': self.env['runbot_merge.batch'].create({
'target': before[-1].id,
[FIX] forwardport: correct batching of intermediate PRs When inserting a new branch, if there are extant forward ports sequences spanning over the insertion, new forward ports must be created for the new branch. This was the case, *however* one of the cases was handled incorrectly: PR batches (PRs to different repositories staged together) must be forward-ported as batches. However when creating the "insert" batches, these PRs would be split apart, leading to either inconsistent states or an inability to merge the new forward ports (if they are co-dependent which is a common case). This is because the handler would look for all the relevant PRs to forward ports, but it would fail to group them by label thereafter. Fix that, create the `insert` batches correctly, and augment the relevant test with an additional repository to test this case. No gh issue was created, but this problem was reported on odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by hand in the runbot and mergebot, but on github the labels remain visibly different, one having the slug ZCwE while the other hasO7Pr). It's possible that the problem had occurred previously and not been noticed (or reported), though it's also possible this issue had never occurred before: for the latest freeze (16.1) there were 18 forward ports spanning the insertion, but of them only 2 were the same batch.
2023-01-19 16:49:16 +07:00
'prs': [(4, c.id, 0) for c in cs],
[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
'active': False,
}).id,
'source': 'insert',
})
def _forward_port_ordered(self, domain=()):
Branches = self.env['runbot_merge.branch']
return Branches.search(expression.AND([
[('project_id', '=', self.id)],
domain or [],
[REM] forwardport: fp_sequence field It's almost certainly not useful, save as a minor convenience for tests: decorrelating the branch sequence and the fp sequence seems like it would only be extremely confusing, and on the mergebot all the fp_sequence values are set to the default while the sequence values are set to something useful and sensible (kinda). Fixes #584
2022-12-08 16:42:12 +07:00
]), order=reverse_order(Branches._order))
[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
class Repository(models.Model):
_inherit = 'runbot_merge.repository'
fp_remote_target = fields.Char(help="where FP branches get pushed")
class Branch(models.Model):
_inherit = 'runbot_merge.branch'
[FIX] forwarport: mark branches as fp-targets by default otherwise the freeze wizard gets very confused
2021-12-02 17:36:10 +07:00
fp_target = fields.Boolean(default=True)
[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
fp_enabled = fields.Boolean(compute='_compute_fp_enabled')
@api.depends('active', 'fp_target')
def _compute_fp_enabled(self):
for b in self:
b.fp_enabled = b.active and b.fp_target
class PullRequests(models.Model):
_inherit = 'runbot_merge.pull_requests'
[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
limit_id = fields.Many2one('runbot_merge.branch', help="Up to which branch should this PR be forward-ported")
[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
parent_id = fields.Many2one(
'runbot_merge.pull_requests', index=True,
help="a PR with a parent is an automatic forward port"
)
source_id = fields.Many2one('runbot_merge.pull_requests', index=True, help="the original source of this FP even if parents were detached along the way")
[ADD] forwardport: views of outstanding forwardports Though the forwardport posts regular reminders that an fw is outdated, it can be easy to miss for the non-subject (and apparently the subjects often just ignore the information entirely). Add a few relevant links there: * on PR pages, add a link to either the source or the forward-ports (if applicable), as well as the merge date * add a new page which lists all the PRs with outstanding forwardports, as well as the forwardports in question Fixes #474
2021-07-28 18:57:58 +07:00
forwardport_ids = fields.One2many('runbot_merge.pull_requests', 'source_id')
[IMP] *: remove default group operator on objects After review, there doesn't seem to be a single integer field created by the mergebot or fortwardbot modules for which a `group_operator` makes sense, let alone the default of `sum`. So just disable them all. Fixes #674
2022-12-07 21:13:55 +07:00
reminder_backoff_factor = fields.Integer(default=-4, group_operator=None)
[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
merge_date = fields.Datetime()
[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
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
detach_reason = fields.Char()
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
fw_policy = fields.Selection([
('ci', "Normal"),
('skipci', "Skip CI"),
# ('skipmerge', "Skip merge"),
], required=True, default="ci")
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
_sql_constraints = [(
'fw_constraint',
'check(source_id is null or num_nonnulls(parent_id, detach_reason) = 1)',
"fw PRs must either be attached or have a reason for being detached",
)]
[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
refname = fields.Char(compute='_compute_refname')
@api.depends('label')
def _compute_refname(self):
for pr in self:
pr.refname = pr.label.split(':', 1)[-1]
[FIX] forwardport: decoration of PullRequests.create Apparently multi is the default, so the method not being decorated blows up when creating fake PRs through the interface.
2021-11-12 22:03:28 +07:00
@api.model_create_single
[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 create(self, vals):
# PR opened event always creates a new PR, override so we can precreate PRs
existing = self.search([
('repository', '=', vals['repository']),
('number', '=', vals['number']),
])
if existing:
return existing
[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
if 'limit_id' not in vals:
branch = self.env['runbot_merge.branch'].browse(vals['target'])
repo = self.env['runbot_merge.repository'].browse(vals['repository'])
vals['limit_id'] = branch.project_id._forward_port_ordered(
ast.literal_eval(repo.branch_filter or '[]')
)[-1].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
if vals.get('parent_id') and 'source_id' not in vals:
vals['source_id'] = self.browse(vals['parent_id'])._get_root().id
[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
if vals.get('state') == 'merged':
vals['merge_date'] = fields.Datetime.now()
[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
return super().create(vals)
def write(self, vals):
# if the PR's head is updated, detach (should split off the FP lines as this is not the original code)
# TODO: better way to do this? Especially because we don't want to
# recursively create updates
# also a bit odd to only handle updating 1 head at a time, but then
# again 2 PRs with same head is weird so...
newhead = vals.get('head')
[IMP] forwardport: notify when FP PR gets de-parented If a PR is explicitly updated, it gets converted to a normal PR[0]. Before this, users had no indication that this had happened and might be wondering what they're supposed to do (or try to r+ via the forwardbot, which doesn't work on a root PR). [0] to an extent: the PR still has a source and might have children, in which case the followups will be created from the source & existing followups should be updated to match Closes #206
2019-09-23 16:50:21 +07:00
with_parents = self.filtered('parent_id')
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
closed_fp = self.filtered(lambda p: p.state == 'closed' and p.source_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
if newhead and not self.env.context.get('ignore_head_update') and newhead != self.head:
vals.setdefault('parent_id', False)
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
if with_parents and vals['parent_id'] is False:
vals['detach_reason'] = f"Head updated from {self.head} to {newhead}"
[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 any children, this is an FP PR being updated, enqueue
# updating children
if self.search_count([('parent_id', '=', self.id)]):
self.env['forwardport.updates'].create({
'original_root': self._get_root().id,
'new_root': self.id
})
if vals.get('parent_id') and 'source_id' not in vals:
vals['source_id'] = self.browse(vals['parent_id'])._get_root().id
[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
if vals.get('state') == 'merged':
vals['merge_date'] = fields.Datetime.now()
[IMP] forwardport: notify when FP PR gets de-parented If a PR is explicitly updated, it gets converted to a normal PR[0]. Before this, users had no indication that this had happened and might be wondering what they're supposed to do (or try to r+ via the forwardbot, which doesn't work on a root PR). [0] to an extent: the PR still has a source and might have children, in which case the followups will be created from the source & existing followups should be updated to match Closes #206
2019-09-23 16:50:21 +07:00
r = super().write(vals)
if self.env.context.get('forwardport_detach_warn', True):
for p in with_parents:
if not p.parent_id:
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': p.repository.id,
'pull_request': p.number,
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
'message': "%sthis PR was modified / updated and has become a normal PR. "
"It should be merged the normal way (via @%s)" % (
p.source_id.ping(),
p.repository.project_id.github_prefix,
),
[IMP] forwardport: notify when FP PR gets de-parented If a PR is explicitly updated, it gets converted to a normal PR[0]. Before this, users had no indication that this had happened and might be wondering what they're supposed to do (or try to r+ via the forwardbot, which doesn't work on a root PR). [0] to an extent: the PR still has a source and might have children, in which case the followups will be created from the source & existing followups should be updated to match Closes #206
2019-09-23 16:50:21 +07:00
'token_field': 'fp_github_token',
})
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
for p in closed_fp.filtered(lambda p: p.state != 'closed'):
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': p.repository.id,
'pull_request': p.number,
'message': "%sthis PR was closed then reopened. "
"It should be merged the normal way (via @%s)" % (
p.source_id.ping(),
p.repository.project_id.github_prefix,
),
'token_field': 'fp_github_token',
})
[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 vals.get('state') == 'merged':
for p in self:
self.env['forwardport.branch_remover'].create({
'pr_id': p.id,
})
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
# if we change the policy to skip CI, schedule followups on existing FPs
if vals.get('fw_policy') == 'skipci' and self.state == 'merged':
self.env['runbot_merge.pull_requests'].search([
('source_id', '=', self.id),
('state', 'not in', ('closed', 'merged')),
])._schedule_fp_followup()
[IMP] forwardport: notify when FP PR gets de-parented If a PR is explicitly updated, it gets converted to a normal PR[0]. Before this, users had no indication that this had happened and might be wondering what they're supposed to do (or try to r+ via the forwardbot, which doesn't work on a root PR). [0] to an extent: the PR still has a source and might have children, in which case the followups will be created from the source & existing followups should be updated to match Closes #206
2019-09-23 16:50:21 +07:00
return r
[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 _try_closing(self, by):
r = super()._try_closing(by)
if r:
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
self.with_context(forwardport_detach_warn=False).write({
'parent_id': False,
'detach_reason': f"Closed by {by}",
})
self.search([('parent_id', '=', self.id)]).write({
'parent_id': False,
'detach_reason': f"{by} closed parent PR {self.display_name}",
})
[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
return r
def _parse_commands(self, author, comment, login):
super(PullRequests, self.with_context(without_forward_port=True))._parse_commands(author, comment, login)
tokens = [
token
[ADD] runbot_merge: allow overriding statuses Adds an `override` mergebot command. The ability to override is set on an individual per-context per-repository basis, similar to but independent from review rights. That is, a given individual may be able to override the status X on repository A and unable to do so on repository B. Overrides are stored in the same format as regular statuses, but independent from them in order to persist them across builds. Only PR statuses can be overridden, statuses which are overridable on PRs would simply not be required on stagings. An alternative to implementing this feature in the mergebot would be to add it to individual status-generating tools on a per-need basis. Pros of that alternative: * display the correct status on PRs, currently the PR will be failing status-wise (on github) but correct as far as the mergebot is concerned * remove complexity from the mergebot Cons of that alternative: * each status-generating tool would have to implement some sort of ACL system * each status-generating tool would have to receive & parse PR comments * each status-generating tool would have to maintain per-pr state in order to track overrides Some sort of helper library / framework ought make that rather easy though. It could also be linked into the central provisioning system thing. Closes #376
2020-07-14 15:06:07 +07:00
for line in re.findall('^\s*[@|#]?{}:? (.*)$'.format(self.repository.project_id.fp_github_name), comment['body'] or '', re.MULTILINE | re.IGNORECASE)
[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
for token in line.split()
]
if not tokens:
_logger.info("found no commands in comment of %s (%s) (%s)", author.github_login, author.display_name,
[ADD] runbot_merge: allow overriding statuses Adds an `override` mergebot command. The ability to override is set on an individual per-context per-repository basis, similar to but independent from review rights. That is, a given individual may be able to override the status X on repository A and unable to do so on repository B. Overrides are stored in the same format as regular statuses, but independent from them in order to persist them across builds. Only PR statuses can be overridden, statuses which are overridable on PRs would simply not be required on stagings. An alternative to implementing this feature in the mergebot would be to add it to individual status-generating tools on a per-need basis. Pros of that alternative: * display the correct status on PRs, currently the PR will be failing status-wise (on github) but correct as far as the mergebot is concerned * remove complexity from the mergebot Cons of that alternative: * each status-generating tool would have to implement some sort of ACL system * each status-generating tool would have to receive & parse PR comments * each status-generating tool would have to maintain per-pr state in order to track overrides Some sort of helper library / framework ought make that rather easy though. It could also be linked into the central provisioning system thing. Closes #376
2020-07-14 15:06:07 +07:00
utils.shorten(comment['body'] or '', 50)
[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
)
return
# TODO: don't use a mutable tokens iterator
tokens = iter(tokens)
[IMP] forwardport: improve disable fp UI * always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
2019-09-19 18:33:38 +07:00
while True:
token = next(tokens, None)
if token is None:
break
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = False
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
close = False
msg = None
if token in ('ci', 'skipci'):
pr = (self.source_id or self)
if pr._pr_acl(author).is_reviewer:
pr.fw_policy = token
[FIX] forwardport: nicer skipci message
2020-06-03 20:05:20 +07:00
msg = "Not waiting for CI to create followup forward-ports." if token == 'skipci' else "Waiting for CI to create followup forward-ports."
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
else:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = True
msg = "you can't configure ci."
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
[IMP] forwardport: improve disable fp UI * always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
2019-09-19 18:33:38 +07:00
if token == 'ignore': # replace 'ignore' by 'up to <pr_branch>'
token = 'up'
tokens = itertools.chain(['to', self.target.name], tokens)
[IMP] forwardport: feedback on commands The forwardbot's command parsing was missing feedback when trying to use commands without the proper ACL. This would make some situations of comments seemingly being lost hard to diagnose. Closes #300
2020-01-29 21:09:09 +07:00
if token in ('r+', 'review+'):
[IMP] forwardport: error message when trying to r+ via fwbot Initial thinking was to remove the check entirely and leave it to the mergebot, but the lack of error reporting / forwarding means while technically correct it would probably be somewhat difficult to grok. Instead, improve the error reporting: * add a dedicated message when trying to r+ via fwbot on a non-fw PR (note: maybe the fwbot should not care? and just send it as-is to the mergebot in that case?) * clarify the ACL error * post both message as the forwardbot rather than the mergebot Also add a missing token note for the feedback from the forwardport limit. fix #469
2021-07-27 20:23:34 +07:00
if not self.source_id:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = True
msg = "I can only do this on forward-port PRs and this is not one, see {}.".format(
self.repository.project_id.github_prefix
)
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
elif not self.parent_id:
ping = True
msg = "I can only do this on unmodified forward-port PRs, ask {}.".format(
self.repository.project_id.github_prefix
)
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
else:
merge_bot = self.repository.project_id.github_prefix
# don't update the root ever
for pr in (p for p in self._iter_ancestors() if p.parent_id if p.state in RPLUS):
# only the author is delegated explicitely on the
pr._parse_commands(author, {**comment, 'body': merge_bot + ' r+'}, login)
[IMP] forwardport: close command This is useful as the author of the original PR doesn't necessarily have (write) access to the repository where the forward-port PR was created. As a result, while they can r+ the PR they're unable to close it (via github's interface). Since the forwardport bot created the PR, it can also close it, which seems like a useful feature. Closes #341
2020-03-02 19:42:07 +07:00
elif token == 'close':
if self.source_id._pr_acl(author).is_reviewer:
close = True
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
else:
ping = True
msg = "you can't close PRs."
[IMP] forwardport: feedback on commands The forwardbot's command parsing was missing feedback when trying to use commands without the proper ACL. This would make some situations of comments seemingly being lost hard to diagnose. Closes #300
2020-01-29 21:09:09 +07:00
elif token == 'up' and next(tokens, None) == 'to':
[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
limit = next(tokens, None)
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = True
[IMP] forwardport: feedback on commands The forwardbot's command parsing was missing feedback when trying to use commands without the proper ACL. This would make some situations of comments seemingly being lost hard to diagnose. Closes #300
2020-01-29 21:09:09 +07:00
if not self._pr_acl(author).is_author:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
msg = "you can't set a forward-port limit.".format(login)
elif not limit:
msg = "please provide a branch to forward-port to."
[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:
limit_id = self.env['runbot_merge.branch'].with_context(active_test=False).search([
('project_id', '=', self.repository.project_id.id),
('name', '=', limit),
])
[FIX] fwbot: properly prevent @up to on forward-port PRs There was already a check, but the way the check behaved means *detached* PRs would not be prevented from setting their forward-port, despite that not doing anything. Fix it by checking if the current PR has a source, not a parent. Fixes #465
2021-07-26 19:00:14 +07:00
if self.source_id:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
msg = "forward-port limit can only be set on " \
[FIX] fwbot: properly prevent @up to on forward-port PRs There was already a check, but the way the check behaved means *detached* PRs would not be prevented from setting their forward-port, despite that not doing anything. Fix it by checking if the current PR has a source, not a parent. Fixes #465
2021-07-26 19:00:14 +07:00
f"an origin PR ({self.source_id.display_name} " \
"here) before it's merged and forward-ported."
[IMP] forwardport: add feedback when being wrong User should probably be warned when they try to set the limit ("up to") in a context where it's going to be ignored: * on a forward port PR (should be set on the source) * on a merged source (should be set before the PR is merged) Closes #213
2019-10-02 22:03:03 +07:00
elif self.state in ['merged', 'closed']:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
msg = "forward-port limit can only be set before the PR is merged."
[IMP] forwardport: add feedback when being wrong User should probably be warned when they try to set the limit ("up to") in a context where it's going to be ignored: * on a forward port PR (should be set on the source) * on a merged source (should be set before the PR is merged) Closes #213
2019-10-02 22:03:03 +07:00
elif not limit_id:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
msg = "there is no branch %r, it can't be used as a forward port target." % limit
[IMP] forwardport: improve disable fp UI * always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
2019-09-19 18:33:38 +07:00
elif limit_id == self.target:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = False
[IMP] forwardport: improve disable fp UI * always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
2019-09-19 18:33:38 +07:00
msg = "Forward-port disabled."
self.limit_id = limit_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
elif not limit_id.fp_enabled:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
msg = "branch %r is disabled, it can't be used as a forward port target." % limit_id.name
[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:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
ping = False
[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
msg = "Forward-porting to %r." % limit_id.name
self.limit_id = limit_id
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
if msg or close:
if msg:
_logger.info("%s [%s]: %s", self.display_name, login, msg)
else:
_logger.info("%s [%s]: closing", self.display_name, login)
[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
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': self.repository.id,
'pull_request': self.number,
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
'message': f'@{author.github_login} {msg}' if msg and ping else msg,
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
'close': close,
[IMP] runbot_merge, forwardport: variable-user feedback Having all the feedback be sent by the mergebot user (github_token) is confusing. Add a way to specify which field of project should be used to source the token used when sending feedback. Fixes #190
2019-09-18 20:37:14 +07:00
'token_field': 'fp_github_token',
[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
})
[IMP] *: attempt to avoid some of the FP spam Attempt to avoid some of the comment spam by dedup-ing input (only signaling when the status actually changes and ignoring identity transformations) and in case of failing CI keeping the last failed status and not signaling on the next update if it's the same failure. Closes #225
2019-10-07 21:38:14 +07:00
def _notify_ci_failed(self, ci):
# only care about FP PRs which are not staged / merged yet
# NB: probably ignore approved PRs as normal message will handle them?
if not (self.state == 'opened' and self.parent_id):
return
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': self.repository.id,
'pull_request': self.number,
'token_field': 'fp_github_token',
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
'message': '%s%s failed on this forward-port PR' % (
self.source_id.ping(),
[IMP] *: attempt to avoid some of the FP spam Attempt to avoid some of the comment spam by dedup-ing input (only signaling when the status actually changes and ignoring identity transformations) and in case of failing CI keeping the last failed status and not signaling on the next update if it's the same failure. Closes #225
2019-10-07 21:38:14 +07:00
ci,
)
})
[IMP] forwardport: improve disable fp UI * always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
2019-09-19 18:33:38 +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
def _validate(self, statuses):
[IMP] forwardport: ping on CI failure It's especially important as users / assignees don't get pinged *during* the forwardport process. closes #203
2019-09-18 13:32:38 +07:00
failed = super()._validate(statuses)
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
self._schedule_fp_followup()
return failed
def _schedule_fp_followup(self):
_logger = logging.getLogger(__name__).getChild('forwardport.next')
[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 the PR has a parent and is CI-validated, enqueue the next PR
for pr in self:
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
_logger.info('Checking if forward-port %s (%s)', pr.display_name, pr)
[FIX] forwardport: breakage in previous commit leftover unnecessary change in 52699d901abe61432c0526a072c75bf434e42fec broke one of the tests
2019-09-18 16:48:01 +07:00
if not pr.parent_id:
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
_logger.info('-> no parent %s (%s)', pr.display_name, pr.parent_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
continue
[FIX] *: fix forward port inserter if last branch is disabled In case where the last branch (before the branch being frozen) is disabled, the forwardport inserter screws up, and fails to correctly create the intermediate forwardports from the new branch. Also when disabling a branch, if there are FW PRs which target that branch and have not been forward-ported further, automatically forward-port them as if the branch had been disabled when they were created, this should limit data loss and confusion. Also change the message set on PRs when disabling a branch: because of user conflicts in test setup, the message about a branch being disabled would close the PRs, which would then orphan the followup, leading to unexpected / inconsistent behaviour. Fixes #665
2022-11-10 21:55:21 +07:00
if not self.env.context.get('force_fw') and self.source_id.fw_policy != 'skipci' and pr.state not in ['validated', 'ready']:
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
_logger.info('-> wrong state %s (%s)', pr.display_name, pr.state)
[IMP] forwardport: ping on CI failure It's especially important as users / assignees don't get pinged *during* the forwardport process. closes #203
2019-09-18 13:32:38 +07:00
continue
[FIX] forwardport: FP PR getting CI'd after initial FP check Test is probably more complex than necessary (thinking about it, the failed staging is probably unnecessary) but that triggers the issue and matches the original scenario. The problem was really with new CI events being received on the last forward-port PR of a sequence: previous PRs would have a child PR so the check would abort, but for the last PR it would go through, fail to find an active batch, then blow up as it tries to create a forwardport.batch without an actual batch. Change this to use the existence of an inactive batch not linked to a staging as a flag that the PR has been processed and forward-ported. Closes #218
2019-10-02 01:51:31 +07:00
# check if we've already forward-ported this branch:
# it has a batch without a staging
batch = self.env['runbot_merge.batch'].with_context(active_test=False).search([
('staging_id', '=', False),
('prs', 'in', pr.id),
], limit=1)
# if the batch is inactive, the forward-port has been done *or*
# the PR's own forward port is in error, so bail
if not batch.active:
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
_logger.info('-> forward port done or in error (%s.active=%s)', batch, batch.active)
[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
continue
[FIX] forwardport: FP PR getting CI'd after initial FP check Test is probably more complex than necessary (thinking about it, the failed staging is probably unnecessary) but that triggers the issue and matches the original scenario. The problem was really with new CI events being received on the last forward-port PR of a sequence: previous PRs would have a child PR so the check would abort, but for the last PR it would go through, fail to find an active batch, then blow up as it tries to create a forwardport.batch without an actual batch. Change this to use the existence of an inactive batch not linked to a staging as a flag that the PR has been processed and forward-ported. Closes #218
2019-10-02 01:51:31 +07:00
# otherwise check if we already have a pending forward port
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
_logger.info("%s %s %s", pr.display_name, batch, ', '.join(batch.mapped('prs.display_name')))
[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 self.env['forwardport.batches'].search_count([('batch_id', '=', batch.id)]):
[FIX] forwardport: logging.warn -> logging.warning logging.warn is deprecated but I've a hard time shaking the habit.
2020-01-27 18:46:50 +07:00
_logger.warning('-> already recorded')
[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
continue
# check if batch-mate are all valid
mates = batch.prs
# wait until all of them are validated or ready
[FIX] *: fix forward port inserter if last branch is disabled In case where the last branch (before the branch being frozen) is disabled, the forwardport inserter screws up, and fails to correctly create the intermediate forwardports from the new branch. Also when disabling a branch, if there are FW PRs which target that branch and have not been forward-ported further, automatically forward-port them as if the branch had been disabled when they were created, this should limit data loss and confusion. Also change the message set on PRs when disabling a branch: because of user conflicts in test setup, the message about a branch being disabled would close the PRs, which would then orphan the followup, leading to unexpected / inconsistent behaviour. Fixes #665
2022-11-10 21:55:21 +07:00
if not self.env.context.get('force_fw') and any(pr.source_id.fw_policy != 'skipci' and pr.state not in ('validated', 'ready') for pr in mates):
[IMP] forwardport: lower level of waiting for batch to be ready Waiting for the current batch to validate before forward-porting it is normal, it's useful information but should not be warned about.
2020-07-30 18:20:41 +07:00
_logger.info("-> not ready (%s)", [(pr.display_name, pr.state) for pr in mates])
[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
continue
# check that there's no weird-ass state
if not all(pr.parent_id for pr in mates):
[FIX] forwardport: logging.warn -> logging.warning logging.warn is deprecated but I've a hard time shaking the habit.
2020-01-27 18:46:50 +07:00
_logger.warning("Found a batch (%s) with only some PRs having parents, ignoring", mates)
[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
continue
if self.search_count([('parent_id', 'in', mates.ids)]):
[FIX] forwardport: logging.warn -> logging.warning logging.warn is deprecated but I've a hard time shaking the habit.
2020-01-27 18:46:50 +07:00
_logger.warning("Found a batch (%s) with only some of the PRs having children", mates)
[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
continue
_logger.info('-> ok')
self.env['forwardport.batches'].create({
'batch_id': batch.id,
'source': 'fp',
})
def _find_next_target(self, reference):
""" Finds the branch between target and limit_id which follows
reference
"""
if reference.target == self.limit_id:
return
# NOTE: assumes even disabled branches are properly sequenced, would
# probably be a good idea to have the FP view show all branches
[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
branches = list(self.target.project_id
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
.with_context(active_test=False)
[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
._forward_port_ordered(ast.literal_eval(self.repository.branch_filter or '[]')))
[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
# get all branches between max(root.target, ref.target) (excluded) and limit (included)
from_ = max(branches.index(self.target), branches.index(reference.target))
to_ = branches.index(self.limit_id)
# return the first active branch in the set
return next((
branch
for branch in branches[from_+1:to_+1]
if branch.fp_enabled
), None)
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
def _commits_lazy(self):
[FIX] forwardport: forgot token when getting PR commits So would break any time it needed to fetch the commits of a PR in a private repo.
2019-09-12 19:35:56 +07:00
s = requests.Session()
s.headers['Authorization'] = 'token %s' % self.repository.project_id.fp_github_token
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
for page in itertools.count(1):
[FIX] forwardport: forgot token when getting PR commits So would break any time it needed to fetch the commits of a PR in a private repo.
2019-09-12 19:35:56 +07:00
r = s.get('https://api.github.com/repos/{}/pulls/{}/commits'.format(
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
self.repository.name,
self.number
), params={'page': page})
r.raise_for_status()
yield from r.json()
if not r.links.get('next'):
return
def commits(self):
""" Returns a PR's commits oldest first (that's what GH does &
is what we want)
"""
commits = list(self._commits_lazy())
# map shas to the position the commit *should* have
idx = {
c: i
for i, c in enumerate(topological_sort({
c['sha']: [p['sha'] for p in c['parents']]
for c in commits
}))
}
return sorted(commits, key=lambda c: idx[c['sha']])
[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 _iter_descendants(self):
pr = self
while True:
pr = self.search([('parent_id', '=', pr.id)])
if pr:
yield pr
else:
break
[IMP] forwardport: unmodified fw automatically inherit overrides Before this change, a CI override would have to be replicated on most / all forward-ports of the base PR. This was intentional to see how it would shake out, the answer being that it's rather annoying. Also add a `statuses_full` computed field on PRs for the aggregate status: the existing `statuses` field is just a copy of the commit statuses which I didn't remember I kept free of the overrides so the commit statuses could be displayed "as-is" in the backend (the overrides are displayed separately). And while at it fix the PR dashboard to use that new field: that was basically the intention but then I went on to use the "wrong" field hence #433. Mebbe the UI part should be displayed using a computed M2M (?) as a table or as tags instead? This m2m could indicate whether the status is an override or an "intrinsic" status. Also removed some dead code: * leftover from the removed tagging feature (removed the tag manipulation but forgot some of the setup / computations) * unused local variables * an empty skipped test case Fixes #439. Fixes #433.
2021-01-13 14:18:17 +07:00
@api.depends('parent_id.statuses')
def _compute_statuses(self):
super()._compute_statuses()
def _get_overrides(self):
# NB: assumes _get_overrides always returns an "owned" dict which we can modify
p = self.parent_id._get_overrides() if self.parent_id else {}
p.update(super()._get_overrides())
return p
[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 _iter_ancestors(self):
while self:
yield self
self = self.parent_id
def _get_root(self):
root = self
while root.parent_id:
root = root.parent_id
return root
def _port_forward(self):
if not self:
return
[FIX] fwbot: properly prevent @up to on forward-port PRs There was already a check, but the way the check behaved means *detached* PRs would not be prevented from setting their forward-port, despite that not doing anything. Fix it by checking if the current PR has a source, not a parent. Fixes #465
2021-07-26 19:00:14 +07:00
all_sources = [(p.source_id or p) for p in self]
[FIX] forwardport: duplicates forwardport on edition On edition of an intermediate PR in a chain, merging the PR would lead to *it* being forward-ported, duplicating the PRs already created from *its* source. Add a check for PRs in the target branch with the same source, suppresses the forward-porting of the newly merged PR. Fixes #451 (hopefully)
2021-03-01 16:35:09 +07:00
all_targets = [s._find_next_target(p) for s, p in zip(all_sources, self)]
[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
[FIX] forwardport: duplicates forwardport on edition On edition of an intermediate PR in a chain, merging the PR would lead to *it* being forward-ported, duplicating the PRs already created from *its* source. Add a check for PRs in the target branch with the same source, suppresses the forward-porting of the newly merged PR. Fixes #451 (hopefully)
2021-03-01 16:35:09 +07:00
ref = self[0]
base = all_sources[0]
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
target = all_targets[0]
[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 target is None:
_logger.info(
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
"Will not forward-port %s: no next target",
ref.display_name,
[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
)
return # QUESTION: do the prs need to be updated?
[FIX] forwardport: duplicates forwardport on edition On edition of an intermediate PR in a chain, merging the PR would lead to *it* being forward-ported, duplicating the PRs already created from *its* source. Add a check for PRs in the target branch with the same source, suppresses the forward-porting of the newly merged PR. Fixes #451 (hopefully)
2021-03-01 16:35:09 +07:00
# check if the PRs have already been forward-ported: is there a PR
# with the same source targeting the next branch in the series
for source in all_sources:
if self.search_count([('source_id', '=', source.id), ('target', '=', target.id)]):
_logger.info("Will not forward-port %s: already ported", ref.display_name)
return
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
# check if all PRs in the batch have the same "next target" , bail if
# that's not the case as it doesn't make sense for forward one PR from
# a to b and a linked pr from a to c
different_target = next((t for t in all_targets if t != target), None)
if different_target:
different_pr = next(p for p, t in zip(self, all_targets) if t == different_target)
for pr, t in zip(self, all_targets):
linked, other = different_pr, different_target
if t != target:
linked, other = ref, target
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': pr.repository.id,
'pull_request': pr.number,
'token_field': 'fp_github_token',
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
'message': "%sthis pull request can not be forward ported: "
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
"next branch is %r but linked pull request %s "
"has a next branch %r." % (
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
pr.ping(), t.name, linked.display_name, other.name
[ADD] forwardport: support for branch filtering Add handling of branch filtering to the forwardport module: * don't forward port (and trigger an error) when trying to port PRs to different next targets * otherwise port normally e.g. given a project with repos A and B and branches a, b and c, with branch b being excluded from repo B: * a PR merged into A.a will be forward-ported to A.b and A.c * a PR merged into B.a will be forward-ported to B.c (skipping the excluded B.b) * a PR set merged into (A.a, B.a) will *not* be forward-ported, and a message will be posted to each PR denoting the incompatibility
2020-01-22 21:41:42 +07:00
)
})
_logger.warning(
"Cancelling forward-port of %s: found different next branches (%s)",
self, all_targets
)
return
[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
proj = self.mapped('target.project_id')
if not proj.fp_github_token:
[IMP] forwardbot: reduce length of fw branch name * shorten the postfix, forwardbot is now a bigram! * shorten the uniquifier: go from 5 to 3 bytes, and use urlsafe base64 that way we only have a 4-char uniquifier instead of 8 * while at it, fix deprecated calls to logging.warn (should be logging.warning) Fixes #226
2019-10-10 16:36:14 +07:00
_logger.warning(
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
"Can not forward-port %s: no token on project %s",
ref.display_name,
[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
proj.name
)
return
notarget = [p.repository.name for p in self if not p.repository.fp_remote_target]
if notarget:
[IMP] runbot_merge, forwardport: update some logging Downgrade an error and a warning to info, and upgrade two warnings to error. Point is to improve logic of log levels & sentry visibility. Fixes #258
2021-01-13 21:48:39 +07:00
_logger.error(
[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
"Can not forward-port %s: repos %s don't have a remote configured",
self, ', '.join(notarget)
)
return
# take only the branch bit
[IMP] forwardbot: reduce length of fw branch name * shorten the postfix, forwardbot is now a bigram! * shorten the uniquifier: go from 5 to 3 bytes, and use urlsafe base64 that way we only have a 4-char uniquifier instead of 8 * while at it, fix deprecated calls to logging.warn (should be logging.warning) Fixes #226
2019-10-10 16:36:14 +07:00
new_branch = '%s-%s-%s-fw' % (
[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
target.name,
base.refname,
# avoid collisions between fp branches (labels can be reused
# or conflict especially as we're chopping off the owner)
[IMP] forwardbot: reduce length of fw branch name * shorten the postfix, forwardbot is now a bigram! * shorten the uniquifier: go from 5 to 3 bytes, and use urlsafe base64 that way we only have a 4-char uniquifier instead of 8 * while at it, fix deprecated calls to logging.warn (should be logging.warning) Fixes #226
2019-10-10 16:36:14 +07:00
base64.urlsafe_b64encode(os.urandom(3)).decode()
[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
)
# TODO: send outputs to logging?
conflicts = {}
with contextlib.ExitStack() as s:
for pr in self:
conflicts[pr], working_copy = pr._create_fp_branch(
target, new_branch, s)
working_copy.push('target', new_branch)
[FIX] forwardport: recover from failure to create forward port PR Currently if the creation of a forward-port pull request fails: * the branches are left un-cleaned * preceding PRs are left open * the PR whose creation failed may or may not have actually failed, and may or may not still be open We need to delete the forward port branches anyway, and IIRC that *should* automatically close the PR. Sadly making it so github predictably / reliably blows up when trying to create a PR via the API is difficult so this is essentially untestable. Closes #296
2020-01-29 21:41:13 +07:00
gh = requests.Session()
gh.headers['Authorization'] = 'token %s' % proj.fp_github_token
[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
has_conflicts = any(conflicts.values())
# problemo: this should forward port a batch at a time, if porting
# one of the PRs in the batch fails is huge problem, though this loop
# only concerns itself with the creation of the followup objects so...
new_batch = self.browse(())
for pr in self:
owner, _ = pr.repository.fp_remote_target.split('/', 1)
source = pr.source_id or pr
[IMP] forwardport: link FP PRs to both root and source In the case where an FP sequence is interrupted (e.g. there was a conflict during one of the intermediate steps), followups get linked to the original source but don't get linked to the "interruption" PR which is a bit confusing. Link FP PRs to both source and root if they're different.
2019-09-20 16:02:53 +07:00
root = pr._get_root()
[FIX] runbot_merge: ensure PR description is correct on merge Because sometimes github updates are missed (usually because github never triggers it), it's possible for the mergebot's view of a PR description to be incorrect. In that case, the PR may get merged with the wrong merge message entirely, through no fault of the user. Since we already fetch the PR info when staging it, there's very little overhead to checking that the PR message we store is correct then, and update it if it's not. This means the forward-port's description should also be correct. While at it, clean the forward port PR's creation a bit: - there should always be a message since the title is required on PRs (only the body can be missing), therefore no need to check that - as we're adding a bunch of pseudo-headers, there always is a body, no need for the condition - inline the `pr_data` and `URL`: they were extracted for the support of draft PRs, since that's been removed it's now unnecessary Fixes #530
2021-09-24 13:03:24 +07:00
message = source.message + '\n\n' + '\n'.join(
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
"Forward-Port-Of: %s" % p.display_name
[IMP] forwardport: link FP PRs to both root and source In the case where an FP sequence is interrupted (e.g. there was a conflict during one of the intermediate steps), followups get linked to the original source but don't get linked to the "interruption" PR which is a bit confusing. Link FP PRs to both source and root if they're different.
2019-09-20 16:02:53 +07:00
for p in root | source
)
[FIX] forwardport: PR message * avoid losing original PR message * add PR source to description
2019-09-10 14:54:25 +07:00
[FIX] forwardport: title-ing of PRs Due to the title formatting of FP PRs, we'd get incorrectly formatted commit messages if the PR was *merged* (rather than squashed / fast-forwarded) due to either "merge" or "rebase-merge" integration mode: in that case the PR message would be used as message for the merge commit and that'd be along the lines of "Forward Port of #xxx to <somebranch> (failed)", followed by the old PR message (e.g. see this commit message). * re-extract and reuse original PR title, just prefix with "[FW]" * finally add support for tagging, and use that to tag the PRs, especially for the failed / conflict marker which is quite important Closes #229
2019-10-11 18:05:36 +07:00
title, body = re.match(r'(?P<title>[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups()
[FIX] forwardport: race condition on PR creation (#357) e9e08fec3c2a64a74e7171b4a8ee5f989f1d8733 attempted to fix the issue but obviously failed as it still occurs: when creating a PR through the API, it's possible that the webhook gets triggered fast enough the transaction creating the PR from the webhook commits before we get around to creating our own PR from the API call. In which case the forward port process aborts. The process is re-run later on and generally succeeds fully, but we're left with a dangling PR we created but couldn't do anything with as its use broke. This issue seems to be getting more frequent so it's becoming quite important to fix it. Therefore we give our Raging Bull a Big Gun and now he has 20 attack *cough cough* we lock the bloody table down tight (only allow concurrent `SELECT`) until we've got the PR back and we've done the updates we need to it and nobody can mess with it... probably. This is not ideal as it's going to block updates to completely unrelated PRs but it doesn't seem like postgres really allows for locking out creations without locking out the rest, short of using advisory locks maybe? E.g. in the `create` override get a `pg_advisory_xact_lock_shared`, then get a `pg_advisory_xact_lock` in the forward-port process that way we're just blocking the concurrent creation of PRs during forward port, but creations don't block one another and we don't block updates. Application-level locks wouldn't really work as the 'bot could be deployed using multi-worker scenarios so we'd need cross-process locks or something. Hopefully fixes #352
2020-04-07 20:02:39 +07:00
self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE')
[FIX] runbot_merge: ensure PR description is correct on merge Because sometimes github updates are missed (usually because github never triggers it), it's possible for the mergebot's view of a PR description to be incorrect. In that case, the PR may get merged with the wrong merge message entirely, through no fault of the user. Since we already fetch the PR info when staging it, there's very little overhead to checking that the PR message we store is correct then, and update it if it's not. This means the forward-port's description should also be correct. While at it, clean the forward port PR's creation a bit: - there should always be a message since the title is required on PRs (only the body can be missing), therefore no need to check that - as we're adding a bunch of pseudo-headers, there always is a body, no need for the condition - inline the `pr_data` and `URL`: they were extracted for the support of draft PRs, since that's been removed it's now unnecessary Fixes #530
2021-09-24 13:03:24 +07:00
r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={
'base': target.name,
'head': f'{owner}:{new_branch}',
[IMP] forwardport: add missing space Instead of "[FW]Create README.md", the forwardport PR is now named "[FW] Create README.md"
2021-11-25 20:01:24 +07:00
'title': '[FW]' + (' ' if title[0] != '[' else '') + title,
[FIX] runbot_merge: ensure PR description is correct on merge Because sometimes github updates are missed (usually because github never triggers it), it's possible for the mergebot's view of a PR description to be incorrect. In that case, the PR may get merged with the wrong merge message entirely, through no fault of the user. Since we already fetch the PR info when staging it, there's very little overhead to checking that the PR message we store is correct then, and update it if it's not. This means the forward-port's description should also be correct. While at it, clean the forward port PR's creation a bit: - there should always be a message since the title is required on PRs (only the body can be missing), therefore no need to check that - as we're adding a bunch of pseudo-headers, there always is a body, no need for the condition - inline the `pr_data` and `URL`: they were extracted for the support of draft PRs, since that's been removed it's now unnecessary Fixes #530
2021-09-24 13:03:24 +07:00
'body': body
})
[IMP] forwardport: error handling when creating PR Don't try to parse the response as JSON in the error case(s): if the errors are bad enough github can return complete non-parseable garbage. Only access the "text" property (response body, decoded, but unparsed) in error cases, only parse in the success case. Also avoid reusing variables for completely different values, even if they're of the same type, especially if they can overlap. fixes #470
2021-07-27 20:49:49 +07:00
if not r.ok:
[FIX] forwardport: recover from failure to create forward port PR Currently if the creation of a forward-port pull request fails: * the branches are left un-cleaned * preceding PRs are left open * the PR whose creation failed may or may not have actually failed, and may or may not still be open We need to delete the forward port branches anyway, and IIRC that *should* automatically close the PR. Sadly making it so github predictably / reliably blows up when trying to create a PR via the API is difficult so this is essentially untestable. Closes #296
2020-01-29 21:41:13 +07:00
_logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name)
# delete all the branches this should automatically close the
# PRs if we've created any. Using the API here is probably
# simpler than going through the working copies
for repo in self.mapped('repository'):
[FIX] runbot_merge: ensure PR description is correct on merge Because sometimes github updates are missed (usually because github never triggers it), it's possible for the mergebot's view of a PR description to be incorrect. In that case, the PR may get merged with the wrong merge message entirely, through no fault of the user. Since we already fetch the PR info when staging it, there's very little overhead to checking that the PR message we store is correct then, and update it if it's not. This means the forward-port's description should also be correct. While at it, clean the forward port PR's creation a bit: - there should always be a message since the title is required on PRs (only the body can be missing), therefore no need to check that - as we're adding a bunch of pseudo-headers, there always is a body, no need for the condition - inline the `pr_data` and `URL`: they were extracted for the support of draft PRs, since that's been removed it's now unnecessary Fixes #530
2021-09-24 13:03:24 +07:00
d = gh.delete(f'https://api.github.com/repos/{repo.fp_remote_target}/git/refs/heads/{new_branch}')
[IMP] forwardport: error handling when creating PR Don't try to parse the response as JSON in the error case(s): if the errors are bad enough github can return complete non-parseable garbage. Only access the "text" property (response body, decoded, but unparsed) in error cases, only parse in the success case. Also avoid reusing variables for completely different values, even if they're of the same type, especially if they can overlap. fixes #470
2021-07-27 20:49:49 +07:00
if d.ok:
[FIX] forwardport: recover from failure to create forward port PR Currently if the creation of a forward-port pull request fails: * the branches are left un-cleaned * preceding PRs are left open * the PR whose creation failed may or may not have actually failed, and may or may not still be open We need to delete the forward port branches anyway, and IIRC that *should* automatically close the PR. Sadly making it so github predictably / reliably blows up when trying to create a PR via the API is difficult so this is essentially untestable. Closes #296
2020-01-29 21:41:13 +07:00
_logger.info("Deleting %s:%s=success", repo.fp_remote_target, new_branch)
else:
[IMP] forwardport: error handling when creating PR Don't try to parse the response as JSON in the error case(s): if the errors are bad enough github can return complete non-parseable garbage. Only access the "text" property (response body, decoded, but unparsed) in error cases, only parse in the success case. Also avoid reusing variables for completely different values, even if they're of the same type, especially if they can overlap. fixes #470
2021-07-27 20:49:49 +07:00
_logger.warning("Deleting %s:%s=%s", repo.fp_remote_target, new_branch, d.text)
raise RuntimeError("Forwardport failure: %s (%s)" % (pr.display_name, r.text))
[FIX] forwardport: recover from failure to create forward port PR Currently if the creation of a forward-port pull request fails: * the branches are left un-cleaned * preceding PRs are left open * the PR whose creation failed may or may not have actually failed, and may or may not still be open We need to delete the forward port branches anyway, and IIRC that *should* automatically close the PR. Sadly making it so github predictably / reliably blows up when trying to create a PR via the API is difficult so this is essentially untestable. Closes #296
2020-01-29 21:41:13 +07:00
[IMP] forwardport: error handling when creating PR Don't try to parse the response as JSON in the error case(s): if the errors are bad enough github can return complete non-parseable garbage. Only access the "text" property (response body, decoded, but unparsed) in error cases, only parse in the success case. Also avoid reusing variables for completely different values, even if they're of the same type, especially if they can overlap. fixes #470
2021-07-27 20:49:49 +07:00
new_pr = self._from_gh(r.json())
[FIX] forwardport: race condition on PR creation (#357) e9e08fec3c2a64a74e7171b4a8ee5f989f1d8733 attempted to fix the issue but obviously failed as it still occurs: when creating a PR through the API, it's possible that the webhook gets triggered fast enough the transaction creating the PR from the webhook commits before we get around to creating our own PR from the API call. In which case the forward port process aborts. The process is re-run later on and generally succeeds fully, but we're left with a dangling PR we created but couldn't do anything with as its use broke. This issue seems to be getting more frequent so it's becoming quite important to fix it. Therefore we give our Raging Bull a Big Gun and now he has 20 attack *cough cough* we lock the bloody table down tight (only allow concurrent `SELECT`) until we've got the PR back and we've done the updates we need to it and nobody can mess with it... probably. This is not ideal as it's going to block updates to completely unrelated PRs but it doesn't seem like postgres really allows for locking out creations without locking out the rest, short of using advisory locks maybe? E.g. in the `create` override get a `pg_advisory_xact_lock_shared`, then get a `pg_advisory_xact_lock` in the forward-port process that way we're just blocking the concurrent creation of PRs during forward port, but creations don't block one another and we don't block updates. Application-level locks wouldn't really work as the 'bot could be deployed using multi-worker scenarios so we'd need cross-process locks or something. Hopefully fixes #352
2020-04-07 20:02:39 +07:00
_logger.info("Created forward-port PR %s", new_pr)
[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_batch |= new_pr
[FIX] forwardport: allow delegates on fw PRs to review followups The forward-port process currently automatically adds delegates of a PR as delegates on its forward ports, but that only works for the *source* pull request. If a delegate is added to a forward-port, they were not able to approve the followups to that initial port, which makes little sense. Fixes #548
2021-10-18 16:46:14 +07:00
# allows PR author to close or skipci
[IMP] fwbot: warning on r+ for failed PR approving a PR which failed CI should trigger a feedback message since 6cb58a322de0fa600f80eee62b0c4eda43113810 (#158), the code has not been removed and the tests still pass. However fwbot r+ would go through its own process for r+ which would explain why that feedback is sometimes gone / lost (cf #327 and #336). * make fwbot r+ delegate to mergebot r+ * add dedicated logging for this operation to better analyze post-mortem * automatically ping the reviewer to specifically tell them they're idiots * move the feedback item out of the state change bit, send it even if it's a useless r+ (because it's already r+'d) * add a test for forward-ports Closes #327, closes #336
2020-03-05 19:31:23 +07:00
source.delegates |= source.author
[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_pr.write({
'merge_method': pr.merge_method,
'source_id': source.id,
# only link to previous PR of sequence if cherrypick passed
'parent_id': pr.id if not has_conflicts else False,
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
'detach_reason': "conflicts: {}".format(
f'\n{conflicts[pr]}\n{conflicts[pr]}'.strip()
) if has_conflicts else None,
[FIX] forwardport: allow delegates on fw PRs to review followups The forward-port process currently automatically adds delegates of a PR as delegates on its forward ports, but that only works for the *source* pull request. If a delegate is added to a forward-port, they were not able to approve the followups to that initial port, which makes little sense. Fixes #548
2021-10-18 16:46:14 +07:00
# Copy author & delegates of source as well as delegates of
# previous so they can r+ the new forward ports.
'delegates': [(6, False, (source.delegates | pr.delegates).ids)]
[CHG] forwardport: automatically delegate original PR author on FP PRs This way the original author can r+ the forward ports if they succeed (and probably requires no attention). Closes #195
2019-09-17 19:43:21 +07:00
})
[IMP] forwardport: on conflict note previous FP can be r+'d Closes #294
2020-03-12 14:33:15 +07:00
if has_conflicts and pr.parent_id and pr.state not in ('merged', 'closed'):
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
message = source.ping() + """\
the next pull request (%s) is in conflict. You can merge the chain up to here by saying
[IMP] forwardport: on conflict note previous FP can be r+'d Closes #294
2020-03-12 14:33:15 +07:00
> @%s r+
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
%s""" % (new_pr.display_name, pr.repository.project_id.fp_github_name, footer)
[IMP] forwardport: on conflict note previous FP can be r+'d Closes #294
2020-03-12 14:33:15 +07:00
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': pr.repository.id,
'pull_request': pr.number,
'message': message,
'token_field': 'fp_github_token',
})
[IMP] forwardport: warning message when co-dep PR has a conflict * Add some more information as to why the user *should* do on the PR the message is printed on, the previous message left that to their imagination * The PR selection was *completely* wrong as it would select the old PRs which really isn't what we want. And turns out there's no good reason to create & send the feedback in the loop creating the forward-port prs, that can be moved to a followup loop where we have created hopefully created all the forward-port PRs. Also technically we could do even better than currently and remap the prs mapped to conflict data to the new PRs and know exactly which of the forward-ported PRs is faulty, but that seems overkill for now.
2020-01-30 19:56:01 +07:00
# not great but we probably want to avoid the risk of the webhook
# creating the PR from under us. There's still a "hole" between
# the POST being executed on gh and the commit but...
self.env.cr.commit()
for pr, new_pr in zip(self, new_batch):
source = pr.source_id or pr
[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
(h, out, err, hh) = conflicts.get(pr) or (None, None, None, None)
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
if h:
sout = serr = ''
if out.strip():
[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
sout = f"\nstdout:\n```\n{out}\n```\n"
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
if err.strip():
[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
serr = f"\nstderr:\n```\n{err}\n```\n"
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
[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
lines = ''
if len(hh) > 1:
lines = '\n' + ''.join(
'* %s%s\n' % (sha, ' <- on this commit' if sha == h else '')
for sha in hh
)
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
message = f"""{source.ping()}cherrypicking of pull request {source.display_name} failed.
[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
{lines}{sout}{serr}
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
In the former case, you may want to edit this PR message as well.
[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
"""
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
elif has_conflicts:
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
message = """%s\
while this was properly forward-ported, at least one co-dependent PR (%s) did \
not succeed. You will need to fix it before this can be merged.
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
Both this PR and the others will need to be approved via `@%s r+` as they are \
all considered "in conflict".
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
%s""" % (
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
source.ping(),
[FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367
2020-05-19 19:13:21 +07:00
', '.join(p.display_name for p in (new_batch - new_pr)),
proj.github_prefix,
footer
)
[FIX] forwardport: not being properly notified on last FP of a seq If the default limit of a forward-port sequence is not a valid target (either disabled or not actually forward-ported to), the last effective forward port in a sequence will be commented on as any intermediate PR rather than get a proper ping and r+ instructions. Also remove a bunch of leftover prints in the tests. Fixes #192
2019-09-13 21:06:34 +07:00
elif base._find_next_target(new_pr) is None:
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
ancestors = "".join(
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
"* %s\n" % p.display_name
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
for p in pr._iter_ancestors()
[FIX] forwardbot: selection of ancestor in FP tail ping In selecting the parent commits to list on the last PR, we would miss the *first* forward-port of the sequence. Not sure why we added a detrimental check on source_id there. Also add a missing space between "chain" and "containing" in the case where there's at least one forward-port PR other than the final one. Fixes #212
2019-09-27 19:14:07 +07:00
if p.parent_id
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
)
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
message = source.ping() + """\
this PR targets %s and is the last of the forward-port chain%s
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
%s
To merge the full chain, say
> @%s r+
[IMP] forwardport: warn when linked PR failed to FW Before this change, if multiple co-dependent PRs get forward-ported and one of them has a conflict the notice on the others is very limited: they're tagged as `conflict` but there is no other information provided in the PR description or in the subsequent message. Add a small warning to these other PRs, for clarity. Closes #302
2020-01-29 21:55:06 +07:00
%s""" % (target.name, ' containing:' if ancestors else '.', ancestors, pr.repository.project_id.fp_github_name, footer)
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
else:
message = """\
This PR targets %s and is part of the forward-port chain. Further PRs will be created up to %s.
[IMP] forwardport: warn when linked PR failed to FW Before this change, if multiple co-dependent PRs get forward-ported and one of them has a conflict the notice on the others is very limited: they're tagged as `conflict` but there is no other information provided in the PR description or in the subsequent message. Add a small warning to these other PRs, for clarity. Closes #302
2020-01-29 21:55:06 +07:00
%s""" % (target.name, base.limit_id.name, footer)
[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
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': new_pr.repository.id,
'pull_request': new_pr.number,
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
'message': message,
[IMP] runbot_merge, forwardport: variable-user feedback Having all the feedback be sent by the mergebot user (github_token) is confusing. Add a way to specify which field of project should be used to source the token used when sending feedback. Fixes #190
2019-09-18 20:37:14 +07:00
'token_field': 'fp_github_token',
[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
})
[FIX] forwardport: title-ing of PRs Due to the title formatting of FP PRs, we'd get incorrectly formatted commit messages if the PR was *merged* (rather than squashed / fast-forwarded) due to either "merge" or "rebase-merge" integration mode: in that case the PR message would be used as message for the merge commit and that'd be along the lines of "Forward Port of #xxx to <somebranch> (failed)", followed by the old PR message (e.g. see this commit message). * re-extract and reuse original PR title, just prefix with "[FW]" * finally add support for tagging, and use that to tag the PRs, especially for the failed / conflict marker which is quite important Closes #229
2019-10-11 18:05:36 +07:00
labels = ['forwardport']
if has_conflicts:
labels.append('conflict')
[IMP] runbot_merge, forwardport: unify tagging Genericise runbot_merge's tagging (move states to the "UI" but only store / manage actual tags), and remove forwardport.tagging as it's now redundant. Closes #232
2020-03-10 19:36:46 +07:00
self.env['runbot_merge.pull_requests.tagging'].create({
[FIX] forwardport: title-ing of PRs Due to the title formatting of FP PRs, we'd get incorrectly formatted commit messages if the PR was *merged* (rather than squashed / fast-forwarded) due to either "merge" or "rebase-merge" integration mode: in that case the PR message would be used as message for the merge commit and that'd be along the lines of "Forward Port of #xxx to <somebranch> (failed)", followed by the old PR message (e.g. see this commit message). * re-extract and reuse original PR title, just prefix with "[FW]" * finally add support for tagging, and use that to tag the PRs, especially for the failed / conflict marker which is quite important Closes #229
2019-10-11 18:05:36 +07:00
'repository': new_pr.repository.id,
'pull_request': new_pr.number,
[IMP] runbot_merge, forwardport: unify tagging Genericise runbot_merge's tagging (move states to the "UI" but only store / manage actual tags), and remove forwardport.tagging as it's now redundant. Closes #232
2020-03-10 19:36:46 +07:00
'tags_add': labels,
[FIX] forwardport: title-ing of PRs Due to the title formatting of FP PRs, we'd get incorrectly formatted commit messages if the PR was *merged* (rather than squashed / fast-forwarded) due to either "merge" or "rebase-merge" integration mode: in that case the PR message would be used as message for the merge commit and that'd be along the lines of "Forward Port of #xxx to <somebranch> (failed)", followed by the old PR message (e.g. see this commit message). * re-extract and reuse original PR title, just prefix with "[FW]" * finally add support for tagging, and use that to tag the PRs, especially for the failed / conflict marker which is quite important Closes #229
2019-10-11 18:05:36 +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
# batch the PRs so _validate can perform the followup FP properly
# (with the entire batch). If there are conflict then create a
# deactivated batch so the interface is coherent but we don't pickup
# an active batch we're never going to deactivate.
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
b = self.env['runbot_merge.batch'].create({
[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
'target': target.id,
'prs': [(6, 0, new_batch.ids)],
'active': not has_conflicts,
})
[ADD] forwardport: ability to skip CI when forward porting Provides a `skipci` command to PR reviewers. This makes it so the followup PRs (after the first one) get created immediately, without waiting for CI to succeed on a given forward-port PR. This can be useful if for some reason a change *must* be merged in branch N+1 before it can be merged in branch N. Fixes #363
2020-04-16 17:42:01 +07:00
# if we're not waiting for CI, schedule followup immediately
if any(p.source_id.fw_policy == 'skipci' for p in b.prs):
b.prs[0]._schedule_fp_followup()
return b
[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
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
@property
def _source_url(self):
return 'https://{}:{}@github.com/{}'.format(
self.repository.project_id.fp_github_name or '',
self.repository.project_id.fp_github_token,
self.repository.name,
)
[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 _create_fp_branch(self, target_branch, fp_branch_name, cleanup):
""" Creates a forward-port for the current PR to ``target_branch`` under
``fp_branch_name``.
:param target_branch: the branch to port forward to
:param fp_branch_name: the name of the branch to create the FP under
:param ExitStack cleanup: so the working directories can be cleaned up
[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
:return: A pair of an optional conflict information and a repository. If
present the conflict information is composed of the hash of the
conflicting commit, the stderr and stdout of the failed
cherrypick and a list of all PR commit hashes
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
:rtype: (None | (str, str, str, list[commit]), Repo)
[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
"""
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
logger = _logger.getChild(str(self.id))
[IMP] forwardport: check commit liveness on the PR we're porting from Check remains a tad dodgy, but since we're actually porting from `root` (the earliest ancestor of `self`) it makes more sense to sanity-check that *its* commits remain visible. Also log that as it makes a tad more sense, hopefully. Closes #600
2022-06-30 17:16:30 +07:00
root = self._get_root()
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
logger.info(
"Forward-porting %s (%s) to %s",
self.display_name, root.display_name, target_branch.name
)
source = self._get_local_directory()
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
r = source.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT).fetch()
logger.info("Updated cache repo %s:\n%s", source._directory, r.stdout.decode())
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
logger.info("Create working copy...")
[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
working_copy = source.clone(
[IMP] forwardport: move working copy to the cache dir Working copies were created in tempdir under the assumption that they're, well, temporary. However after thinking about it more there are two issues with this: * tempdirs might not be in the same FS as the cache dir, meaning meaning `git clone` can't hardlink the repo objects and has to copy them * tempdirs are often on RAM-backed tmpfs, which is not great when we're filling them with multiple GB worth of git repository...
2019-09-16 18:16:09 +07:00
cleanup.enter_context(
tempfile.TemporaryDirectory(
[IMP] forwardport: check commit liveness on the PR we're porting from Check remains a tad dodgy, but since we're actually porting from `root` (the earliest ancestor of `self`) it makes more sense to sanity-check that *its* commits remain visible. Also log that as it makes a tad more sense, hopefully. Closes #600
2022-06-30 17:16:30 +07:00
prefix='%s-to-%s-' % (
root.display_name,
[IMP] forwardport: move working copy to the cache dir Working copies were created in tempdir under the assumption that they're, well, temporary. However after thinking about it more there are two issues with this: * tempdirs might not be in the same FS as the cache dir, meaning meaning `git clone` can't hardlink the repo objects and has to copy them * tempdirs are often on RAM-backed tmpfs, which is not great when we're filling them with multiple GB worth of git repository...
2019-09-16 18:16:09 +07:00
target_branch.name
),
dir=user_cache_dir('forwardport')
)),
[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
branch=target_branch.name
)
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
r = working_copy.with_config(stdout=subprocess.PIPE, stderr=subprocess.STDOUT) \
.fetch(self._source_url, root.head)
logger.info(
"Fetched head of %s into %s:\n%s",
root.display_name,
working_copy._directory,
r.stdout.decode()
)
if working_copy.check(False).cat_file(e=root.head).returncode:
raise ForwardPortError(
f"During forward port of {self.display_name}, unable to find "
f"expected head of {root.display_name} ({root.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
project_id = self.repository.project_id
# add target remote
working_copy.remote(
'add', 'target',
'https://{p.fp_github_name}:{p.fp_github_token}@github.com/{r.fp_remote_target}'.format(
r=self.repository,
p=project_id
)
)
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
logger.info("Create FP branch %s in %s", fp_branch_name, working_copy._directory)
[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
working_copy.checkout(b=fp_branch_name)
try:
root._cherry_pick(working_copy)
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
return None, working_copy
[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
except CherrypickError as e:
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
h, out, err, commits = e.args
[FIX] forwardport: change method for conflicting working copy The original method was to `git diff | git apply` in order to get a complete overview of conflicts generated by the forward port (if any). However this turns out to have a huge issue in the presence of renamed or removed files: in that case `git apply` will simply not do anything, and fail with a completely clean working copy. Which is very much undesirable. -> alternative method, squash the PR to a single commit then cherry-pick that single commit, this should provide us with proper conflicts & their markers. Also add tests for conflicts due to deleted files...
2019-09-11 20:04:19 +07:00
# using git diff | git apply -3 to get the entire conflict set
# turns out to not work correctly: in case files have been moved
# / removed (which turns out to be a common source of conflicts
# when forward-porting) it'll just do nothing to the working copy
# so the "conflict commit" will be empty
# switch to a squashed-pr branch
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
working_copy.check(True).checkout('-bsquashed', root.head)
[FIX] forwardport: fix deduplication of authorship in multi-pr conflict a45f7260fa36acfdcca1e03c3f388f4f93e96e07 had intended to use the original authorship information for conflict commit even if there were multiple commits, as long as there was only one author (/ committer) for the entire sequence. Sadly the deduplication was buggy as it took the *authorship date* in account, which basically ensured commits would never be considered as having the same authorship outside of tests (where it was possible for commits to be created at the same second). Related to #505
2021-08-02 14:18:30 +07:00
# commits returns oldest first, so youngest (head) last
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
head_commit = commits[-1]['commit']
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
[FIX] forwardport: fix deduplication of authorship in multi-pr conflict a45f7260fa36acfdcca1e03c3f388f4f93e96e07 had intended to use the original authorship information for conflict commit even if there were multiple commits, as long as there was only one author (/ committer) for the entire sequence. Sadly the deduplication was buggy as it took the *authorship date* in account, which basically ensured commits would never be considered as having the same authorship outside of tests (where it was possible for commits to be created at the same second). Related to #505
2021-08-02 14:18:30 +07:00
to_tuple = operator.itemgetter('name', 'email')
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
to_dict = lambda term, vals: {
'GIT_%s_NAME' % term: vals[0],
'GIT_%s_EMAIL' % term: vals[1],
'GIT_%s_DATE' % term: vals[2],
}
authors, committers = set(), set()
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
for c in (c['commit'] for c in commits):
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
authors.add(to_tuple(c['author']))
committers.add(to_tuple(c['committer']))
[FIX] *: prevent merging conflicts commits with loss of authorship Proper attribution is important in general, but especially for external contributors. Before this change, and the previous change fixing authorship deduplication, it was rather easy for a "squashed" conflict commit (attributed to the 'bot for lack of a really clean option) to get merged by mistake. This commit changes two things: * The mergebot now refuses to stage commits without an email set, the github API rejects those commits anyway so any integration mode other than `merge` would fail, just with a very unclear error * The forwardbot now creates commits with an empty author / committer email when the pull request as a whole has multiple authors / committers. This leverages the mergebot update. Also clean up the staging process to provide richer error reporting using bespoke exceptions instead of simple assertions. I'm not sure we've ever encountered most of these errors so they're really sanity checks but the old reporting would be... less than great. Fixes #505
2021-08-03 18:45:21 +07:00
fp_authorship = (project_id.fp_github_name, '', '')
[FIX] forwardport: fix deduplication of authorship in multi-pr conflict a45f7260fa36acfdcca1e03c3f388f4f93e96e07 had intended to use the original authorship information for conflict commit even if there were multiple commits, as long as there was only one author (/ committer) for the entire sequence. Sadly the deduplication was buggy as it took the *authorship date* in account, which basically ensured commits would never be considered as having the same authorship outside of tests (where it was possible for commits to be created at the same second). Related to #505
2021-08-02 14:18:30 +07:00
author = fp_authorship if len(authors) != 1\
else authors.pop() + (head_commit['author']['date'],)
committer = fp_authorship if len(committers) != 1 \
else committers.pop() + (head_commit['committer']['date'],)
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
conf = working_copy.with_config(env={
**to_dict('AUTHOR', author),
**to_dict('COMMITTER', committer),
[FIX] forwardport: don't keep the commit date This is not super useful and causes issues with runbot as it uses commit dates to decide how "old" branches are, and ignores (doesn't create when it scans them) branches more than a month old. So the forwardport branches will be completely ignored (not considered let alone tested) if we happen to forward-port a PR last updated more than a month ago. Which is somewhat inconvenient. Closes #274
2019-12-19 21:25:23 +07:00
'GIT_COMMITTER_DATE': '',
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
})
[IMP] forwardport: unweird singleton conflicting commits When creating the conflicting commit of a single commit PR, reuse the original commit's meta-information so the developer / fixer can more easily update it in-place. Closes #204
2019-09-20 21:07:22 +07:00
# squash to a single commit
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
conf.reset('--soft', commits[0]['parents'][0]['sha'])
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
conf.commit(a=True, message="temp")
squashed = conf.stdout().rev_parse('HEAD').stdout.strip().decode()
[FIX] forwardport: change method for conflicting working copy The original method was to `git diff | git apply` in order to get a complete overview of conflicts generated by the forward port (if any). However this turns out to have a huge issue in the presence of renamed or removed files: in that case `git apply` will simply not do anything, and fail with a completely clean working copy. Which is very much undesirable. -> alternative method, squash the PR to a single commit then cherry-pick that single commit, this should provide us with proper conflicts & their markers. Also add tests for conflicts due to deleted files...
2019-09-11 20:04:19 +07:00
# switch back to the PR branch
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
conf.checkout(fp_branch_name)
# cherry-pick the squashed commit to generate the conflict
[FIX] forwardport: use diff3 for conflict style Existing conflict style is the local default ("merge", most likely). `diff3` is a lot more informative as it provides the common ancestor's code for the hunk, which helps see how the two branches diverged and thus resolve the conflict. Even better would be zdiff3 but that's a bit too recent... Fixes #619
2022-06-24 20:57:45 +07:00
conf.with_params('merge.renamelimit=0', 'merge.conflictstyle=diff3')\
[IMP] forwardport: logging during cherrypick process * fix small error which generated an extra commit in case of conflict probably (would create a `temp` commit, then put the conflict information on an empty commit on top of it). Avoids committing the cherrypick though a probable alternative would be to commit the message with `amend`. * improve the cherrypick header: instead of one log line per commit, put all commits within the sole header log line (with a newline), should make things less noisy * put *all* log records below the correct logger (two of them were on the toplevel logger) * log a purported inflateInit error as warning, should be sent to Sentry instead of having to wait for people to wonder why the thing is completely broken
2021-01-15 20:19:15 +07:00
.with_config(check=False)\
.cherry_pick(squashed, no_commit=True)
[IMP] forwardport: feedback on conflict append `git status` data to stderr, should be somewhat more informative especially when a conflict is a DU (where the file has been deleted on one side, so there is no conflict marker anywhere). Fixes #461
2021-03-01 15:44:18 +07:00
status = conf.stdout().status(short=True, untracked_files='no').stdout.decode()
if err.strip():
err = err.rstrip() + '\n----------\nstatus:\n' + status
else:
err = 'status:\n' + status
[IMP] forwardport: unweird singleton conflicting commits When creating the conflicting commit of a single commit PR, reuse the original commit's meta-information so the developer / fixer can more easily update it in-place. Closes #204
2019-09-20 21:07:22 +07:00
# if there was a single commit, reuse its message when committing
# the conflict
# TODO: still add conflict information to this?
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
if len(commits) == 1:
msg = root._make_fp_message(commits[0])
conf.with_config(input=str(msg).encode()) \
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
.commit(all=True, allow_empty=True, file='-')
[IMP] forwardport: unweird singleton conflicting commits When creating the conflicting commit of a single commit PR, reuse the original commit's meta-information so the developer / fixer can more easily update it in-place. Closes #204
2019-09-20 21:07:22 +07:00
else:
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
conf.commit(
[IMP] forwardport: unweird singleton conflicting commits When creating the conflicting commit of a single commit PR, reuse the original commit's meta-information so the developer / fixer can more easily update it in-place. Closes #204
2019-09-20 21:07:22 +07:00
all=True, allow_empty=True,
message="""Cherry pick of %s failed
[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
stdout:
%s
stderr:
%s
[IMP] forwardport: feedback on conflict append `git status` data to stderr, should be somewhat more informative especially when a conflict is a DU (where the file has been deleted on one side, so there is no conflict marker anywhere). Fixes #461
2021-03-01 15:44:18 +07:00
""" % (h, out, err))
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
return (h, out, err, [c['sha'] for c in commits]), working_copy
[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 _cherry_pick(self, working_copy):
""" Cherrypicks ``self`` into the working copy
:return: ``True`` if the cherrypick was successful, ``False`` otherwise
"""
# <xxx>.cherrypick.<number>
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
logger = _logger.getChild(str(self.id)).getChild('cherrypick')
[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 head so we can reset
[FIX] forwardport: broken fp if intermediate commit requires renamelimit=0 On a cherrypick failing due to renamelimit issues, the cherrypick would be retried after resetting the target to its *original* commit. This only works correctly if the first commit works after a retry, if a latter commit has to be retried then it gets re-cherry-picked onto the target branch rather than its own parent. Fix by remembering the previously successful cherry-picked commit and resetting to *that*. However I can't really test it because it's rather hard to get into a situation where the rename detection fails using synthetic tests. While at it, clean the logs by stripping the "performing inexact rename detection" stuff from all stderr (both the CherrypickError from which it was already stripped and the debug messages). Fixes #444
2021-01-11 20:43:36 +07:00
prev = original_head = working_copy.stdout().rev_parse('HEAD').stdout.decode().strip()
[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
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
commits = self.commits()
[IMP] forwardport: logging during cherrypick process * fix small error which generated an extra commit in case of conflict probably (would create a `temp` commit, then put the conflict information on an empty commit on top of it). Avoids committing the cherrypick though a probable alternative would be to commit the message with `amend`. * improve the cherrypick header: instead of one log line per commit, put all commits within the sole header log line (with a newline), should make things less noisy * put *all* log records below the correct logger (two of them were on the toplevel logger) * log a purported inflateInit error as warning, should be sent to Sentry instead of having to wait for people to wonder why the thing is completely broken
2021-01-15 20:19:15 +07:00
logger.info("%s: copy %s commits to %s\n%s", self, len(commits), original_head, '\n'.join(
'- %s (%s)' % (c['sha'], c['commit']['message'].splitlines()[0])
for c in commits
))
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +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
for commit in commits:
[FIX] forwardport: ask github what the PR's commits are Attempts to use rev-list seem to work locally but they fail *a lot* when live. The previous attempt to fix it in 0f4c22490c2b90285ca82889a5652b54c06c65ea made things worse rather than better once deployed. Give up on that (at least for now?), and just ask github what the PR's commits are then cherrypick exactly that.
2019-09-12 16:58:10 +07:00
commit_sha = commit['sha']
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
# config (global -c) or commit options don't really give access to
# setting dates
cm = commit['commit'] # get the "git" commit object rather than the "github" commit resource
[IMP] forwardport: cherrypick logging Enable GIT_TRACE to try and get more information pertaining to the cherrypicking process during failure. Related to #449
2021-01-18 13:37:59 +07:00
env = {
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
'GIT_AUTHOR_NAME': cm['author']['name'],
'GIT_AUTHOR_EMAIL': cm['author']['email'],
'GIT_AUTHOR_DATE': cm['author']['date'],
'GIT_COMMITTER_NAME': cm['committer']['name'],
'GIT_COMMITTER_EMAIL': cm['committer']['email'],
[IMP] forwardport: cherrypick logging Enable GIT_TRACE to try and get more information pertaining to the cherrypicking process during failure. Related to #449
2021-01-18 13:37:59 +07:00
}
configured = working_copy.with_config(env=env)
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
[IMP] forwardport: cherrypick logging Enable GIT_TRACE to try and get more information pertaining to the cherrypicking process during failure. Related to #449
2021-01-18 13:37:59 +07:00
conf = working_copy.with_config(
env={**env, 'GIT_TRACE': 'true'},
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
check=False
)
[FIX] forwardport: attempt no renamelimit & filter out messages * a high renamelimit increases the cherrypick runtime and can apparently fail for reasons other than actual conflicts (cf odoo/odoo#36893) so first attempt to cherrypick with the default renamelimit, and fallback to renamelimit=0 if that fails * in that case, filter out the spam of "progress" messages about inexact rename detection as it's not really useful. There seems to be no built-in way to suppress these. Closes #196
2019-09-13 18:13:58 +07:00
# first try with default / low renamelimit
r = conf.cherry_pick(commit_sha)
[IMP] forwardport: logging during cherrypick process * fix small error which generated an extra commit in case of conflict probably (would create a `temp` commit, then put the conflict information on an empty commit on top of it). Avoids committing the cherrypick though a probable alternative would be to commit the message with `amend`. * improve the cherrypick header: instead of one log line per commit, put all commits within the sole header log line (with a newline), should make things less noisy * put *all* log records below the correct logger (two of them were on the toplevel logger) * log a purported inflateInit error as warning, should be sent to Sentry instead of having to wait for people to wonder why the thing is completely broken
2021-01-15 20:19:15 +07:00
logger.debug("Cherry-picked %s: %s\n%s\n%s", commit_sha, r.returncode, r.stdout.decode(), _clean_rename(r.stderr.decode()))
[FIX] forwardport: attempt no renamelimit & filter out messages * a high renamelimit increases the cherrypick runtime and can apparently fail for reasons other than actual conflicts (cf odoo/odoo#36893) so first attempt to cherrypick with the default renamelimit, and fallback to renamelimit=0 if that fails * in that case, filter out the spam of "progress" messages about inexact rename detection as it's not really useful. There seems to be no built-in way to suppress these. Closes #196
2019-09-13 18:13:58 +07:00
if r.returncode:
# if it failed, retry with high renamelimit
[FIX] forwardport: broken fp if intermediate commit requires renamelimit=0 On a cherrypick failing due to renamelimit issues, the cherrypick would be retried after resetting the target to its *original* commit. This only works correctly if the first commit works after a retry, if a latter commit has to be retried then it gets re-cherry-picked onto the target branch rather than its own parent. Fix by remembering the previously successful cherry-picked commit and resetting to *that*. However I can't really test it because it's rather hard to get into a situation where the rename detection fails using synthetic tests. While at it, clean the logs by stripping the "performing inexact rename detection" stuff from all stderr (both the CherrypickError from which it was already stripped and the debug messages). Fixes #444
2021-01-11 20:43:36 +07:00
configured.reset('--hard', prev)
[FIX] forwardport: attempt no renamelimit & filter out messages * a high renamelimit increases the cherrypick runtime and can apparently fail for reasons other than actual conflicts (cf odoo/odoo#36893) so first attempt to cherrypick with the default renamelimit, and fallback to renamelimit=0 if that fails * in that case, filter out the spam of "progress" messages about inexact rename detection as it's not really useful. There seems to be no built-in way to suppress these. Closes #196
2019-09-13 18:13:58 +07:00
r = conf.with_params('merge.renamelimit=0').cherry_pick(commit_sha)
[IMP] forwardport: logging during cherrypick process * fix small error which generated an extra commit in case of conflict probably (would create a `temp` commit, then put the conflict information on an empty commit on top of it). Avoids committing the cherrypick though a probable alternative would be to commit the message with `amend`. * improve the cherrypick header: instead of one log line per commit, put all commits within the sole header log line (with a newline), should make things less noisy * put *all* log records below the correct logger (two of them were on the toplevel logger) * log a purported inflateInit error as warning, should be sent to Sentry instead of having to wait for people to wonder why the thing is completely broken
2021-01-15 20:19:15 +07:00
logger.debug("Cherry-picked %s (renamelimit=0): %s\n%s\n%s", commit_sha, r.returncode, r.stdout.decode(), _clean_rename(r.stderr.decode()))
[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 r.returncode: # pick failed, reset and bail
[IMP] forwardport: logging during cherrypick process * fix small error which generated an extra commit in case of conflict probably (would create a `temp` commit, then put the conflict information on an empty commit on top of it). Avoids committing the cherrypick though a probable alternative would be to commit the message with `amend`. * improve the cherrypick header: instead of one log line per commit, put all commits within the sole header log line (with a newline), should make things less noisy * put *all* log records below the correct logger (two of them were on the toplevel logger) * log a purported inflateInit error as warning, should be sent to Sentry instead of having to wait for people to wonder why the thing is completely broken
2021-01-15 20:19:15 +07:00
# try to log inflateInit: out of memory errors as warning, they
# seem to return the status code 128
logger.log(
logging.WARNING if r.returncode == 128 else logging.INFO,
"forward-port of %s (%s) failed at %s",
self, self.display_name, commit_sha)
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
configured.reset('--hard', original_head)
[FIX] forwardport: attempt no renamelimit & filter out messages * a high renamelimit increases the cherrypick runtime and can apparently fail for reasons other than actual conflicts (cf odoo/odoo#36893) so first attempt to cherrypick with the default renamelimit, and fallback to renamelimit=0 if that fails * in that case, filter out the spam of "progress" messages about inexact rename detection as it's not really useful. There seems to be no built-in way to suppress these. Closes #196
2019-09-13 18:13:58 +07:00
raise CherrypickError(
commit_sha,
r.stdout.decode(),
[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
_clean_rename(r.stderr.decode()),
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
commits
[FIX] forwardport: attempt no renamelimit & filter out messages * a high renamelimit increases the cherrypick runtime and can apparently fail for reasons other than actual conflicts (cf odoo/odoo#36893) so first attempt to cherrypick with the default renamelimit, and fallback to renamelimit=0 if that fails * in that case, filter out the spam of "progress" messages about inexact rename detection as it's not really useful. There seems to be no built-in way to suppress these. Closes #196
2019-09-13 18:13:58 +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
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
msg = self._make_fp_message(commit)
[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
# replace existing commit message with massaged one
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
configured \
[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
.with_config(input=str(msg).encode())\
.commit(amend=True, file='-')
[FIX] forwardport: broken fp if intermediate commit requires renamelimit=0 On a cherrypick failing due to renamelimit issues, the cherrypick would be retried after resetting the target to its *original* commit. This only works correctly if the first commit works after a retry, if a latter commit has to be retried then it gets re-cherry-picked onto the target branch rather than its own parent. Fix by remembering the previously successful cherry-picked commit and resetting to *that*. However I can't really test it because it's rather hard to get into a situation where the rename detection fails using synthetic tests. While at it, clean the logs by stripping the "performing inexact rename detection" stuff from all stderr (both the CherrypickError from which it was already stripped and the debug messages). Fixes #444
2021-01-11 20:43:36 +07:00
prev = configured.stdout().rev_parse('HEAD').stdout.decode()
logger.info('%s: success -> %s', commit_sha, prev)
[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
[IMP] runbot_merge, forwardport: better signed-off-by handling Remove original-signed-off-by, doesn't actually seem useful given the semantics of signed-off-by according to the kernel doc'. Plus it didn't actually work as the intent was to keep the signoff of the original PR in the forward-port, but that signoff is not part of the commit we're cherrypicking (it gets added on the fly when the commit is merged). Therefore explicitly get the ack-chain into the PR: when merging an FP PR, try to integrate the signoff of the original PR, that of the final FP pr, and while at it that of the last explicit update in the commit chain (e.g. in case there's been a conflict or something). Fixes #284
2020-03-02 14:54:58 +07:00
def _build_merge_message(self, message, related_prs=()):
msg = super()._build_merge_message(message, related_prs=related_prs)
# ensures all reviewers in the review path are on the PR in order:
# original reviewer, then last conflict reviewer, then current PR
reviewers = (self | self._get_root() | self.source_id)\
.mapped('reviewed_by.formatted_email')
sobs = msg.headers.getlist('signed-off-by')
msg.headers.remove('signed-off-by')
msg.headers.extend(
('signed-off-by', signer)
for signer in sobs
if signer not in reviewers
)
msg.headers.extend(
('signed-off-by', reviewer)
for reviewer in reversed(reviewers)
)
return msg
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
def _make_fp_message(self, commit):
cmap = json.loads(self.commits_map)
msg = self._parse_commit_message(commit['commit']['message'])
# write the *merged* commit as "original", not the PR's
msg.headers['x-original-commit'] = cmap.get(commit['sha'], commit['sha'])
# don't stringify so caller can still perform alterations
return msg
[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 _get_local_directory(self):
repos_dir = pathlib.Path(user_cache_dir('forwardport'))
repos_dir.mkdir(parents=True, exist_ok=True)
repo_dir = repos_dir / self.repository.name
if repo_dir.is_dir():
return git(repo_dir)
else:
[FIX] forwardport: clone op and add logging
2019-09-10 14:28:30 +07:00
_logger.info("Cloning out %s to %s", self.repository.name, repo_dir)
[FIX] forwardport: storage of old garbage, repo cache sizes Since the forwardport bot works off of PRs, when it was created leveraging the magic refs of github (refs/pull/*/head) seemed sensible: when updating the cache repo, the magic refs would be updated alongside and the forward-porting would have all the commits available. Turns out there are a few issues with this: - the magic refs have become a bit unreliable, so we need a fallback (b1c16fce8768080d30430f4e6d3788b60ce13de7) - the forwardport bot only needs the commits on a transient basis, but the magic refs live forever and diverge from all other content (since we rarely `merge` PRs), for a large repo with lots of PRs this leads to a significant inflation in size of repo (both on-disk and objects count) e.g. odoo/odoo has about 25% more objects with the pull refs included (3486550 to 4395319) and takes nearly 50% more space on disk (1.21G to 1.77) As a result, we can just stop configuring the pull refs entirely, and instead fetch the heads (~refs) we need as we need them. This can be a touch more expensive at times as it requires two `fetch` calls, and we'll have a few redundant calls as every forward port of a chain will require a fetch of the root's head, *however* it will avoid retrieving PR data for e.g. prs to master, as they don't get forward-ported, they also won't get parasite updates from PRs being ignored or eventually closed. Eventually this could be optimised further by - performing an update of the cache repo at the start of the cron iff there are batches to port - creating a temp clone for the batch - fetching the heads of all PRs to forward port in the temp clone in a single `fetch` - performing all the ports by cloning the temp clone (and not `fetch`-ing into those) - then cleaning up the temp clone after having cleaned up individual forward port clones Small followup for #489
2022-11-08 18:02:45 +07:00
subprocess.run(['git', 'clone', '--bare', self._source_url, str(repo_dir)], check=True)
# bare repos don't have fetch specs by default, and fetching *into*
# them is a pain in the ass, configure fetch specs so `git fetch`
# works properly
[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
repo = git(repo_dir)
[FIX] forwardport: cached repo setup Turns out bare repositories (unlike normal ones) don't have any refspecs by default. So adding the PR refspec would... replace the default behaviour (apparently?) and the base branches would never get fetched at all. --mirror looks to be an other option as it has a default refspec but it's a bit of an odd duck: it has pull requests in refs/pull but not all of them? And open / closed doesn't seem to be a factor.
2019-09-12 19:36:34 +07:00
repo.config('--add', 'remote.origin.fetch', '+refs/heads/*:refs/heads/*')
[IMP] forwardport: enable negative refspecs Requires Git 2.29, new mergebot has access to Git 2.34
2022-12-07 19:24:07 +07:00
# negative refspecs require git 2.29
repo.config('--add', 'remote.origin.fetch', '^refs/heads/tmp.*')
repo.config('--add', 'remote.origin.fetch', '^refs/heads/staging.*')
[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
return repo
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
def _outstanding(self, cutoff):
[ADD] forwardport: views of outstanding forwardports Though the forwardport posts regular reminders that an fw is outdated, it can be easy to miss for the non-subject (and apparently the subjects often just ignore the information entirely). Add a few relevant links there: * on PR pages, add a link to either the source or the forward-ports (if applicable), as well as the merge date * add a new page which lists all the PRs with outstanding forwardports, as well as the forwardports in question Fixes #474
2021-07-28 18:57:58 +07:00
""" Returns "outstanding" (unmerged and unclosed) forward-ports whose
source was merged before ``cutoff`` (all of them if not provided).
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
[ADD] forwardport: views of outstanding forwardports Though the forwardport posts regular reminders that an fw is outdated, it can be easy to miss for the non-subject (and apparently the subjects often just ignore the information entirely). Add a few relevant links there: * on PR pages, add a link to either the source or the forward-ports (if applicable), as well as the merge date * add a new page which lists all the PRs with outstanding forwardports, as well as the forwardports in question Fixes #474
2021-07-28 18:57:58 +07:00
:param str cutoff: a datetime (ISO-8601 formatted)
:returns: an iterator of (source, forward_ports)
"""
return groupby(self.env['runbot_merge.pull_requests'].search([
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
# only FP PRs
('source_id', '!=', False),
# active
('state', 'not in', ['merged', 'closed']),
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
('source_id.merge_date', '<', cutoff),
[ADD] forwardport: views of outstanding forwardports Though the forwardport posts regular reminders that an fw is outdated, it can be easy to miss for the non-subject (and apparently the subjects often just ignore the information entirely). Add a few relevant links there: * on PR pages, add a link to either the source or the forward-ports (if applicable), as well as the merge date * add a new page which lists all the PRs with outstanding forwardports, as well as the forwardports in question Fixes #474
2021-07-28 18:57:58 +07:00
], order='source_id, id'), lambda p: p.source_id)
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
def _hall_of_shame(self):
"""Provides data for the HOS view
* outstanding forward ports per reviewer
* pull requests with outstanding forward ports, oldest-merged first
"""
cutoff_dt = datetime.datetime.now() - DEFAULT_DELTA
outstanding = self.env['runbot_merge.pull_requests'].search([
('source_id', '!=', False),
('state', 'not in', ['merged', 'closed']),
('source_id.merge_date', '<', cutoff_dt),
], order=None)
# only keep merged because apparently some PRs are in a weird spot
# where they're sources but closed?
sources = outstanding.mapped('source_id').filtered('merge_date').sorted('merge_date')
outstandings = []
reviewers = collections.Counter()
for source in sources:
outstandings.append(Outstanding(source=source, prs=source.forwardport_ids & outstanding))
reviewers[source.reviewed_by] += 1
return HallOfShame(
reviewers=reviewers.most_common(),
outstanding=outstandings,
)
[ADD] forwardport: views of outstanding forwardports Though the forwardport posts regular reminders that an fw is outdated, it can be easy to miss for the non-subject (and apparently the subjects often just ignore the information entirely). Add a few relevant links there: * on PR pages, add a link to either the source or the forward-ports (if applicable), as well as the merge date * add a new page which lists all the PRs with outstanding forwardports, as well as the forwardports in question Fixes #474
2021-07-28 18:57:58 +07:00
def _reminder(self):
cutoff = self.env.context.get('forwardport_updated_before') \
or fields.Datetime.to_string(datetime.datetime.now() - DEFAULT_DELTA)
cutoff_dt = fields.Datetime.from_string(cutoff)
for source, prs in self._outstanding(cutoff):
[IMP] forwardport: exponential backoff on reminders The reminder feature is a bit brutal when people go on holidays or whatever as it keeps commenting every day. This should comment every day for a few days, then quickly taper down. Closes #285
2020-03-16 18:19:06 +07:00
backoff = dateutil.relativedelta.relativedelta(days=2**source.reminder_backoff_factor)
prs = list(prs)
[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
if source.merge_date > (cutoff_dt - backoff):
[IMP] forwardport: exponential backoff on reminders The reminder feature is a bit brutal when people go on holidays or whatever as it keeps commenting every day. This should comment every day for a few days, then quickly taper down. Closes #285
2020-03-16 18:19:06 +07:00
continue
source.reminder_backoff_factor += 1
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': source.repository.id,
'pull_request': source.number,
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
'message': "%sthis pull request has forward-port PRs awaiting action (not merged or closed):\n%s" % (
source.ping(),
'\n- '.join(pr.display_name for pr in sorted(prs, key=lambda p: p.number))
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
),
'token_field': 'fp_github_token',
})
[FIX] forwardport: fix pinging on forwardport PRs - avoid pinging the author of the fw PR (which is the forward-bot itself) - instead ping the author and reviewer of the source, and possibly the reviewer of the PR if any - might also be a good idea to ping reviewers of intermediate PRs?
2022-11-03 17:46:31 +07:00
def ping(self, author=True, reviewer=True):
source = self.source_id
if not source:
return super().ping(author=author, reviewer=reviewer)
# use a dict literal to maintain ordering (3.6+)
pingline = ' '.join(
f'@{p.github_login}'
for p in filter(None, {
author and source.author: None,
reviewer and source.reviewed_by: None,
reviewer and self.reviewed_by: None,
})
)
return pingline and (pingline + ' ')
[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 Stagings(models.Model):
_inherit = 'runbot_merge.stagings'
def write(self, vals):
r = super().write(vals)
[FIX] forwardport: co-dependent FPs where one PR is updated In the case where we have a co-dependent forward port (co-dependent PRs got forward-ported, which they should be together) where *one* of the PRs got explicitly updated, the batch would "fall into a hole" being handled as neither "this is part of a forward-port sequence" nor "this is a new merge to forward-port" (the latter being the proper one). Modify & remove guards which checked that either no or all PRs in a batch have parents: should be either all or not all. Fixes #231
2019-10-15 13:54:25 +07:00
# we've just deactivated a successful staging (so it got ~merged)
if vals.get('active') is False and self.state == 'success':
# check al batches to see if they should be forward ported
[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
for b in self.with_context(active_test=False).batch_ids:
[FIX] forwardport: co-dependent FPs where one PR is updated In the case where we have a co-dependent forward port (co-dependent PRs got forward-ported, which they should be together) where *one* of the PRs got explicitly updated, the batch would "fall into a hole" being handled as neither "this is part of a forward-port sequence" nor "this is a new merge to forward-port" (the latter being the proper one). Modify & remove guards which checked that either no or all PRs in a batch have parents: should be either all or not all. Fixes #231
2019-10-15 13:54:25 +07:00
# if all PRs of a batch have parents they're part of an FP
# sequence and thus handled separately, otherwise they're
# considered regular merges
if not all(p.parent_id for p in b.prs):
[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
self.env['forwardport.batches'].create({
'batch_id': b.id,
'source': 'merge',
})
return r
[IMP] runbot_merge, forwardport: variable-user feedback Having all the feedback be sent by the mergebot user (github_token) is confusing. Add a way to specify which field of project should be used to source the token used when sending feedback. Fixes #190
2019-09-18 20:37:14 +07:00
class Feedback(models.Model):
_inherit = 'runbot_merge.pull_requests.feedback'
token_field = fields.Selection(selection_add=[('fp_github_token', 'Forwardport Bot')])
[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
[IMP] forwardport: gc/maintenance of local repo caches The current system makes / lets GC run during fetching. This has a few issues: - the autogc consumes resources during the forward-porting process (not that it's hugely urgent but it seems unnecessary) - the autogc commonly fails due to the combination of large repository (odoo/odoo) and low memory limits (hardmem for odoo, which get translated into soft ulimits) As a result, the garbage collection of the repository sometimes stops entirely, leading to an increase in repository size and a decrease in performances. To mitigate this issue, disable the automagic gc and maintenance during normal operation, and instead add a weekly cron which runs an aggressive GC with memory limits disabled (as far as they can get, if the limits are imposed externally there's nothing to be done). The maintenance is implemented using a full lockout of the forward-port cron and an in-place GC rather than a copy/gc/swap, as doing this maintenance at the small hours of the week-end (sat-sun night) seems like a non-issue: currently an aggressive GC of odoo/odoo (using the default aggressive options) takes a total of 2:30 wallclock (5h user) on a fairly elderly machine (it's closer to 20mn wallclock and 2h user on my local machine, also turns out the cache repos are kinda badly configured leading to ~30% more objects than necessary which doesn't help). For the record, a fresh checkout of odoo/odoo right now yields: | Overall repository size | | | * Commits | | | * Count | 199 k | | * Total size | 102 MiB | | * Trees | | | * Count | 1.60 M | | * Total size | 2.67 GiB | | * Total tree entries | 74.1 M | | * Blobs | | | * Count | 1.69 M | | * Total size | 72.4 GiB | If this still proves insufficient, a further option would be to deploy a "generational repacking" strategy: https://gitlab.com/gitlab-org/gitaly/-/issues/2861 (though apparently it's not yet been implemented / deployed on gitlab so...). But for now we'll see how it shakes out. Close #489
2022-11-07 15:53:11 +07:00
ALWAYS = ('gc.auto=0', 'maintenance.auto=0')
[FIX] forwardport: relax limits on all git operations Apparently it's not just GC which causes trouble, on the new configuration anyway.
2022-12-12 13:40:39 +07:00
def _bypass_limits():
resource.setrlimit(resource.RLIMIT_AS, (resource.RLIM_INFINITY, resource.RLIM_INFINITY))
[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 git(directory): return Repo(directory, check=True)
class Repo:
def __init__(self, directory, **config):
self._directory = str(directory)
[IMP] forwardport: git commands logging - When updating the local repo cache, always capture both stdout and stderr and log them out rather than having them in journalctl hard to relate to the main log. - In the git layer, capture stderr by default and log it automatically on command failure.
2021-02-26 15:49:41 +07:00
config.setdefault('stderr', subprocess.PIPE)
[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
self._config = config
self._params = ()
self._opener = subprocess.run
def __getattr__(self, name):
return GitCommand(self, name.replace('_', '-'))
def _run(self, *args, **kwargs):
opts = {**self._config, **kwargs}
[IMP] forwardport: git commands logging - When updating the local repo cache, always capture both stdout and stderr and log them out rather than having them in journalctl hard to relate to the main log. - In the git layer, capture stderr by default and log it automatically on command failure.
2021-02-26 15:49:41 +07:00
args = ('git', '-C', self._directory)\
[IMP] forwardport: gc/maintenance of local repo caches The current system makes / lets GC run during fetching. This has a few issues: - the autogc consumes resources during the forward-porting process (not that it's hugely urgent but it seems unnecessary) - the autogc commonly fails due to the combination of large repository (odoo/odoo) and low memory limits (hardmem for odoo, which get translated into soft ulimits) As a result, the garbage collection of the repository sometimes stops entirely, leading to an increase in repository size and a decrease in performances. To mitigate this issue, disable the automagic gc and maintenance during normal operation, and instead add a weekly cron which runs an aggressive GC with memory limits disabled (as far as they can get, if the limits are imposed externally there's nothing to be done). The maintenance is implemented using a full lockout of the forward-port cron and an in-place GC rather than a copy/gc/swap, as doing this maintenance at the small hours of the week-end (sat-sun night) seems like a non-issue: currently an aggressive GC of odoo/odoo (using the default aggressive options) takes a total of 2:30 wallclock (5h user) on a fairly elderly machine (it's closer to 20mn wallclock and 2h user on my local machine, also turns out the cache repos are kinda badly configured leading to ~30% more objects than necessary which doesn't help). For the record, a fresh checkout of odoo/odoo right now yields: | Overall repository size | | | * Commits | | | * Count | 199 k | | * Total size | 102 MiB | | * Trees | | | * Count | 1.60 M | | * Total size | 2.67 GiB | | * Total tree entries | 74.1 M | | * Blobs | | | * Count | 1.69 M | | * Total size | 72.4 GiB | If this still proves insufficient, a further option would be to deploy a "generational repacking" strategy: https://gitlab.com/gitlab-org/gitaly/-/issues/2861 (though apparently it's not yet been implemented / deployed on gitlab so...). But for now we'll see how it shakes out. Close #489
2022-11-07 15:53:11 +07:00
+ tuple(itertools.chain.from_iterable(('-c', p) for p in self._params + ALWAYS))\
[IMP] forwardport: git commands logging - When updating the local repo cache, always capture both stdout and stderr and log them out rather than having them in journalctl hard to relate to the main log. - In the git layer, capture stderr by default and log it automatically on command failure.
2021-02-26 15:49:41 +07:00
+ args
try:
[FIX] forwardport: relax limits on all git operations Apparently it's not just GC which causes trouble, on the new configuration anyway.
2022-12-12 13:40:39 +07:00
return self._opener(args, preexec_fn=_bypass_limits, **opts)
[IMP] forwardport: git commands logging - When updating the local repo cache, always capture both stdout and stderr and log them out rather than having them in journalctl hard to relate to the main log. - In the git layer, capture stderr by default and log it automatically on command failure.
2021-02-26 15:49:41 +07:00
except subprocess.CalledProcessError as e:
[FIX] forwardport: avoid logging git error if there's no stream data If no stream data was captured (no stderr and no stdout), would just log git call error: as error, with no further information. Don't do that if we have neither stderr nor stdout data, since we're re-raising the exception anyway, it's just confusing.
2022-10-25 16:51:51 +07:00
stream = e.stderr if e.stderr else e.stdout
if stream:
_logger.error("git call error: %s", stream)
[IMP] forwardport: git commands logging - When updating the local repo cache, always capture both stdout and stderr and log them out rather than having them in journalctl hard to relate to the main log. - In the git layer, capture stderr by default and log it automatically on command failure.
2021-02-26 15:49:41 +07:00
raise
[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 stdout(self, flag=True):
if flag is True:
return self.with_config(stdout=subprocess.PIPE)
elif flag is False:
return self.with_config(stdout=None)
return self.with_config(stdout=flag)
def lazy(self):
r = self.with_config()
r._config.pop('check', None)
r._opener = subprocess.Popen
return r
def check(self, flag):
return self.with_config(check=flag)
def with_config(self, **kw):
opts = {**self._config, **kw}
r = Repo(self._directory, **opts)
r._opener = self._opener
r._params = self._params
return r
def with_params(self, *args):
r = self.with_config()
r._params = args
return r
def clone(self, to, branch=None):
self._run(
[IMP] forwardport: refs There's an issue of too many commits being selected for cherry-picking. It still isn't quite clear, but the forward ports to 11.0 systematically seem to get everything from 5045b5f4c346e60c9b127403ef1fde453d49396a to the PR head, and that commit was one of the first to land after the last merge-based forward port. So the commits selection seems to behave as if the commits range was `..origin/pull/36692` rather than the `origin/10.0..origin/pull/36692` which is passed to rev-list. This might be an issue of concurrent access / update of the refs (though I can't reproduce it locally, missing refs generate an error). This change attempts to "pin" the local branch in the working copy rather than go and get it from the repo. It's unclear that this will change / fix anything, but it might just work. Also stop creating shared clones, that seems like an unnecessary risk (and might be the actual source of the issue).
2019-09-11 20:41:07 +07:00
'clone',
[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 branch is None else ['-b', branch]),
self._directory, to,
)
return Repo(to)
class GitCommand:
def __init__(self, repo, name):
self._name = name
self._repo = repo
def __call__(self, *args, **kwargs):
return self._repo._run(self._name, *args, *self._to_options(kwargs))
def _to_options(self, d):
for k, v in d.items():
if len(k) == 1:
yield '-' + k
else:
yield '--' + k.replace('_', '-')
if v not in (None, True):
assert v is not False
yield str(v)
class CherrypickError(Exception):
...
[FIX] forwardport: broken fp if intermediate commit requires renamelimit=0 On a cherrypick failing due to renamelimit issues, the cherrypick would be retried after resetting the target to its *original* commit. This only works correctly if the first commit works after a retry, if a latter commit has to be retried then it gets re-cherry-picked onto the target branch rather than its own parent. Fix by remembering the previously successful cherry-picked commit and resetting to *that*. However I can't really test it because it's rather hard to get into a situation where the rename detection fails using synthetic tests. While at it, clean the logs by stripping the "performing inexact rename detection" stuff from all stderr (both the CherrypickError from which it was already stripped and the debug messages). Fixes #444
2021-01-11 20:43:36 +07:00
[IMP] forwardport: handling of missing magic refs Github can fail to create the magic refs on PRs (`pull/refs/?/head`). Since forwardport relies on these refs to fetch PR content this is an issue when it occurs, as the forward ports fail in a loop. After discussion with Github support, it turns out Github enabled `allowReachableSHA1InWant` a while back, meaning it's possible to fetch content by commit (rather than ref) as long as the content is "in network". Use this property as fallback when checking if we can see the PR head before forward porting. Also: - remove explicit configuration of GC during fetch, it doesn't disable the autogc (yet?) but that's likely going to happen anyway - update logging and logger hierarchy during forward port to make things clearer and easier to extract, although based on PR id rather than number - rate limit failing forward ports to avoid running them on every cron (~ every minute), run them every ~30mn instead, this provides higher odds of recovery with less log garbage in case of transient github failure, and if the PR is stuck it limits the log pollution Fixes #658
2022-10-28 13:03:12 +07:00
class ForwardPortError(Exception):
pass
[FIX] forwardport: broken fp if intermediate commit requires renamelimit=0 On a cherrypick failing due to renamelimit issues, the cherrypick would be retried after resetting the target to its *original* commit. This only works correctly if the first commit works after a retry, if a latter commit has to be retried then it gets re-cherry-picked onto the target branch rather than its own parent. Fix by remembering the previously successful cherry-picked commit and resetting to *that*. However I can't really test it because it's rather hard to get into a situation where the rename detection fails using synthetic tests. While at it, clean the logs by stripping the "performing inexact rename detection" stuff from all stderr (both the CherrypickError from which it was already stripped and the debug messages). Fixes #444
2021-01-11 20:43:36 +07:00
def _clean_rename(s):
""" Filters out the "inexact rename detection" spam of cherry-pick: it's
useless but there seems to be no good way to silence these messages.
"""
return '\n'.join(
l for l in s.splitlines()
if not l.startswith('Performing inexact rename detection')
)
[IMP] forwardport: hall of fame layout and informations The list of outstanding forwardports was pretty messy as the ordering was unclear and there was little way to really drill into the thing. * Shows outstanding forward ports sorted by merged date ascending, the oldest-merged PRs are the ones most in need of fixing while PRs which were only just merged can safely be ignored. * List reviewers with outstanding forward-ports, allow filtering by clicking on their name, allow deseleting through the subtitle of the page. * Don't display reviewer in list when page is already filtered by reviewer. Also improve PR page a bit: * Add reviewer. * Add direct link to backend (closes #524). Closes #529
2021-10-19 19:39:19 +07:00
class HallOfShame(typing.NamedTuple):
reviewers: list
outstanding: list
class Outstanding(typing.NamedTuple):
source: object
prs: object
Reference in New Issue Copy Permalink