Commit Graph

1671 Commits

Author SHA1 Message Date
Xavier Morel
0a839a4857 [FIX] forwardport: don't break forward porting on huge conflicts
On forward-porting, odoo/odoo#170183 generates a conflict on pretty
much every one of the 1111 files it touches, because they are
modify/delete conflicts that generates a conflict message over 200
bytes per file, which is over 200kB of output.

For this specific scenario, the commit message was being passed
through arguments to the `git` command, resulting in a command line
exceeding `MAX_ARG_STRLEN`[^1]. The obvious way to fix this is to pass
the commit message via stdin as is done literally in the line above
where we just copy a non-generated commit message.

However I don't think hundreds of kbytes worth of stdout[^2] is of any
use, so shorten that a bit, and stderr while at it.

Don't touch the commit message size for now, possibly forever, but
note that some log diving reveals a commit with a legit 18kB message
(odoo/odoo@42a3b704f7) so if we want to
restrict that the limit should be at least 32k, and possibly 64. But
it might be a good idea to make that limit part of the ready / merge
checks too, rather than cut things off or trigger errors during
staging.

Fixes #900

[^1]: Most resources on "Argument list too long" reference `ARG_MAX`,
    but on both my machine and the server it is 2097152 (25% of the
    default stack), which is ~10x larger than the commit message we
    tried to generate. The actual limit is `MAX_ARG_STRLEN` which
    can't be queried directly but is essentially hard-coded to
    PAGE_SIZE * 32 = 128kiB, which tracks.

[^2]: Somewhat unexpectedly, that's where `git-cherry-pick` sends the
    conflict info.
2024-06-25 15:54:23 +02:00
Xavier Morel
dc90a207d6 [ADD] runbot_merge: help command, and help on error
Fixes #898
2024-06-24 22:16:43 +02:00
Xavier Morel
f3a0a5c27c [FIX] runbot_merge: tracking message author on PullRequest events
d4fa1fd353 added tracking to changes
from *comments* (as well as a few hacks around authorship transfer),
however it missed two things:

First, it set the `change-author` during comments handling only, so
changes from the `PullRequest` hook e.g. open, synchronise, close,
edit, don't get attributed to their actual source, and instead just
fall back to uid(1). This is easy enough to fix as the `sender` is
always provided, that can be resolved to a partner which is then set
as the author of whatever changes happen.

Second, I actually missed one of the message hooks: there's both
`_message_log` and `_message_log_batch` and they don't call one
another, so both have to be overridden in order for tracking to be
consistent. In this case, specifically, the *creation* of a tracked
object goes through `_message_log_batch` (since that's a very generic
message and so works on every tracked object created during the
transaction... even though batch has a message per record anyway...)
while *updates* go through `_message_log`.

Fixes #895
2024-06-21 16:33:44 +02:00
Xavier Morel
b109225f44 [IMP] runbot_merge: quality of feedback on errorneous commands
- When a redundant approval is sent to a PR, notify but don't ignore
  the entire command set, there's no actual risk.
- Indicate that the entire comment was ignored when finding something
  which does not parse.

Fixes #892, fixes #893
2024-06-21 15:38:54 +02:00
Xavier Morel
7cd9afe7f2 [IMP] runbot_merge: trigger commits cron
The commit cron needs to be triggered any time we:

- create a new commit
- update a commit to set its `to_check`

So do that in create and write as well as the SQL query in the
webhook handler.

This should mean we don't need the periodic cron anymore, but for
safety's sake run it on 30mn for now.

TBF even if we miss triggers, the next `status` webhook hitting will
check all the relevant commits anyway...
2024-06-21 11:02:50 +02:00
Xavier Morel
92e8eecbb5 [FIX] runbot_merge: ability to create PRs via the UI
This is useful to repro issues.

60c4b5141d added `inverse=readonly`
hooks to various newly computed fields to ensure they can not be *written*
to, either overwriting the content (stored) or silently being
dropped (non-stored).

However because they're `inverse` hooks this had the effect of making
them writeable from the backend UI since the ORM uses `inverse` as a
signal to make the field writeable. This then caused the web client to
send stuff for those fields, which are not necessarily even visible in
the form, leading to write errors when trying to save a PR creation.

By marking the fields as `readonly` explicitly we make sure that
doesn't happen, and we can create PRs from the backend UI (kinda, I
think the label is still an issue).
2024-06-21 10:42:37 +02:00
Xavier Morel
906505ed15 [IMP] runbot_merge: filter on the base attribute not computed
Should not actually do anything relevant, but seems like a good idea.
2024-06-21 10:42:08 +02:00
Xavier Morel
3410f50248 [FIX] runbot_merge: Commit.create
The method was not marked as a create, following which it did not
allow creating commits via the UI (annoying for testing / reproducing
issues involving statuses).
2024-06-21 10:41:01 +02:00
Xavier Morel
737cbd5de2 [IMP] *: merge fw overrides into their parent
Not actually useful in any way, but it does remove a few lines, avoids
a few dupe writes, and furthers the cause of #789
2024-06-21 10:40:06 +02:00
Xavier Morel
f303674434 [FIX] *: re-enable notification on status failure
If a PR gets approved *then* fails CI, there should be a notification
warning the author & reviewer since
48e08b657b, it even has a test, which
passes (in fact it has *two*, one of which is redundant, so merge
`test_ci_failure_after_review` into the later `test_ci_approved`).

*However* this is in runbot_merge, turns out in
fafa7ef437 some refactoring was done in
order to override the notification and customise it for *forward
ports* with a failed status... except that override never called its
`super()`, so as soon as forwardport is installed the base
notification stops working, and that's been that since October
2019 (had been added in March that year, ignoring deployment lag).

This can be revealed by adding the corresponding check in the
*forwardport* tests, revealing the failure.

This was a pain to track down, thankfully it reproduced relatively
easily locally.

While this could be resolved in the override, might as well fold it
into the base method in furtherance of #789: the mergebot is only
used by odoo, and only with both modules combined, so splitting them
is not useful. And furthermore it things should work fine with the
forwardport installed but unused.

Fixes #894
2024-06-21 10:27:01 +02:00
Xavier Morel
4a521e1251 [IMP] runbot_merge: hide backend links from group_user
The backend links in the PR dashboard were gated behind the
`group_user` (internal user) group, however turns out while internal
users have read access to PRs they don't have access to ancillary
objects (e.g. batches, stagings, the link between stagings and
batches), and I think the only way to fix the issue would be to move
it to an optional inheritance (inheritance + group), because `groups`
on view nodes only hides the content without removing it.

I believe in more recent Odoo versions this actually works correctly,
so that might actually be more of an incentive to upgrade...
2024-06-20 14:21:40 +02:00
Xavier Morel
20d259aa77 [IMP] runbot_merge: always display PR title
Previous version would always hide the title if the PR was
blocked (e.g. blocked or failed), turns out there are people who
actually use the PR title on the main dashboard, so suppressing that
is inconvenient for them.

Try to show the PR title if available, and add the blocked message if
present.
2024-06-20 13:49:17 +02:00
Xavier Morel
728524db12 [IMP] runbot_merge: send merge method warning faster, and on review
- Instead of warning about the merge method on ready PRs, also warn on
  *approved* (but exclude staged just cuz), as that's really when the
  user wants to know that they forgot to set the merge method
