Commit Graph

386 Commits

Author SHA1 Message Date
Xavier Morel
73e4ac6066 [REM] runbot_merge: check_visibility
Its sole use was removed with the switch to local staging, but I
missed removing it.

Closes #625 as there is no need to update it to v2 smart protocol.
2023-08-29 13:26:12 +02:00
Xavier Morel
b0b609ebe7 [CHG] runbot_merge: perform stagings in a local clone of the repo
The github API has gotten a lot more constraining (with rate
restrictions being newly enforced or added somewhat out of nowhere),
and as importantly a lot less reliable. So move the staging process
off of github and locally, similar to the forward porting
process (whose repo cache is being reused for this).

Fixes #247
2023-08-25 15:33:25 +02:00
Xavier Morel
f0344fd34a [ADD] runbot_merge: link back from commit to PR 2023-08-25 15:31:06 +02:00
Xavier Morel
4d2c0f86e1 [CHG] runbot_merge: convert freeze wizard to local repo
Probably less necessary than for the regular staging stuff, but might
as well while at it.

Requires updating one of the test to generate a non-ff push, as
O_CREAT doesn't exist at the git level, and the client (and it is
client-side) only protects against force pushes. So there is no way to
trigger an issue with just the creation of the new branch, it needs to
exist *and point to a non-ancestor commit*.

Also remove a sleep in the ref update loop as there are no ref updates
anymore, until the very final sync via git.

NB: maybe it'd be possible to push both bump and release PRs together
for each repo, but getting which update failed in case of failure
seems difficult.
2023-08-25 15:06:04 +02:00
Xavier Morel
85a7890023 [CHG] runbot_merge: switch staging from github API to local
It has been a consideration for a while, but the pain of subtly
interacting with git via the ignominous CLI kept it back. Then ~~the
fire nation attacked~~ github got more and more tight-fisted (and in
some ways less reliable) with their API.

Staging pretty much just interacts with the git database, so it's both
a facultative github operator (it can just interact with git directly)
and a big consumer of API requests (because the git database endpoints
are very low level so it takes quite a bit of work to do anything
especially when high-level operations like rebase have to be
replicated by hand).

Furthermore, an issue has also been noticed which can be attributed to
using the github API (and that API's reliability getting worse): in
some cases github will fail to propagate a ref update / reset, so when
staging 2 PRs it's possible that the second one is merged on top of
the temporary branch of the first one, yielding a kinda broken commit
(in that it's a merge commit with a broken error message) instead of
the rebase / squash commit we expected.

As it turns out it's a very old issue but only happened very early so
was misattributed and not (sufficiently) guarded against:

- 41bd82244bb976bbd4d4be5e7bd792417c7dae6b (October 8th 2018) was
  spotted but thought to be a mergebot issue (might have been one of
  the opportunities where ref-checks were added though I can't find
  any reference to the commit in the runbot repo).
- 2be25052e147b151d1d8a5bc73cceb351586ce03 (October 15th, 2019) was
  missed (or ignored).
- 5a9fe7a7d05a9df7186072a7bffd60c6b428fd0e (July 31st, 2023) was
  spotted, but happened at a moment where everything kinda broke
  because of github rate-limiting ref updates, so the forensics were
  difficult and it was attributed to rate limiting issues.
- f10d03bf0f2e8f88f62a5d8356b84f714196130f (August 24th, 2023) broke
  the camel's back (and the head block): the logs were not too
  interspersed with other garbage and pretty clear that github ack'd a
  ref update, returned the correct oid when checking the ref, then
  returned the wrong oid when fetching it later on.

No Working Copy
===============

The working copy turns out to not be necessary, the plumbing commands
we *need* work just fine on a bare repository.

Working without a WC means we had to reimplement the high level
operations (rebase) by hand much as we'd done previously, *but* we
needed to do that anyway as git doesn't seem to provide any way to
retrieve the mapping when rebasing/cherrypicking, and cherrypicking by
commit doesn't work well as it can't really find the *merge base* it
needs.

Forward-porting can almost certainly be implemented similarly (with
some overhead), issue #803 has been opened to keep track of the idea.

No TMP
======

The `tmp.` branches are no more, the process of creating stagings is
based entirely around oids, if staging something fails we can just
abandon the oids (they'll be collected by the weekly GC), we only
need to update the staging branches at the very end of the process.

This simplifies things a fair bit.

For now we have stopped checking for visibility / backoff as we're
pushing via git, hopefully it is a more reliable reference than the
API.

Commmit Message Formatting
==========================

There's some unfortunate churn in the test, as the handling of
trailing newlines differs between github's APIs and git itself.

Fixes #247

PS: It might be a good idea to use pygit2 instead of the CLI
    eventually, the library is typed which is nice, and it avoids
    shelling out although that's really unlikely to be a major cost.
2023-08-25 15:06:04 +02:00
Xavier Morel
2fbbe3fcdb [ADD] runbot_merge: github identity for the mergebot
Necessary to create commits *as* the mergebot without going through
the github API. Copy of the improved version from forwardport. *Not*
an override, to avoid unnecessarily triggering one or the other which
is confusing and weird.
2023-08-25 15:04:48 +02:00
Xavier Morel
86a1b5523e [MOV] runbot_merge: all the staging creation code to a separate module
Move *almost* all the staging code to free functions, in a separate
module, and extensively typed.

The only bits which didn't move are:

- the entry point (the cron hook), because it has to be a model method
  in order to be called
- the `_build_merge_message` method, because it needs to be
  overridable

There's also a bit of an import mess, because the cron &
`_build_merge_message` need to call into the new module, but the new
module wants the types they belong to, so it's a bit circular.
2023-08-25 15:04:48 +02:00
Xavier Morel
9de18de454 [CHG] *: move repo cache from forwardbot to mergebot
If the stagings are going to be created locally (via a git working
copy rather than the github API), the mergebot part needs to have
access to the cache, so move the cache over. Also move the maintenance
cron.

In an extermely minor way, this prefigures the (hopeful) eventual
merging of the ~~planes~~ modules.
2023-08-25 15:04:48 +02:00
Xavier Morel
7bca6f0bd7 [ADD] runbot_merge: allow resolving commits by sha
`_rec_name = 'sha'` means name_search and cross-model searches will
work much better.

Relates to #802
2023-08-25 11:01:46 +02:00
Xavier Morel
0826b3484b [ADD] runbot_merge: view improvements
- add formatting for a bunch of backend objects
- add cross-links in order to use toplevel navigation between objects
  e.g. project -> branch -> staging -> PR with breadcrumbs instead of
  shitty dialog boxes

Relates to #802
2023-08-25 11:01:38 +02:00
Xavier Morel
9b5bb338b4 [REM] runbot_merge: status compatibility functions
When I updated the status storage (including `previous_failure`) for
some reason I didn't just migrate from the old to the new format, and
added bridge functions instead.

This is not really necessary (or useful), so convert all the legacy
data and remove the conversion helpers.

Relates to #802
2023-08-24 10:47:16 +02:00
Xavier Morel
90961b99c9 [ADD] *: changelog entries I forgot
Can't hurt to *have* them.
2023-08-14 09:28:19 +02:00
Xavier Morel
7348e4d7a4 [IMP] runbot_merge: ensure at least 1s between mutating GH calls
Mostly a temporary safety feature after the events of 07-31: it's
still not clear whether that was a one-off issue or a change in
policy (I was not able to reproduce locally even doing several
set_refs a second) and the gh support is not super talkative, but it
probably doesn't hurt to commit the workaround until #247 gets
implemented.

On 2023-07-31, around 08:30 UTC, `set_ref` started failing, a lot
(although oddly enough not continuously), with the unhelpful message
that

> 422: Reference cannot be updated

This basically broke all stagings, until a workaround was implemented
by adding a 1s sleep before `set_ref` to ensure no more than 1
`set_ref` per second, which kinda sorta has been the github
recommendation forever but had never been an issue
before. Contributing to this suspicion is that in late 2022, the
documentation of error 422 on `PATCH git/refs/{ref}` was updated to:

> Validation failed, or the endpoint has been spammed.

Still would be nice if GH was clear about it and sent a 429 instead.

Technically the recommendation is:

> If you're making a large number of POST, PATCH, PUT, or DELETE
> requests for a single user or client ID, wait at least one second
> between each request.

So... actually implement that. On a per-worker basis as for the most
part these are serial processes (e.g. crons), we can still get above
the rate limit due to concurrent crons but it should be less likely.

Also take `Retry-After` in account, can't hurt, though we're supposed
to retry just the request rather than abort the entire thing. Maybe a
future update can improve this handling.

Would also be nice to take `X-RateLimit` in account, although that's
supposed to apply to *all* requests so we'd need a second separate
timestamp to track it. Technically that's probably also the case for
`Retry-After`. And fixing #247 should cut down drastically on the API
calls traffic as staging is a very API-intensive process, especially
with the sanity checks we had to add, these days we might be at 4
calls per commit per PR, and up to 80 PRs/staging (5 repositories and
16 batches per staging), with 13 live branches (though realistically
only 6-7 have significant traffic, and only 1~2 get close to filling
their staging slots).
2023-08-11 12:32:21 +02:00
Xavier Morel
85a74a9e32 [ADD] runbot_merge: staging query endpoints
`/runbot_merge/stagings`
========================

This endpoint is a reverse lookup from any number of commits to a
(number of) staging(s):

- it takes a list of commit hashes as either the `commits` or the
  `heads` keyword parameter
- it then returns the stagings which have *all* these commits as
  respectively commits or heads, if providing all commits for a
  project the result should always be unique (if any)
- `commits` are the merged commits, aka the stuff which ends up in the
  actual branches
- `heads` are the staging heads, aka the commits at the tip of the
  `staging.$name` branches, those may be the same as the corresponding
  commit, or might be deduplicator commits which get discarded on
  success

`/runbot_merge/stagings/:id`
============================

Returns a list of all PRs in the staging, grouped by batch (aka PRs
which have the same label and must be merged together).

For each PR, the `repository` name, `number`, and `name` in the form
`$repository#$number` get returned.

`/runbot_merge/stagings/:id1/:id2`
==================================

Returns a list of all the *successfully merged* stagings between `id1`
and `id2`, from oldest to most recent. Individual records have the
form:

- `staging` is the id of the staging
- `prs` is the contents of the previous endpoint (a list of PRs
  grouped by batch)

`id1` *must* be lower than `id2`.

By default, this endpoint is inclusive on both ends, the
`include_from` and / or `include_to` parameters can be passed with the
`False` value to exclude the corresponding bound from the result.

Related to #768
2023-08-11 11:13:34 +02:00
Xavier Morel
4eefc980bb [IMP] runbot_merge: logger messages 2023-08-10 16:14:33 +02:00
Xavier Morel
e9f7252ed1 [FIX] runbot_merge: sentry issue via monkeypatch
`auto_session_tracking` causes issues when not specified on the super
old version of the client which is available on ubuntu.

Also disable tracing as it seems less useful than hoped for, and I've
not been using what's been collected so far.
2023-08-10 15:27:20 +02:00
Xavier Morel
b1af2e573a [IMP] runbot_merge: split staging heads out to join tables
Currently the heads of a staging (both staging heads and merged heads)
are just JSON data on the staging itself. Historically this was
convenient as the heads were mostly of use to the staging process, and
thus accessed directly through the staging essentially exclusively.

However this makes finding stagings from merged commits e.g. for
forensic research almost impossible, because querying based on
the *values* of a JSON map is expensive, and indexing it is difficult.

To make this use case more feasible, split the `heads` field into two
join tables, one for the staging heads and one for the merged heads,
this makes looking for stagings by commits much more
efficient (although the queries may not be trivial). Also add two
utility RPC methods, so that it's possible to query stagings
reasonably easily and efficiently based on a set of commits (branch
heads).

related to #768
2023-08-10 14:04:59 +02:00
Xavier Morel
cdffa83191 [IMP] runbot_merge, forwardport: minor cleanups
Remove unused imports, unnecessary f-strings, dead code, fix
less-than-ideal operators.
2023-08-10 13:33:16 +02:00
Xavier Morel
a692163f6e [IMP] runbot_merge: add quick jump from stagings to PRs
In the backend, the intermediate jump through batches is really not
convenient (even if we kinda have to jump through batches *anyway*).

Fixes #751
2023-07-10 15:23:31 +02:00
Xavier Morel
780e20bfd6 [IMP] runbot_merge: filtering options and UX on stagings list
Allow filtering stagings by state (success or failure), and provide a
control to explicitly update the staging date limit.

Should make it easier to drill through stagings when looking for
specific information.

Related to #751
2023-07-10 15:23:31 +02:00
Xavier Morel
5bce73c97d [IMP] *: optimise loading of home page
Fix outstanding query to make a positive `state` filtering, instead of
negative, matching 3b52b1aace8674259812a76b1566260937dbcacb.

Also manually create a map of stagings (grouped by branch) sharing a
single prefetch set.

For odoo the mergebot home page has 12 branches in the odoo project
and 8 in spreadsheet, 6 stagings each. This means 120 queries to
retrieve all the heads (Odoo stagings have 5 heads and spreadsheet
have 1, but that seems immaterial).

By fixing `_compute_statuses` and creating a single prefetch set for
all stagings of all branches we can fetch all the commits in a single
query instead of 120.
2023-07-10 15:23:31 +02:00
Xavier Morel
81ce4ea02b [IMP] rewrite /forwardport/outstanding
- add support for authorship (not just approval)
- make display counts directly
- fix `state` filter: postgres can't do negative index lookups
- add indexes for author and reviewed_by as we look them up
- ensure we handle the entire source filtering via a single subquery

Closes #778
2023-07-10 15:23:31 +02:00
Xavier Morel
765281a665 [FIX] runbot_merge: make provisioning more resilient
A few cases of conflict were missing from the provisioning
handler.

They can't really be auto-fixed, so just output a warning and ignore
the entry, that way the rest of the provisioning succeeds.
2023-06-21 14:26:19 +02:00
Xavier Morel
9260384284 [FIX] runbot_merge: concurrency error in freeze wizard (hopefully)
During the 16.3 freeze an issue was noticed with the concurrency
safety of the freeze wizard (because it blew up, which caused a few
issues): it is possible for the cancelling of an active staging to the
master branch to fail, which causes the mergebot side of the freeze to
fail, but the github state is completed, which puts the entire thing
in a less than ideal state.

Especially with the additional issue that the branch inserter has its
own concurrency issue (which maybe I should fix): if there are
branches *being* forward-ported across the new branch, it's unable to
see them, and thus can not create the now-missing PRs.

Try to make the freeze wizard more resilient:

1. Take a lock on the master staging (if any) early on, this means if
   we can acquire it we should be able to cancel it, and it won't
   suffer a concurrency error.
2. Add the `process_updated_commits` cron to the set of locked crons,
   trying to read the log timeline it looks like the issue was commits
   being impacted on that staging while the action had started:
   REPEATABLE READ meant the freeze's transaction was unable to see
   the update from the commit statuses, therefore creating a diverging
   update when it cancelled the staging, which postgres then reported
   as a serialization error.

I'd like to relax the locking of the cron (to just FOR SHARE), but I
think it would work, per postgres:

> SELECT FOR UPDATE, and SELECT FOR SHARE commands behave the same as
> SELECT in terms of searching for target rows: they will only find
> target rows that were committed as of the transaction start
> time. However, such a target row might have already been updated (or
> deleted or locked) by another concurrent transaction by the time it
> is found. In this case, the repeatable read transaction will wait
> for the first updating transaction to commit or roll back (if it is
> still in progress). If the first updater rolls back, then its
> effects are negated and the repeatable read transaction can proceed
> with updating the originally found row. But if the first updater
> commits (and actually updated or deleted the row, not just locked
> it) then the repeatable read transaction will be rolled back with
> the message

This means it would be possible to lock the cron, and then get a
transaction error because the cron modified one of the records we're
going to hit while it was running: as far as the above is concerned
the cron's worker had "just locked" the row so it's fine to
continue. However this makes it more and more likely an error will be
hit when trying to freeze (to no issue, but still). We'll have to see
how that ends up.

Fixes #766 maybe
2023-06-21 14:26:19 +02:00
Xavier Morel
ed0fd88854 [ADD] runbot_merge: sentry instrumentation
Currently sentry is only hooked from the outside, which doesn't
necessarily provide sufficiently actionable information.

Add some a few hooks to (try and) report odoo / mergebot metadata:

- add the user to WSGI transactions
- add a transaction (with users) around crons
- add the webhook event info to webhook requests
- add a few spans to the long-running crons, when they cover multiple
  units per iteration (e.g. a span per branch being staged)

Closes #544
2023-06-21 14:26:19 +02:00
Xavier Morel
06a3a1bab5 [IMP] runbot_merge: add sentry filtering, rework some error messages
- move sentry configuration and add exception-based filtering
- clarify and reclassify (e.g. from warning to info) a few messages
- convert assertions in rebase to MergeError so they can be correctly
  logged & reported, and ignored by sentry, also clarify them
  (especially the consistency one)

Related to #544
2023-06-15 08:21:20 +02:00
Xavier Morel
cd4ded899b [IMP] runbot_merge: error reporting
Largely informed by sentry,

- Fix an inconsistency in staging ref verification, `set_ref`
  internally waits for the observed head to match the requested head,
  but then `try_staging` would re-check that and immediately fail if
  it didn't.

  Because github is *eventually* consistent (hopefully) this second
  check can fail (and is also an extra API call), breaking staging
  unnecessarily, especially as we're still going to wait for the
  update to be visible to git.

  Remove this redundant check entirely, as github provides no way to
  ensure we have a consistent view of anything, it doesn't have much
  value and can do much harm.
- Add github request id to one of the sanity check warnings as that
  could be a useful thing to send upstream, missing github request ids
  in the future should be noted and added.
- Reworked the GH object's calls to be clearer and more coherent:
  consistently log the same thing on all GH errors (if `check`),
  rather than just on the one without a `check` entry.

  Also remove `raise_for_status` and raise `HTTPError` by hand every
  time we hit a status >= 400, so we always forward the response body
  no matter what its type is.
- Try again to log the request body (in full as it should be pretty
  small), also remove stripping since we specifically wanted to add a
  newline at the start, I've no idea what I was thinking.

Fixes #735, #764, #544
2023-06-14 16:01:45 +02:00
Xavier Morel
270dfdd495 [REF] *: move most feedback messages to pseudo-templates
Current system makes it hard to iterate feedback messages and make
them clearer, this should improve things a touch.

Use a bespoke model to avoid concerns with qweb rendering
complexity (we just want GFM output and should not need logic).

Also update fwbot test setup to always configure an fwbot name, in
order to avoid ping messages closing the PRs they're talking
about, that took a while to debug, and given the old message I assume
I'd already hit it and just been too lazy to fix. This requires
updating a bunch of tests as fwbot ping are sent *to*
`fp_github_name`, but sent *from* the reference user (because that's
the key we set).

Note: noupdate on CSV files doesn't seem to work anymore, which isn't
great. But instead set tracking on the template's templates, it's not
quite as good but should be sufficient.

Fixes #769
2023-06-14 16:01:45 +02:00
Xavier Morel
e14616b2fb [IMP] runbot_merge: add support for draft check
1cea247e6c missed the update of the
`draft` flag, add support for it.

Fixes #753
2023-06-14 16:01:45 +02:00
Xavier Morel
2009177ada [IMP] *: allow disabling staging on branch, remove fp target flag
- currently disabling staging only works globally, allow disabling on
  a single branch

  - use a toggle
  - remove a pair of tests which work specifically with `fp_target`,
    can't work with `active` (probably)
  - cleanup search of possible and active stagings, add relevant
    indexes and use direct search of relevant branches instead of
    looking up from the project

- also use toggle button for `active` on branches
- shitty workaround for upgrading DB: apparently mail really wants to
  have a `user_id` to do some weird thing, so need to re-add it after
  resetting everything

Fixes #727
2023-06-14 16:01:42 +02:00
Xavier Morel
4a4252b4b9 [FIX] runbot_merge: holes in provisioning
- github logins are case-insensitive while the db field is CI the dict
  in which partners are stored for matching is not, And the caller may
  not preserve casing.

  Thus it's necessary to check the casefolded database values against
  casefolded parameters, rather than exactly.
- users may get disabled by mistake or when one leaves the project,
  they may also get switched from internal to portal, therefore it is
  necessary to re-enable and re-enroll them if they come back.
- while at it remove the user's email when they depart, as they likely
  use an organisational email which they don't have access to anymore

Side-note, but remove the limit on the number of users / partners
being created at once: because there are almost no modules in the
mergebot's instance, creating partner goes quite fast (compared to a
full instance), thus the limitation is almost certainly unnecessary
(creating ~300 users seems to take ~450ms).

Fixes ##776
2023-06-14 16:01:42 +02:00
Xavier Morel
611f9150ff [IMP] runbot_merge: add signed kw support to from_role, use it
Closes #774
2023-06-14 16:01:42 +02:00
Xavier Morel
485d2d7b55 [IMP] runbot_merge: add sitemap params to http controllers
When it's missing, website complains because it's dumb.

Fixes #763
2023-06-14 16:01:42 +02:00
Xavier Morel
4f237d15b0 [FIX] runbot_merge: correctly check request in handle_pr
652b1ff9ae wanted to check if a request
was available, however it deref'd the `request` object without
checking it which is not correct: a `request` normally has an
`httprequest`, but the `request` itself might be missing if the
handler is called from e.g. a cron.

Fixes #739
2023-06-14 16:01:42 +02:00
Xavier Morel
6bc6dd77ab [FIX] runbot_merge: mismatch can contain non-str values
The mismatch diff attribute contains values from the in-db object and
the github PR structure, some of which are explicitly *not*
strings (e.g. the squash flag, possibly the commits # in the future).

As a result, when the squash-flag of a PR differs from the actual the
formatting for diffing blows up, because difflib can't handle
non-strings.

Stringify values between passing them to `format_items`, this way the
string operations on names and values should work correctly.
2023-04-17 08:27:57 +02:00
Xavier-Do
2ad188201b [IMP] runbot_merge: speedup frontend page
The mergebot page become a bit slow with the years, it is time to make
small optimisation to speed up thinks a little.

Note: all changes where applied modifying the views or adding index by
hand. There is still room for improvement but it would need more in
depth refactoring, mainly adding specialized computed fields to
enable a better batching.

The first issue was using branch.staging_ids

    branch.staging_ids.sorted(lambda s: s.staged_at, reverse=True)[:6]

The number of staging_ids is increasing and prefetching + sorting all
of them is slow.

The proposed solution is to replace it by a search, not ideal, a
specialized compute field may be a good idea, but this is a quick fix
that can be done editing a view.

    branch.env['runbot_merge.stagings'].search([('target', '=', branch.id)],order='staged_at desc', limit=6)

Other changes are just index on critical columns.

Before changes, /runbot_merge page takes ~5s to load
After changes,  /runbot_merge page takes ~1s to load

Small note: note 100% sure that runbot_merge.batch.target was useful
2023-04-05 09:05:50 +02:00
Xavier Morel
5e1048c6da [IMP] runbot_merge: logging of gh requests
The loggers would only print the "tail" of the path, not including the
repo name, or the `/repos` prefix.

While this made logs shorter, it was not intentional and made
debugging some issues on endpoints harder than necessary as the calls
had to be adjusted mentally, which is completely unnecessary.
2023-02-21 15:11:59 +01:00
Xavier Morel
6e1fc61781 [IMP] runbot_merge: add json & requests to server actions context 2023-02-20 10:13:05 +01:00
Xavier Morel
23e1b93465 [FIX] runbot_merge: a few issues with updated staging check
1cea247e6c tried to improve staging
checks to avoid staging PRs in the wrong state, however it had two
issues:

PR state
--------

The process would reset the PR's state to open, but unless the head
was being resync'd it wouldn't re-apply the statuses on the state,
leading to a PR with all-valid statuses, but a missing CI.

Message
-------

The message check didn't compose the PR message the same way PR
creation / update did (it did not trim the title and description
individually, only after concatenation), resulting in a
not-actually-existing divergence getting signaled in the case where
the PR title ends or the description starts with whitespace.

Expand relevant test, add a utility function to compose a PR message
and use it everywhere for coherence.

Also update the logging and reporting to show a diff of all the
updated items (hidden behind a `details` element).
2023-02-14 13:45:28 +01:00
Xavier Morel
907c6072d1 [FIX] runbot_merge: cancel master staging on freeze
If there are bump PRs anyway: the bump commits will cause the
forward-port of the staging to fail, so might as well clearly notify
everybody of the issue if there is a pending staging, and not waste
too much time waiting for a staging which can not succeed.

We could also cancel stagings when there's no bump PR, but it's not
clear that there's any reason to do so: if we didn't touch any master
branch, there's no reason for the staging to fail, or to otherwise
cancel it.

And obviously we can't have staged anything on the new branch so
there's nothing to cancel.

Part-Of: #718
2023-01-25 12:25:45 +01:00
Xavier Morel
8d7d6302d3 [FIX] runbot_merge: make freeze wizard labels lookup not shit
I DECLARE BANKRUPTCY!!!

The previous implementation of labels lookup was really not
intuitive (it was just a char field, and matched labels by equality
including the owner tag), and was also full of broken edge
cases (e.g. traceback if a label matched multiple PRs in the same repo
because people reuse branch names).

Tried messing about with contextual `display_name` and `name_search`
on PRs but the client goes wonky in that case, and there is no clean
autocomplete for non-relational fields.

So created a view which reifies labels, and that can be used as the
basis for our search. It doesn't have to be maintained by hand, can be
searched somewhat flexibly, we can add new view fields in the future
if desirable, and it seems to work fine providing a nice
understandable UX, with the reliability of using a normal Odoo model
the normal way.

Also fixed the handling of bump PRs, clearly clearing the entire field
before trying to update existing records (even with a link_to
inbetween) is not the web client's fancy, re-selecting the current
label would just empty the thing entirely.

So use a two-step process slightly closer to the release PRs instead:

- first update or delete the existing bump PRs
- then add the new ones

The second part is because bump PRs are somewhat less critical than
release, so it can be a bit more DWIM compared to the more deliberate
process of release PRs where first the list of repositories involved
has to be set up just so, then the PRs can be filled in each of them.

Fixes #697
2023-01-25 12:25:45 +01:00
Xavier Morel
ae53c87fc9 [FIX] runbot_merge: allow adding and removing release PR lines
In order to support partial freezing, we need the ability to remove
some of the release lines for the repos we don't want to
freeze (e.g. because they don't use per-version branches).

This subsequently means we need the ability to *create* new lines if
we fucked up and removed one we should not have. Alternatively the
freeze meat-bot could cancel the entire thing and redo the wizard but
that seems harsh and mean, so don't do that.

Fixes 0f3647b7c7 which specifically
mentioned partial freeze then proceeded to make them entirely
impossible anyway.

Part of #718
2023-01-25 12:25:45 +01:00
Xavier Morel
fb60c38731 [IMP] runbot_merge: add color key to freeze wizard
Was difficult to understand what the colors meant on the required PRs.

Part of #718
2023-01-25 12:25:45 +01:00
Xavier Morel
1cea247e6c [FIX] runbot_merge: sync PR target and message on check
Previously the mergebot would only sync the head commit, but synching
more is useful.

Also update the final sanity check on staging:

- as with check, update the message & target branch
- reset PR state and post a message when updating message instead of
  doing so silently

Note: maybe only fail the staging if the message is updated *and*
relevant to staging (aka there's a merge method and it's not
`rebase`)?

Fixes #680
2023-01-25 12:25:45 +01:00
Xavier Morel
652b1ff9ae [IMP] runbot_merge: more logging on synchronize
Was missing a logging message in the case where the current and sync'd
head are identical, which seems to occur from time to time but can
only be inferred (by seeing a sync event then nothing happening).

Add a logging warning (because it's a strange situation) in order to
explicitely note the issue.

Also make the sync logging messages more regular for clarity.

And add the delivery information (delivery id and user-agent) to event
log, so it's more possible to report issues to github.
2023-01-19 10:56:11 +01:00
Xavier Morel
629e1aea4a [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-08 10:46:22 +01:00
Xavier Morel
03c9a7e492 [ADD] runbot_merge: stagings canceling wizard
- allows providing a canceling reason
- allows nuking the splits at the same time (hopefully)

Fixes #661
2022-12-08 10:46:22 +01:00
Xavier Morel
2746a13163 [FIX] runbot_merge: multi-commit squashing
If commits have different authors (/ committers), the mergebot would
ask github to create a commit with an author (/ committer) of `None` /
`null`.

Apparently github really does not like that, and complains that

> nil is not an object

So remove the key entirely. Also fix the collision between `author`
and the `Co-Authored-By` list, which could lead to trying to set an
`author` of `[name, email]` instead of an object, which is also not
accepted by github.
2022-12-08 10:46:22 +01:00
Xavier Morel
0f3647b7c7 [FIX] *: freeze wizard take 3
Fixes to the new bits which didn't really work:

- Fix borked view layout
- Add some help to the label fields
- Improve the resolution of label -> pr, and fix
- Also make the feature actually work for bump PRs
- Also make pr -> label work more reliably, now allows setting one PR
  and getting the other PRs of the same batch (with the same label)
  even without setting the label by hand

An autocomplete for the label has been considered but there is no
autocomplete field for char/selection fields, and it seems way too
much work for the utility:

- either create a brand new widget for 15.0 which will have to be
  entirely rewritten in 16
- or create a transient model composed entirely of fake records to
  provide an m2o to records which don't actually exist as label
  bearers, which is also a lot of unnecessary work

NOTE: we want to support partial freezing (aka not freeze all the
      branches because some of them have different release models
      than others), so some project repos *not* having a release
      PR is fine and normal, such a validation should not be added.

Fixes #664
2022-12-08 10:46:22 +01:00
Xavier Morel
7948e59c51 [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-12-01 10:57:32 +01:00