search

laminas / .github

Laminas Organization Templates
https://getlaminas.org/
5 stars 38 forks source link

Prevent renovate from ever merging PRs #44

Open Xerkus opened 1 year ago

Xerkus commented 1 year ago

Bug Report

In service manager repo renovate opened lockfile maintenance PR against default branch that was 3.21.x at the time. Since then default branch changed to 4.0.x and CI issues were fixed in that branch. On the next lockfile maintenance attempt renovate force pushed updates from the new default branch 4.0.x and since CI passed it auto-merged the PR that was still opened against old default branch 3.21.x. See https://github.com/laminas/laminas-servicemanager/issues/197 and the renovate PR in question https://github.com/laminas/laminas-servicemanager/pull/185

This exposed flawed behavior that can have serious consequences while we are unaware.

As such auto-merging of pull requests must be entirely disabled in renovate config. That will not affect the lockfile maintenance flow of automerging branch without creating the PR.

internalsystemerror commented 1 year ago

This could still happen with automerging branches if the default branch is changed down between one run and the next whether a PR is created or not. A PR would usually have many runs between creation and merger, however that wouldn't be guaranteed.

It also doesn't look like renovate has the option to only automerge branches and not once the PR is opened, though this could be a new feature request.

Would a better option be to ensure we never change the default branch to an unreleased major? I know this was supposed to have been implemented in automatic-releases already (by me), but it never seemed to work correctly, and the original suggestion only to compare the minor didn't seem feasible at the time either.

On renovate's side, I wonder if it would be useful to force recreation of a branch if the default branch had changed between runs, in order to ensure this doesn't happen for anyone else too.

Xerkus commented 1 year ago

renovate will force push branch from default and merge into default at consecutive runs, however.

Xerkus commented 1 year ago

Would a better option be to ensure we never change the default branch to an unreleased major?

it does not matter whether it is released. It is unacceptable behavior and there are many many repos with lockfile PRs that target wrong branch now.

internalsystemerror commented 1 year ago

This is only an issue because we switched the default branch down though right? Going from 3.1.x to 3.2.x shouldn't carry over any commits even with rebasing? The same would be true for automerging branches or PRs, because its always over consecutive runs since it has to wait for CI checks to be green on a branch before merging, whether there is a PR or not.

I'm thinking this is what we need:

On renovate's side, I wonder if it would be useful to force recreation of a branch if the default branch had changed between runs, in order to ensure this doesn't happen for anyone else too.

internalsystemerror commented 1 year ago

I'm really curious why/how this happened though as they state that they don't use git rebase, and instead reapplies the changes off the HEAD of the base branch.

https://docs.renovatebot.com/updating-rebasing/

Xerkus commented 1 year ago

PR base branch is always the default branch at the time PR is opened. renovate force pushes updates always from default branch at the time when it does the force push. If renovate then automerges PR then it merges that into original branch PR was opened against but commits are from current default branch.

Xerkus commented 1 year ago

Automerge for PR can not be disabled separately from automerge for branch.

New feature to change base branch is required to resolve this: https://github.com/renovatebot/renovate/issues/19900