Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent renovate from ever merging PRs #44

Open
Xerkus opened this issue Jun 5, 2023 · 7 comments · Fixed by renovatebot/renovate#22663
Open

Prevent renovate from ever merging PRs #44

Xerkus opened this issue Jun 5, 2023 · 7 comments · Fixed by renovatebot/renovate#22663
Assignees
Labels
Bug Something isn't working

Comments

@Xerkus
Copy link
Member

Xerkus commented Jun 5, 2023

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 laminas/laminas-servicemanager#197 and the renovate PR in question laminas/laminas-servicemanager#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.

@Xerkus Xerkus added the Bug Something isn't working label Jun 5, 2023
@internalsystemerror
Copy link
Member

internalsystemerror commented Jun 6, 2023

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
Copy link
Member Author

Xerkus commented Jun 6, 2023

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

@Xerkus
Copy link
Member Author

Xerkus commented Jun 6, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

Xerkus commented Jun 6, 2023

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
Copy link
Member Author

Xerkus commented Jun 7, 2023

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

New feature to change base branch is required to resolve this: renovatebot/renovate#19900

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants