Attempt to avoid some of the comment spam by dedup-ing input (only
signaling when the status actually changes and ignoring identity
transformations) and in case of failing CI keeping the last failed
status and not signaling on the next update if it's the same failure.
Closes#225
* only show 2 stagings on cellphones as 4 is way too much, moving to a
vertical layout would probably be a bad idea as stagings can already
be very tall and then we have multiple branches stacked on one
another, unless we also make branches foldable
the more complete list of stagings (per branch) is available on the
branch's page anyway so providing a not-completely-broken home looks
more useful, and at a fundamental level the current / last staging
is really the one we care about
* remove the size bounds on stagings to avoid smushing all the cells
together and overlapping text, sadly can't overflow scroll the
stagings element because you can't have an overflow-x: scroll and an
overflow-y: visible (that becomes auto)
When running tests on some machines, it's apparently possible for the
PR-creation webhook to come back before the PR creation request has,
leading to the creation of the PR from the API call duplicating that of
the webhook and blowing up.
To fix, immediately commit the transaction then check if we already have
the PR we just created in the system, and only create it explicitly if
not.
User should probably be warned when they try to set the limit ("up
to") in a context where it's going to be ignored:
* on a forward port PR (should be set on the source)
* on a merged source (should be set before the PR is merged)
Closes#213
The staging validation routine would ignore stagings which were
cancelled or ff_failed, but it should also have ignored failed and
successful aka all terminal state.
Simplify the condition for that: just ignore a staging's validation if
the staging is not pending.
Closes#211
In selecting the parent commits to list on the last PR, we would miss
the *first* forward-port of the sequence. Not sure why we added a
detrimental check on source_id there.
Also add a missing space between "chain" and "containing" in the case
where there's at least one forward-port PR other than the final one.
Fixes#212
Test is probably more complex than necessary (thinking about it, the
failed staging is probably unnecessary) but that triggers the issue
and matches the original scenario.
The problem was really with new CI events being received on the last
forward-port PR of a sequence: previous PRs would have a child PR so
the check would abort, but for the last PR it would go through, fail
to find an active batch, then blow up as it tries to create a
forwardport.batch without an actual batch.
Change this to use the existence of an inactive batch not linked to a
staging as a flag that the PR has been processed and forward-ported.
Closes#218
Turns out we don't want to close the cursor on success, we just want to
commit, but that's not what the default context manager does.
So don't use said context manager.
If a _validate call blows up, the entire Commit._notify cron gets
stuck, which is an issue because not only does it stop creating
forward ports, it also stops "progressing" stagings.
If the CI is greatly backed up (either insufficient capacity or jobs
spike) a timeout which is normally perfectly fine might be
insufficient e.g. given a 2h timeout, if a job normally takes 80mn but
the staging's job starts 40mn after the staging was actually created
we're sunk. And cancelling the staging once the job has finally gotten
started is not going to improve load on the CI, it just wastes a CI
slot.
Therefore assume a `pending` event denotes the actual start of the job
on the CI, and reset the timeout to start from that moment so
ci_timeout is the timeout of the CI job itself, not of the staging
having been created.
Closes#202
Converge the pytest setups of runbot_merge and forwardport a bit
more (the goal is obviously to eventually share the infrastructure so
they run the same way).
If a PR is explicitly updated, it gets converted to a normal
PR[0]. Before this, users had no indication that this had happened and
might be wondering what they're supposed to do (or try to r+ via the
forwardbot, which doesn't work on a root PR).
[0] to an extent: the PR still has a source and might have children,
in which case the followups will be created from the source &
existing followups should be updated to match
Closes#206
When creating the conflicting commit of a single commit PR, reuse the
original commit's meta-information so the developer / fixer can more
easily update it in-place.
Closes#204
In the case where an FP sequence is interrupted (e.g. there was a
conflict during one of the intermediate steps), followups get linked
to the original source but don't get linked to the "interruption" PR
which is a bit confusing.
Link FP PRs to both source and root if they're different.
* always allow specifying the PR's own branch as a forward-port limit
/ target (even if deactivated or disabled)
* add an "ignore" alias to "up to <pr target>" for clarity
* add dedicated feedback when deactivating forward-port of a PR
Fixes#191
Having all the feedback be sent by the mergebot user (github_token) is
confusing. Add a way to specify which field of project should be used to
source the token used when sending feedback.
Fixes#190
When a build_error active field is changed, the onchange leads to a
traceback. Anyway, the onchange was not a good idea as it only reflects
UI changes.
With this commit, the write method is overwritten to change the
child_ids active fields too. Also, the active_test context is used to
correctly compute the childs_ids and children_build_ids.
A test is also added for all that.
The new feature in odoo/enterprise#4879 needs the firebase-admin
package. As it cannot be added to the requirements.txt, the package is
added in the Dockerfile to be able to test it on the runbot instances.
- Add a keep running flag on the build to allow a build to stay in
running state until the flag is switched off ( or the build killed)
- Do not update configs and config_steps data
- Add a first/last_seen_build and first/last_seen_date on build.error
- Children error builds now include the parent builds too
- Use a notebook on build.error form view to display builds and linked
errors
- Update result when a build triggers a change from 'warn' to 'ko' too
- Add the sticky flag on the error logs stored sql view
When a build error appears with the same fingerprint as already known
one which was supposedly fixed, the build is simply added to the known
build error.
In order to keep an eye on such reappearing bugs and keep the fixing
history separated, this commit simply creates a new build_error.
Old build errors with the same hash (or child_ids 's hashes) appears in
a computed field error_history_ids.
When using the runbot frontend, it's sometimes very frustrating when
trying to copy branch name, some mouse gym is necessary.
With this commit, a copy to clipboard button is added near the branch
name on the frontend.
* don't warn on every PR on the dot every week, instead wait for the
PRs to be "sufficiently old" (at least 3 days)
* after discussion with bugfix, the reminder ping should be sent every
day following the PR being "old enough"
* run the cron every day instead of every week
* add an override context key for test purposes
Closes#198
Working copies were created in tempdir under the assumption that
they're, well, temporary.
However after thinking about it more there are two issues with this:
* tempdirs might not be in the same FS as the cache dir, meaning
meaning `git clone` can't hardlink the repo objects and has to
copy them
* tempdirs are often on RAM-backed tmpfs, which is not great when we're
filling them with multiple GB worth of git repository...
If the default limit of a forward-port sequence is not a valid
target (either disabled or not actually forward-ported to), the last
effective forward port in a sequence will be commented on as any
intermediate PR rather than get a proper ping and r+ instructions.
Also remove a bunch of leftover prints in the tests.
Fixes#192
* a high renamelimit increases the cherrypick runtime and can
apparently fail for reasons other than actual conflicts (cf
odoo/odoo#36893) so first attempt to cherrypick with the default
renamelimit, and fallback to renamelimit=0 if that fails
* in that case, filter out the spam of "progress" messages about
inexact rename detection as it's not really useful. There seems to
be no built-in way to suppress these.
Closes#196
Turns out bare repositories (unlike normal ones) don't have any
refspecs by default. So adding the PR refspec would... replace the
default behaviour (apparently?) and the base branches would never get
fetched at all.
--mirror looks to be an other option as it has a default refspec but
it's a bit of an odd duck: it has pull requests in refs/pull but not
all of them? And open / closed doesn't seem to be a factor.
Attempts to use rev-list seem to work locally but they fail *a lot*
when live. The previous attempt to fix it in
0f4c22490c made things worse rather than
better once deployed.
Give up on that (at least for now?), and just ask github what the PR's
commits are then cherrypick exactly that.
The ping message was not clear
- don't know if anything is needed from the author
- don't know if the last step in the chain
Ping the authors only when something needs to be done (error or last
step).
Do not ping non-reviewers as they will not have the rights to r+ or
modify the PR anyway
Closes#192
There's an issue of too many commits being selected for
cherry-picking. It still isn't quite clear, but the forward ports to
11.0 systematically seem to get everything from
5045b5f4c346e60c9b127403ef1fde453d49396a to the PR head, and that
commit was one of the first to land after the last merge-based forward
port.
So the commits selection seems to behave as if the commits range was
`..origin/pull/36692` rather than the `origin/10.0..origin/pull/36692`
which is passed to rev-list. This might be an issue of concurrent
access / update of the refs (though I can't reproduce it locally,
missing refs generate an error).
This change attempts to "pin" the local branch in the working copy
rather than go and get it from the repo. It's unclear that this will
change / fix anything, but it might just work.
Also stop creating shared clones, that seems like an unnecessary
risk (and might be the actual source of the issue).
The original method was to `git diff | git apply` in order to get a
complete overview of conflicts generated by the forward port (if
any).
However this turns out to have a huge issue in the presence of renamed
or removed files: in that case `git apply` will simply not do
anything, and fail with a completely clean working copy. Which is very
much undesirable.
-> alternative method, squash the PR to a single commit then
cherry-pick that single commit, this should provide us with proper
conflicts & their markers.
Also add tests for conflicts due to deleted files...
The new feature in odoo/enterprise#4892 needs the dbfread module.
As this lib is not required for other Odoo modules, it cannot be added
to the requirements.txt file.
In order to run the tests and use this new feature on the runbot, this
commit installs the dbfread lib in the Docker image.
* Cherrypicking is handrolled because there seems to be no easy way to
programmatically edit commit messages during the cherrypicking
sequence: `-n` basically squashes all commits and `-e` invokes a
subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it
interrupts the process before each commit), however there doesn't
seem to be clean status codes so it's difficult to know if the
cherrypick failed or if it's just waiting for a commit of this step.
Instead, cherrypick commits individually then edit / rewrite their
commit messages:
* add a reference to the original commit
* convert signed-off-by to something else as the original commit was
signed off but not necessarily this one
* Can't assign users when creating PRs: only repository collaborators
or people who commented on the issue / PR (which we're in the
process of creating) can be assigned.
PR authors are as likely to be collaborators as not, and we can have
non-collaborator reviewers. So pinging via a regular comment seems
less fraught as a way to notify users.
Prepares for more complex edition operations on the forwardbot side
* split out the pseudo-headers from the message body
* don't separate the co-authored-by headers from the others, seems
unnecessary, we just need to ensure they're at the end so github
doesn't miss them (/it)
When finding new commits, if there is more pending builds on a repo than
the running_max parameter, the exceeding builds are skipped.
As a result, when nightly builds are created on the runbot, it happens
that some of them are skipped.
Also, since e51412d , only refs newer than max_age are builded; thus the
logic is not needed to prevent rebuild of olds refs in case of a fresh
runbot install.