- The cron only triggers hourly, but *if* a user approves a PR *and*
  the merge method is not set yet, chances are good they'll need a
  reminder (if they `r+ rebase-merge` or w/e the cron will just ignore
  the PR and it's no skin off our back), so `_trigger` the cron for
  validation.
- Also do the same when skipchecks is set as it's very similar.

In reality we might want to hook off of the state transitioning to
reviewed but I'm not sure there's good ways to do that (triggering a
cron inside a compute doesn't seem like a good idea).

Update a pair of tests which would approve a multi-commit PR without
setting a merge method, just because the helper they use to build the
PR happens to create multiple commits.

Fix #891
2024-06-13 13:36:34 +02:00
Xavier Morel
9d9cae1d57 [FIX] runbot_merge: access to self in loop
This is a low issue as the prs of a commit are only listed from the
form so the compute is pretty much always called with a single record,
but still an unforced error which can easily be fixed.
2024-06-13 09:35:29 +02:00
Xavier Morel
2662411b96 [FIX] runbot_merge: _schedule_fp_followup not handling multiple batches
`_schedule_fp_followup` correctly iterates on `self`, however some of
the per-iteration work did not handle that correctly, and would try to
access fields on `self`.

Thankfully in most cases it only works on one batch at a time
anyway, *however* if multiple PRs share a HEAD (which is weird but...)
then `_validate` is called on multiple PRs, which through the
forwardport override leads to `_schedule_fp_followup` being called on
multiple batches, and failing when trying to access the `fw_policy`.

Fix by avoiding the misuse of `self` in the two locations where it's
doing something other than accessing `env`.
2024-06-13 08:04:12 +02:00
Xavier Morel
7711d09854 [IMP] *: add fw=no, deprecate ignore
Without fw-bot being its bearer, "ignore" is a lot less clear than it
used to as it looks to be asking to ignore the PR entirely (as if it
was targeted to an unmanaged branch).

Deprecate this command, and tack on the shortcut to the fw
subcommand. It is slightly sub-par as technically it does not quite
fit with the other subcommands, and furthermore can't be disabled via
fw=default... although maybe it could be? Maybe instead of setting the
limit fw=no could set that value to the forwardport mode, and the
fw_policy users could check that? It would require some more finessing
tho:

- `DEFAULT` would need to be accessible to the author as well as the
  reviewers so the author could toggle between `NO` and `DEFAULT`.
- There should probably be a warning of some sort when setting a limit
  to an unportable PR.
- The dashboards would need to take `NO` in account (though I guess
  that's just defaulting the limit to the target).
2024-06-12 16:08:25 +02:00
Xavier Morel
413027ad5b [IMP] runbot_merge: formatting & langage of PR attributes
Replace the unclear "unchecked" and "unreviewed" by "missing statuses"
and "missing r+", which are hopefully clearer as they better match
other lingo.

Also increase font for attributes, as size 10 was a bit small.

And finally add staging state to caching key, to differentiate "ready"
from "staged" pictures in gh's cache. "ready" should not be necessary
as it ought be implied by the label.
2024-06-12 15:51:17 +02:00
Xavier Morel
a2d7180216 [IMP] runbot_merge: move limit to fwport tab
And filter it to only consider branches in the same project as the PR,
and a lower sequence than its target. That way it's harder to fuck up
when trying to set limits from the backend.
2024-06-12 15:34:39 +02:00
Xavier Morel
d010f0374a [FIX] *: dashboard when PRs have different limits
The code selecting the lower and upper bounds for the PR dashboard did
not deal correctly with getting multiple limits in the same genealogy.
2024-06-12 15:09:47 +02:00
Xavier Morel
81f133de15 [IMP] test utils: don't ACL-protect repos by default
This is quite frustrating when trying to access the frontend from a
test especially combined with the lack of in-log feedback (before the
previous commit).
2024-06-12 15:09:42 +02:00
Xavier Morel
d2e730c39b [IMP] runbot_merge: log ACL error in PR controller
Currently this just silently returns a 404. Since repos are gated by
default (only accessible to internal users) this can get very
confusing when trying to setup a new repo or when forgetting this
information when writing tests.
2024-06-12 15:09:42 +02:00
Xavier Morel
2ab06ca96b [IMP] *: require explicitly specifying whether exceptions in logs are valid
Seems like a good idea to better keep track of the log of an Odoo used
to testing, and avoid silently ignoring logged errors.

- intercept odoo's stderr via a pipe, that way we can still write it
  back out and pytest is able to read & buffer it, pytest's capfd
  would not work correctly: it breaks output capturing (and printing
  on failure); and because of the way it hooks in it's unable to
  capture from subprocesses inheriting the standard stream, cf
  pytest-dev/pytest#4428
- update the env fixture to check that the odoo log doesn't have any
  exception on failure
- make that check conditional on the `expect_log_errors` marker, this
  way we can mark tests for which we expect errors to be logged, and
  assert that that does happen
2024-06-12 15:09:42 +02:00
Xavier Morel
60c4b5141d [FIX] runbot_merge: leftover direct setting of PR state
Setting the PR state directly really doesn't work as it doesn't
correctly save (and can get overwritten by any dependency of which
there are many).

This caused setting odoo/odoo#165777 in error to fail, leading to it
being re-staged (and failing) repeatedly, and the PR being spammed
with comments.

- create a more formal helper for preventing directly setting computed
  functions (without an actual inverse)
- replace direct state setting by setting the corresponding dependency
  e.g. `error` for error and `skipchecks` to force a PR to ready
- add a `skipchecks` inverse to the PR so it can also set itself as
  reviewed, and is convenient, might be worth also adding stuff to
  `Batch.write`
2024-06-11 15:41:20 +02:00
Xavier Morel
e320de0439 [FIX] runbot_merge: handle gh comments ending with newlines
Regex `$` apparently does not quite strip that out.
2024-06-11 15:24:09 +02:00
Xavier Morel
c67325fdab [IMP] forwardport: don't ping *every* forwardport
98aaa910 updated the forwardport notifications system to notify on the
forward ports rather than the source, to try and mitigate or at least
shift some of the spam: spam the followers of the original
source (which might be many people) somewhat less, at the possible
cost of spamming the author and reviewer more because they get a
message per forgotten forward port.

This change aims to alleviate part of the latter, by only notifying on
PRs which actually need to be r+'d, and not notifying on those which
will implicitly "inherit" the reviews. This should cut down on
redundant notifications and let users focus on the important ones.
2024-06-10 18:47:49 +02:00
Xavier Morel
187f7f6429 [CHG] runbot_merge: allow pr author to approve all fw
- trigger FW section on all forward ports, not just attached ones
- allow author of original PR to approve any fwport
2024-06-10 15:21:24 +02:00
Xavier Morel
e403593799 [FIX] runbot_merge: incorrect computation dependencies
`Batch.staging_ids` is a computed field, it can't be used as a
dependency for an other compute (at least not in 15.0).
2024-06-10 14:31:02 +02:00
Xavier Morel
14a2b0068d [FIX] runbot_merge: type error in conflict handling 2024-06-10 14:29:55 +02:00
Xavier Morel
4af515b20d [IMP] runbot_merge: stagings button wrapping
Because one of the previous commits adds the duration of the staging
to the staging dropdown toggles, it's now much longer, and by default
the text does not wrap so it looks like shit and goes completely out
the column "CSS is awesome" style.

Update the style of the dropdown toggles specifically to allow text
wrapping. Also align them left instead of centering, because the text
makes a centered layout super ugly.
2024-06-07 17:07:10 +02:00
Xavier Morel
2fb85c515e [ADD] runbot_merge: missing staging migration
44084e303c changed the interpretation
and schema of the `statuses_cache` field on stagings, but I forgot to
add a migration, so it would just blow up on opening the home
dashboard or the staging lists.
2024-06-07 17:05:58 +02:00
Xavier Morel
f4035932e3 [IMP] runbot_merge: add staging status to dashboard
The dashboard can be a bit unclear as to the state of a PR when
everything's gone well. Make it more clear / explicit that it's ready
or staged.

Fixes #888
2024-06-07 15:51:26 +02:00
Xavier Morel
ceb2bd457e [IMP] forwardport: automatically approve freeze backfills
if the corresponding master PR was approved. That the backfill needs
approval can be considered a race condition: its dual is approved
and *might* have been merged before the freeze, but was not, so a
backfill was needed.

Copying over the approval is a convenience feature with pretty much no
actual risk I can think of.

Fixes #884
2024-06-07 15:02:28 +02:00
Xavier Morel
fec3d39d19 [ADD] *: per-repository webhook secret
Currently webhook secrets are configured per *project* which is an
issue both because different repositories may have different
administrators and thus creates safety concerns, and because multiple
repositories can feed into different projects (e.g. on mergebot,
odoo-dev/odoo is both an ancillary repository to the main RD project,
and the main repository to the minor / legacy master-wowl
project). This means it can be necessary to have multiple projects
share the same secret as well, this then mandates the secret for more
repositories per (1).

This is a pain in the ass, so just detach secrets from projects and
link them *only* to repositories, it's cleaner and easier to manage
and set up progressively.

This requires a lot of changes to the tests, as they all need to
correctly configure the signaling.

For `runbot_merge` there was *some* setup sharing already via the
module-level `repo` fixtures`, those were merged into a conftest-level
fixture which could handle the signaling setup. A few tests which
unnecessarily set up repositories ad-hoc were also moved to the
fixture. But for most of the ad-hoc setup in `runbot_merge`, as well
as `forwardport` where it's all ad-hoc, events sources setup was just
appended as is. This should probably be cleaned up at one point, with
the various requirements collected and organised into a small set of
fixtures doing the job more uniformly.

Fixes #887
2024-06-06 11:07:57 +02:00
Xavier Morel
cc9f239261 [IMP] forwardport: use ORT and zdiff3 for cherrypicks
- zdiff3 should provide better conflict annotations than diff3
- ORT might also provide less conflicts period
- always perform cherrypicks without rename limit, since we're
  re-trying every failure without rename limit, it seems like
  unnecessary work, assuming git only ever looks as far as it needs
- also enable copy support maybe...

Fixes #827
2024-06-04 14:21:15 +02:00
Xavier Morel
c1e2e5a2e0 [REF] forwardport: update re_matches to not use a regex
Using a regex as the pattern is quite frustrating due to all the
escaping necessary, which in this refactoring I found out I'd missed,
multiple times.

Convert the pattern to something bespoke but not too complicated, we
may want to add anchoring support and a bit more finesse and the
future but for now straightforward "holes" seem to work well. I've
added support for capturing and even named groups even if this as yet
unnecessary and unused.

Fixes #861

[^1]: https://docs.pytest.org/en/stable/reference.html#pytest.hookspec.pytest_assertrepr_compare
2024-06-04 14:18:04 +02:00
Xavier Morel
98aaa9107f [CHG] forwardport: notify the outstanding forwardports rather than source
I have been convinced that this might be an improvement to the affairs
of the people: originally the message was sent to the source PR so we
wouldn't have to ping the author & reviewer and to limit the amount of
spam, *however*:

- we ended up adding pings anyway
- it also pings the followers of the source PR
- it increases the size of the original discussion (especially if was
- originally long)
- it adds steps to fixing the issue as you need to bounce from the
  source to the forward ports

Note that this might still notify a lot of people as they might be
made followers of the forward ports automatically, and it increases
the messaging load of the forwardbot significantly. But we'll see how
things go. Worst case scenario, we can revert it back.

Fixes #836
2024-06-04 08:56:51 +02:00
Xavier Morel
9c51f87aed [ADD] runbot_merge: support for non-webhook staging validation
Add support for the ability to validate *stagings* over RPC rather
than via webhook. This may later be expanded to PRs as well.

The core motivation for this is to avoid bouncing through github which
sometimes drops the ball on statuses, and it's frustrating to have a
staging time out because GH fucked up.

Implemented via RPC, requiring both the staging itself (by id) and the
head commit being affected, as that is necessary to know what CIs are
required for that head and correctly report cross branch on the
various PRs.

Fix #881 (kinda)
2024-06-04 08:56:51 +02:00
Xavier Morel
44084e303c [REF] runbot_merge: compute staging state
Rather than compute staging state directly from commit statuses, copy
statuses into the staging's `statuses_cache` then compute state based
on that. Also refactor `statuses` and `staging_end` to be computed
based on the statuses cache instead of the commits (or `state`
update).

The goal is to allow non-webhook validation of stagings, for direct
communications between the mergebot and the CI (mostly runbot).
2024-05-31 12:33:13 +02:00
Xavier Morel
68cfeddaed [ADD] runbot_merge: display required statuses after merge
Github makes it painfully difficult to access the statuses (especially
their URL / related build) once a PR has been merged, as it's
necessary to find the last non-staging commit mention / update in
order to find its statuses checkbox thingie, open that, and access the
statuses.

The mergebot has all the links, so it can just display them in the
merged mode as well rather than only display them in open mode. That
way even on a merged PR the statuses are just two clicks away.

Fixes #873
2024-05-30 15:28:25 +02:00
Xavier Morel
67f1c1e288 [IMP] runbot_merge: add staging duration
Computed on the fly for now. Formatted nicely in the frontend, there
does not seem to be any sort of duration widget in the backend so
just display the integer number of seconds.

Fixes #865
2024-05-30 15:11:38 +02:00
Xavier Morel
3f4519d605 [CHG] runbot_merge: add signoff & related to all commits
if rebased. Untouched commits (straight merge) remain unalterated, but
all rebased or squashed commits now get signoff and `Related` headers
added on top of the already previously added `part-of`.

Implement by generalising `_build_merge_message` to `_build_message`
and having `add_self_references` delegate to it, removes some of the
redundancy / differential handling.

Update the `part_of` helper to also add the S-O-B header to the PR,
although it currently does not reference the entire forward port
chain.

Fixes #876
2024-05-30 10:59:07 +02:00
Xavier Morel
5703513c46 [MERGE] runbot_merge, forwardport: v2 model
Massive rewrite of commands set, state management, and batch
management:

- the commands set was largely rewritten, removing the interactions
  with the fwbot entirely (only one bot receives commands anymore),
  and commands were made more flexible & orthogonal, also parsing was
  formalised
- state management was rewritten to better leverage computes
- batches are now persistent rather than being ad-hoc staging-only
  concepts, a batch is still a horizontal set of PRs but it now
  "keeps" through multiple stagings and batches can be linked
- which allows a v2 dashboard showing a complete overview of the
  batches through forward-ports, including showing the overview on the
  PR itself
2024-05-29 10:36:57 +02:00
Xavier Morel
3c3100adfe [IMP] runbot_merge: cleanup PR backend
Shove a bunch of stuff in notebook tabs, add a few
affordances (e.g. github and frontend links, links from m2m), surface
a few missing fields.

Hopefully makes the backend form both easier to navigate and easier to
administrate from.
2024-05-29 07:55:07 +02:00
Xavier Morel
232aa271b0 [ADD] runbot_merge: PR dashboard V2
Displays the entire batch set as a table, along both
repository (linked PRs) and branch (forward ports). Should provide a
much more complete overview.

Adds a copy of the dashboard as a raster render, to link from the PR:
as usual SVG is shit, content-based viewboxes are hell and having to
duplicate the entire CSS because `<img/>`-linked CSS can't run is
gross. And there's no payoff since the image is not interactible
anyway.

Performing manual ad-hoc table rendering via pillow is not
significantly worse, it works fine and it's possible to do *really*
good conditional request handling (hopefully) because I've basically
got all the information I need right here.

In fact it might make sense to upgrade the regular HTML page with
similar conditional request handling, at least for the last-update
bit if not the etag.

Fixes #771,fixes #770
2024-05-29 07:55:07 +02:00
Xavier Morel
3191c44459 [ADD] runbot_merge: synthetic batches & stagings to freeze wizard
Merged PRs should have a batch which should have a staging, this makes
the treatment uniform across the board and avoids funky data which is
hard to place or issues when reconstructing history.

Also create synthetic batches & stagings for older freezes (and bumps)
2024-05-29 07:55:07 +02:00
Xavier Morel
bbce5f8f46 [IMP] *: don't remove PRs from batches on close
Initially wanted to skip this only for FW PRs, but after some thinking
I feel this info could still be valuable even for non-fw PRs which
were never merged in the first place.

Requires a few adjustments to not break *everything*: `batch.prs`
excludes closed PRs by default as most processes only expect to be
faced by a closed PR inside a batch, and we *especially* want to avoid
that before the batch is merged (as we'd risk staging a closed PR).

However since PRs don't get removed from batches anymore (and batches
don't get deleted when they have no PRs) we now may have a bunch of
batches whose PRs (usually a single one) are all closed, this has two
major side-effects:

- a new PR may get attached to an old batch full of closed PRs (as
  batches are filtered out on being *merged*), which is weird
- the eventual list of batches gets polluted with a bunch of
  irrelevant batches which are hard to filter out

The solution is to reintroduce an `active` field, as a stored compute
field based on the state of batch PRs. This way if all PRs of a batch
are closed it switches to inactive, and is automatically filtered out
by search which solves both issues.
2024-05-29 07:55:07 +02:00
Xavier Morel
0e0348e4df [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.
2024-05-29 07:55:07 +02:00
Xavier Morel
e7e81bf375 [IMP] *: handle the addition of a new PR to a fw-ported batch
Given a batch which has been merged, and been forward-ported, to
multiple branches (because skipci was set or ci passed on the repos
the batch covers).

There might come the need to add a PR for one of the uncovered
repos. This raises the question of what to do with it, since the
forward-ports for the batch already exist it's not going to get
forwardported normally, nor may we want to, possibly?

Options are:

- don't do anything, such additions don't get ported, this is
  incongruous and unexpected as by default PRs are forward-ported, and
  if the batch wasn't an intermediate (but e.g. a conflict) it
  probably would be ported forward
- port on merge, this allows configuring the PR properly (as it might
  need its own limit) but it means further batches may get
  unexpectedly merged (or at least retied) without the additional PR
  even though we likely want it in
- immediately port the additional PR on creation, this makes the limit
  harder or impossible to configure but it makes the *batch sequence*
  more consistent

We ended up selecting the latter, it feels closer to the updates
system, and it creates more consistent batches through the
sequence. It's also technically easier to ad-hoc port a PR through a
bunch of branches than it is to update the "normal" forward-port
process to handle partial fixups.
2024-05-29 07:55:07 +02:00
Xavier Morel
1e9fa48652 [ADD] runbot_merge: migration of models refactoring
This is definitely non-trivial, due to the structural changes and the
amounts of stuff to move around (e.g. lift from PR to batch), as well
as the reification of previously non-existent relations (batches,
batch history, ...) which sometimes uncovers inconsistencies in the
current state of the mergebot (some of which are the result of bugs,
the bug got fixed but the nonsense it generated was left untouched).
2024-05-29 07:55:02 +02:00
Xavier Morel
6972749fee [IMP] forwardport: remove duplicate-ish make_basic in test_weird
The one from the utils can easily be adapted for the job, and
documented while at it, no need for an extra 50 lines or so of
complete duplicate.
2024-05-24 09:08:56 +02:00