[IMP] runbot_merge: preserve batch ordering in stagings

Batch ordering in stagings is important in order to correctly
reconstitute the full project history.

In the old mergebot, since batches are created on the fly during
staging this information is reified by the batch ids. But since batch
ids are now persistent and there is no relationship between the
creation of a batch and its merging (especially not relative to other
batches) it's an issue as reconstituting sub-staging git history would
be impossible.

Which is not the worst, but is not great.

The solution is to reify the join table between stagings and batches
in order for *that* to keep history (simply via the sequential PK),
and in converting to the new system carefully generate the new links
in an order matching the old batch ids.

runbot_merge/models/batch.py

@@ -9,6 +9,7 @@ from collections import defaultdict
 from collections.abc import Iterator
 
 import requests
+from psycopg2 import sql
 
 from odoo import models, fields, api
 from .utils import enum
@@ -18,6 +19,28 @@ _logger = logging.getLogger(__name__)
 FOOTER = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n'
 
 
+class StagingBatch(models.Model):
+    _name = 'runbot_merge.staging.batch'
+    _description = "link between batches and staging in order to maintain an " \
+                   "ordering relationship between the batches of a staging"
+    _log_access = False
+    _order = 'id'
+
+    runbot_merge_batch_id = fields.Many2one('runbot_merge.batch', required=True)
+    runbot_merge_stagings_id = fields.Many2one('runbot_merge.stagings', required=True)
+
+    def init(self):
+        super().init()
+
+        self.env.cr.execute(sql.SQL("""
+        CREATE UNIQUE INDEX IF NOT EXISTS runbot_merge_staging_batch_idx
+            ON {table} (runbot_merge_stagings_id, runbot_merge_batch_id);
+
+        CREATE INDEX IF NOT EXISTS runbot_merge_staging_batch_rev
+            ON {table} (runbot_merge_batch_id) INCLUDE (runbot_merge_stagings_id);
+        """).format(table=sql.Identifier(self._table)))
+
+
 class Batch(models.Model):
     """ A batch is a "horizontal" grouping of *codependent* PRs: PRs with
     the same label & target but for different repositories. These are
@@ -31,7 +54,12 @@ class Batch(models.Model):
     _parent_store = True
 
     target = fields.Many2one('runbot_merge.branch', store=True, compute='_compute_target')
-    staging_ids = fields.Many2many('runbot_merge.stagings')
+    batch_staging_ids = fields.One2many('runbot_merge.staging.batch', 'runbot_merge_batch_id')
+    staging_ids = fields.Many2many(
+        'runbot_merge.stagings',
+        compute="_compute_staging_ids",
+        context={'active_test': False},
+    )
     split_id = fields.Many2one('runbot_merge.split', index=True)
 
     prs = fields.One2many('runbot_merge.pull_requests', 'batch_id')
@@ -74,6 +102,11 @@ class Batch(models.Model):
         context={"active_test": False},
     )
 
+    @api.depends('batch_staging_ids.runbot_merge_stagings_id')
+    def _compute_staging_ids(self):
+        for batch in self:
+            batch.staging_ids = batch.batch_staging_ids.runbot_merge_stagings_id
+
     @property
     def source(self):
         return self.browse(map(int, self.parent_path.split('/', 1)[:1]))
@@ -452,6 +485,10 @@ class Batch(models.Model):
 
         return True
 
+    @api.ondelete(at_uninstall=True)
+    def _on_delete_clear_stagings(self):
+        self.batch_staging_ids.unlink()
+
     def unlink(self):
         """
         batches can be unlinked if they:

runbot_merge/models/pull_requests.py

@@ -1763,7 +1763,8 @@ class Stagings(models.Model):
 
     target = fields.Many2one('runbot_merge.branch', required=True, index=True)
 
-    batch_ids = fields.Many2many('runbot_merge.batch', context={'active_test': False})
+    staging_batch_ids = fields.One2many('runbot_merge.staging.batch', 'runbot_merge_stagings_id')
+    batch_ids = fields.Many2many('runbot_merge.batch', context={'active_test': False}, compute="_compute_batch_ids")
     pr_ids = fields.One2many('runbot_merge.pull_requests', compute='_compute_prs')
     state = fields.Selection([
         ('success', 'Success'),
@@ -1814,6 +1815,11 @@ class Stagings(models.Model):
             for staging in self
         ]
 
+    @api.depends('staging_batch_ids.runbot_merge_batch_id')
+    def _compute_batch_ids(self):
+        for staging in self:
+            staging.batch_ids = staging.staging_batch_ids.runbot_merge_batch_id
+
     @api.depends('heads')
     def _compute_statuses(self):
         """ Fetches statuses associated with the various heads, returned as

runbot_merge/models/stagings_create.py

@@ -8,13 +8,12 @@ import os
 import re
 from collections.abc import Mapping
 from difflib import Differ
-from itertools import takewhile
 from operator import itemgetter
 from typing import Dict, Union, Optional, Literal, Callable, Iterator, Tuple, List, TypeAlias
 
 from werkzeug.datastructures import Headers
 
-from odoo import api, models, fields
+from odoo import api, models, fields, Command
 from odoo.tools import OrderedSet, groupby
 from .pull_requests import Branch, Stagings, PullRequests, Repository
 from .batch import Batch
@@ -166,7 +165,7 @@ For-Commit-Id: {it.head}
     # create actual staging object
     st: Stagings = env['runbot_merge.stagings'].create({
         'target': branch.id,
-        'batch_ids': [(4, batch.id, 0) for batch in staged],
+        'staging_batch_ids': [Command.create({'runbot_merge_batch_id': batch.id}) for batch in staged],
         'heads': heads,
         'commits': commits,
     })

runbot_merge/security/ir.model.access.csv

@@ -16,6 +16,7 @@ access_runbot_merge_stagings_commits_admin,Admin access to staging commits,model
 access_runbot_merge_stagings_cancel_admin,Admin access to cancelling stagings,model_runbot_merge_stagings_cancel,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_split_admin,Admin access to splits,model_runbot_merge_split,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_batch_admin,Admin access to batches,model_runbot_merge_batch,runbot_merge.group_admin,1,1,1,1
+access_runbot_merge_staging_batch_admin,Admin access to batch/staging link,model_runbot_merge_staging_batch,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_fetch_job_admin,Admin access to fetch jobs,model_runbot_merge_fetch_job,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_pull_requests_feedback_admin,Admin access to feedback,model_runbot_merge_pull_requests_feedback,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_review_rights,Admin access to review permissions,model_res_partner_review,runbot_merge.group_admin,1,1,1,1

runbot_merge/tests/test_basic.py

@@ -2784,6 +2784,11 @@ class TestBatching(object):
 
         staging = ensure_one(env['runbot_merge.stagings'].search([]))
         assert staging.pr_ids == pr11 | pr12 | pr21
+        assert list(staging.batch_ids) == [
+            pr11.batch_id,
+            pr12.batch_id,
+            pr21.batch_id,
+        ]
         assert not pr22.staging_id
 
     def test_batching_urgent(self, env, repo, config):
@@ -2804,6 +2809,10 @@ class TestBatching(object):
         staging_1 = sm_all.staging_id
         assert staging_1
         assert len(staging_1) == 1
+        assert list(staging_1.batch_ids) == [
+            p_11.batch_id,
+            p_12.batch_id,
+        ]
 
         # no statuses run on PR0s
         with repo: