[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

@@ -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',

runbot_merge/models/crons/__init__.py

@@ -1 +1,2 @@
 from . import git_maintenance
+from . import cleanup_scratch_branches

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)

runbot_merge/models/crons/cleanup_scratch_branches.xml

@@ -0,0 +1,23 @@
+<odoo>
+    <record id="access_branch_cleanup" model="ir.model.access">
+        <field name="name">Access to branch cleanup is useless</field>
+        <field name="model_id" ref="model_runbot_merge_branch_cleanup"/>
+        <field name="perm_read">0</field>
+        <field name="perm_create">0</field>
+        <field name="perm_write">0</field>
+        <field name="perm_unlink">0</field>
+    </record>
+
+    <record model="ir.cron" id="branch_cleanup">
+        <field name="name">Removal of scratch refs for deactivated branch</field>
+        <field name="model_id" ref="model_runbot_merge_branch_cleanup"/>
+        <field name="state">code</field>
+        <field name="code">model._run()</field>
+        <!--
+        nota: even though this is only triggered, numbercall has to be
+        non-zero because the counter is taken in account by cron triggers
+        -->
+        <field name="numbercall">-1</field>
+        <field name="doall" eval="False"/>
+    </record>
+</odoo>

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')

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