From 76f4ed3bf647458bffeab30134905fae00ed3f39 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 31 Aug 2023 09:03:17 +0200 Subject: [PATCH] [ADD] runbot_merge: delete scratch branches when a branch is disabled If a branch `foo` is disabled, then `tmp.foo` and `staging.foo` become unnecessary (with #247 fixed the tmp refs are not used for creating stagings anymore, but for now they're still used for the "safety dance" of merging a successful staging into the corresponding mainline). Fixes #605 --- runbot_merge/__manifest__.py | 1 + runbot_merge/models/crons/__init__.py | 1 + .../models/crons/cleanup_scratch_branches.py | 32 +++++++++++++++++++ .../models/crons/cleanup_scratch_branches.xml | 23 +++++++++++++ runbot_merge/models/pull_requests.py | 1 + runbot_merge/tests/test_disabled_branch.py | 18 +++++++++++ 6 files changed, 76 insertions(+) create mode 100644 runbot_merge/models/crons/cleanup_scratch_branches.py create mode 100644 runbot_merge/models/crons/cleanup_scratch_branches.xml diff --git a/runbot_merge/__manifest__.py b/runbot_merge/__manifest__.py index 1f6ae7f4..266605d3 100644 --- a/runbot_merge/__manifest__.py +++ b/runbot_merge/__manifest__.py @@ -8,6 +8,7 @@ 'data/merge_cron.xml', 'models/crons/git_maintenance.xml', + 'models/crons/cleanup_scratch_branches.xml', 'data/runbot_merge.pull_requests.feedback.template.csv', 'views/res_partner.xml', 'views/runbot_merge_project.xml', diff --git a/runbot_merge/models/crons/__init__.py b/runbot_merge/models/crons/__init__.py index 939a44cc..f3eed923 100644 --- a/runbot_merge/models/crons/__init__.py +++ b/runbot_merge/models/crons/__init__.py @@ -1 +1,2 @@ from . import git_maintenance +from . import cleanup_scratch_branches diff --git a/runbot_merge/models/crons/cleanup_scratch_branches.py b/runbot_merge/models/crons/cleanup_scratch_branches.py new file mode 100644 index 00000000..1e7b6274 --- /dev/null +++ b/runbot_merge/models/crons/cleanup_scratch_branches.py @@ -0,0 +1,32 @@ +import logging + +from odoo import models + + +_logger = logging.getLogger(__name__) +class BranchCleanup(models.TransientModel): + _name = 'runbot_merge.branch_cleanup' + _description = "cleans up scratch refs for deactivated branches" + + def _run(self): + deactivated = self.env['runbot_merge.branch'].search([ + ('active', '=', False), + ('write_date', '>=', self.env.context['lastcall']), + ]) + _logger.info( + "deleting scratch (tmp and staging) refs for branches %s", + ', '.join(b.name for b in deactivated) + ) + # loop around the repos first, so we can reuse the gh instance + for r in deactivated.mapped('project_id.repo_ids'): + gh = r.github() + for b in deactivated: + if b.project_id != r.project_id: + continue + + res = gh('delete', f'git/refs/heads/tmp.{b.name}', check=False) + if res.status_code != 204: + _logger.info("no tmp branch found for %s:%s", r.name, b.name) + res = gh('delete', f'git/refs/heads/staging.{b.name}', check=False) + if res.status_code != 204: + _logger.info("no staging branch found for %s:%s", res.name, b.name) diff --git a/runbot_merge/models/crons/cleanup_scratch_branches.xml b/runbot_merge/models/crons/cleanup_scratch_branches.xml new file mode 100644 index 00000000..e9172f6d --- /dev/null +++ b/runbot_merge/models/crons/cleanup_scratch_branches.xml @@ -0,0 +1,23 @@ + + + Access to branch cleanup is useless + + 0 + 0 + 0 + 0 + + + + Removal of scratch refs for deactivated branch + + code + model._run() + + -1 + + + diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index a6d210bf..96581736 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -282,6 +282,7 @@ class Branch(models.Model): 'pull_request': pr.number, 'message': tmpl._format(pr=pr), } for pr in self.prs]) + self.env.ref('runbot_merge.branch_cleanup')._trigger() return True @api.depends('staging_ids.active') diff --git a/runbot_merge/tests/test_disabled_branch.py b/runbot_merge/tests/test_disabled_branch.py index 17282985..08cbf117 100644 --- a/runbot_merge/tests/test_disabled_branch.py +++ b/runbot_merge/tests/test_disabled_branch.py @@ -1,9 +1,15 @@ +import pytest + from utils import seen, Commit, pr_page def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, config, users, page): """ PRs to disabled branches are ignored, but what if the PR exists *before* the branch is disabled? """ + # run crons from template to clean up the queue before possibly creating + # new work + assert env['base'].run_crons() + repo = make_repo('repo') project.branch_ids.sequence = 0 project.write({'branch_ids': [ @@ -81,6 +87,18 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf assert pr_id.target == env['runbot_merge.branch'].search([('name', '=', 'other2')]) assert pr_id.staging_id + # staging of `pr` should have generated a staging branch + _ = repo.get_ref('heads/staging.other') + # stagings should not need a tmp branch anymore, so this should not exist + with pytest.raises(AssertionError, match=r'Not Found'): + repo.get_ref('heads/tmp.other') + + assert env['base'].run_crons() + + # triggered cleanup should have deleted the staging for the disabled `other` + # target branch + with pytest.raises(AssertionError, match=r'Not Found'): + repo.get_ref('heads/staging.other') def test_new_pr_no_branch(env, project, make_repo, setreviewers, users): """ A new PR to an *unknown* branch should be ignored and warn