renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.87k stars 2.37k forks source link

[Feature Request] Running renovate on multiple branches #1279

Closed darkbasic closed 6 years ago

darkbasic commented 6 years ago

I use a software called tortilla (https://github.com/Urigo/tortilla) to create easy-to-maintain tutorials. After creating the commits, you can refer to them from the Markdown template using simple tags. I used tortilla to create the Angular-Meteor tutorial: https://angular-meteor.com/tutorials/whatsapp2/ionic/setup This is the repository with the code and the markdown template (which resides inside the .tortilla directory): https://github.com/Urigo/Ionic2CLI-Meteor-WhatsApp

Once created you can easily rebase the tutorial, modify the code, add new commits etc. Tortilla will take care of adding prefixes to the commits to be able to easily refer to them (checksums will change after a rebase).

Each tutorial is divided in chapters and in each new chapter you will often overhaul the previous code, deleting dependencies and adding new ones. That means that the final chapter will expose only a fraction of the code, because most of it has been reworked/deleted during later chapters. That's why we created a new, still unreleased, version of Tortilla which exposes each chapter as a separate branch. That way all the code of the tutorial will be exposed to continuous integration testing (Travis). Since debugging issues caused by dependency updates has proved to be one of the most time consuming maintenance tasks, we thought about running renovate on each branch (which contain a different chapter each one). Such a way renovate will simply append new commits at the end of each chapter, while we will theoreticall need to rebase the tutorial to actually update the dependency (because of how tortilla works.) Fortunately we don't really need to release a new version of the tutorial if we know that latest version of each dependency still works: we just need to know when something breaks to be able to manually update it. That's why renovate is such a wonderful piece of software which will perfectly suit our needs, but to be able to use it with tortilla we will need it to to run on multiple branches. We don't want renovate to run on the master branch (appending commits to the master branch will break tortilla and we want people to be able to clone it to work on the turorial). Instead we want it to run on all the master-step* branches, which are created just for the sake of Travis and Renovate.

darkbasic commented 6 years ago

In my previous comment I actually pointed to the old Angular-Meteor tutorial, which uses the old tortilla and doesn't have separate branches for each chapter. Here you can see the new tutorial, which uses the new tortilla and has separate branches for each chapter: https://github.com/Urigo/whatsapp-client-angularcli-material

In the new tortilla we changed several things, for example now we have the client and the server in two separate repos. This is the server: https://github.com/Urigo/whatsapp-server-express

Both of them do not contain templates. We create templates for each client+server combination in different repositories, for example: https://github.com/Urigo/whatsapp-angularcli-material

In the client and in the server we want to run unit tests only. On the contrary in the template repo (the one for the client+server combination) we will include both the client and the server as git submodules. Such a way we will be able to run end to end tests in that repo.

rarkins commented 6 years ago

@darkbasic thanks for the details here. I don't think I understand everything about how tortilla works, but I probably don't need to.

I think the fundamental thing is that you need Renovate to support multiple base branches. Currently Renovate detects the base branch (usually master) or alternatively allows an alternative baseBranch to be defined in the config and uses that instead. The base branch is used for both the "existing content" (e.g. package.json contents) and also as the target for any PRs.

My first thoughts on this is that it's hopefully achievable without too much Renovate architecture change. What I think needs to happen is that we essentially end up running Renovate multiple times per repo, with an alternative base branch each time (doesn't really matter whether master is one of them or not).

So in your repository default branch (probably master) you'd have your renovate.json config that includes a new config option like:

  "baseBranches": ["master-step-1", "master-step-2", "master-step-3"]

(I think for now we make it manually configured rather than regex)

Although you may not need it now, we probably then need to make it possible that the renovate.json in each of those base branches overrides whatever is in master branch, i.e. in case you need different rules for different branches.

Pull Requests can target only one base branch at a time, so that means we need separate PRs for each of your base branches even if the upgrades are identical (e.g. new version of angular libraries). And for that we'd need separate branches. I propose we'd construct them like this:

You probably also want to have PR titles different for each, so you can distinguish them, e.g. "Renovate angular monorepo packages [master-step-1]".

Does this sound OK, and are there any other things to consider that you've thought of?

darkbasic commented 6 years ago

Seems completely fine to me. All the PRs which will pass the CI tests are going to be automatically merged, so I don't think we are going to get too much noise because of the multiple PRs.

rarkins commented 6 years ago

ok. I'll keep planning it and hopefully have time to work on it next week.

rarkins commented 6 years ago

It's still a work in progress (lots more to do) but here is an example onboarding PR using a fork of your repo: https://github.com/renovate-tests/whatsapp-client-angularcli-material/pull/1

I think we'd run out of characters in the PR body if all steps were added though..

darkbasic commented 6 years ago

Thanks. Maybe we could make separate PRs on a per-branch basis?

rarkins commented 6 years ago

@darkbasic there's no choice but to make separate PRs actually, because each PR can target only one base branch at a time. So if angular is updated, you will need one separate PR per base branch. The problem I described above is just about fitting the entire description into the "Configure Renovate" PR!

darkbasic commented 6 years ago

Oh so it's only for the first time configuration summary, right? If so, maybe it could simply attach a text file including all the changes?

rarkins commented 6 years ago

Maybe I'll have an option to just list all the PR names instead of with details, if it's too long.

darkbasic commented 6 years ago

Yes, it shouldn't be a blocker in my opinion anyway.

darkbasic commented 6 years ago

Hi @rarkins, how are you? Whats' the current status of this? Should I be able to start using renovate with tortilla?

rarkins commented 6 years ago

@darkbasic sorry, I was taking a kind of vacation recently and not working on anything too complicated. The functionality seemed mostly complete so I will add tests and get you to test it out. It would be good if you have a "test" repository (e.g. fork of your main one) that you can run it on first just in case anything is not as I expected.

darkbasic commented 6 years ago

Don't worry, I imagined you were on vacation so I avoided to keep pinging until today. A test repository would be fine, thanks :)

rarkins commented 6 years ago

@darkbasic this is now implemented. You can see my testing of your forked repository here: https://github.com/renovate-tests/whatsapp-client-angularcli-material/blob/master/renovate.json

darkbasic commented 6 years ago

Hi @rarkins, I would like to thank you for making this possible. I just updated the repo and tested it, awesome! Is there some kind of threshold for max simultaneous PRs? Because I got PRs for step3, step4, step5 and step 6, but I didn't get PRs for step7, step8 and step 9 until I merged the previous ones. If there is such threshold I think that it should me multiplied for the number of base branches. For example if the threshold is 4, with 7 base branches it should be 28.

rarkins commented 6 years ago

@darkbasic what you're likely seeing is actually a "rate limit", rather than "max simultaneous" limit. It's intention is that you don't get a HUGE flood of PRs after onboarding, and get a more manageable flow. The config:base preset which you might have has a limit of 2 new branches created per "calendar hour", e.g. :00 - :59. If you want to increase it, you can add something like this to your renovate.json:

  "prHourlyLimit": 10
darkbasic commented 6 years ago

Even though I merged all the PRs, since 3 branches are failing I don't get any more PRs for the other 4 branches which passed the CI: https://github.com/Urigo/whatsapp-client-angularcli-material/pulls

Edit: I didn't see your previous post when I wrote it.

rarkins commented 6 years ago

OK, that's another problem altogether! As mentioned in those PRs:

image

Renovate has a (non-configurable) behaviour that waits for any pin PRs to be merged first - assuming you configured pinning - before it attempts to then upgrade any. i.e. in order to establish a kind of baseline set of deps.

When you have multiple branches, I guess we could still allow upgrades to any base branches that have successfully pinned, and skip those that haven't, but that's going to require some extra logic to be added.

If you can't fix these pinning problems (btw such a case means that your base branch was already "broken" to begin with and every upgrade will likely fail tests too), then you may want to temporarily disable pinning by adding "pinVersions": false to your config. After that you should see Renovate autoclose those existing Pin Dependencies PRs and proceed with upgrade PRs.

The good news is that any that are already pinned won't get unpinned by Renovate - Renovate never changes pinned versions to ranges - it just means that Renovate will stop trying to reduce ranges down to exact versions for you for anything that's left unpinned (e.g. those 3 branches that are failing).

rarkins commented 6 years ago

BTW closing the Pin PRs doesn't work. If you've configured pinning, or left Renovate to its default behaviour of autodetecting pinning, then it will create or recreate Pin Dependencies PRs whenever now or in the future it detects that the base branch contains a range. If you want to skip pinning, please add the config I suggest above to skip the pinning step and proceed to upgrade PRs.

darkbasic commented 6 years ago

btw such a case means that your base branch was already "broken" to begin with and every upgrade will likely fail tests too

That's exactly the case, because one of the dependencies broke and so all the chapters starting from 7 onward will not build anymore. It happened because of a sub-dependency which moved to peerDep (I didn't publish package-lock.json to ease rebases). This is something which could happen again in future, so I think that the best thing would be to wait for pin PRs on a per-branch basis.

For the moment I will simply manually merge all the pin PRs despite them failing the tests. As you said, it's not renovate's fault if the tests don't pass.

One more thing: let's say that I want to automatically merge ALL PRs which pass the tests, which option should I set in the config?

Thanks

rarkins commented 6 years ago

One more thing: let's say that I want to automatically merge ALL PRs which pass the tests, which option should I set in the config?

If you want to automerge only patch and minor updates - and therefore manually review/merge any major updates - then add ":automergeMinor" to your extends array. If you want to manually merge any updates that pass, including major, then add ":automergeMajor" instead.

darkbasic commented 6 years ago

https://travis-ci.org/Urigo/whatsapp-client-angularcli-material/builds/331476169?utm_source=github_status&utm_medium=notification

3.91s$ git fetch origin +refs/pull/67/merge:
fatal: Couldn't find remote ref refs/pull/67/merge
The command "eval git fetch origin +refs/pull/67/merge: " failed. Retrying, 2 of 3.
fatal: Couldn't find remote ref refs/pull/67/merge
The command "eval git fetch origin +refs/pull/67/merge: " failed. Retrying, 3 of 3.
fatal: Couldn't find remote ref refs/pull/67/merge
The command "eval git fetch origin +refs/pull/67/merge: " failed 3 times.
The command "git fetch origin +refs/pull/67/merge:" failed and exited with 128 during .
Your build has been stopped.

Any idea why it happens?

rarkins commented 6 years ago

I don't know exact timestamps on Travis, but it looks like you closed the PR and then Travis ran and couldn't find it:

image image

i.e. looks like Travis's failed run started at least a minute or two after you deleted the PR.

darkbasic commented 6 years ago

I didn't close or delete the PR, it did everything by itself. I simply put ":automergeMajor" in renovate.json.

rarkins commented 6 years ago

It's not possible for Renovate to close a PR and make it look like a user did it instead, so either GitHub has a massive bug, or you closed it yourself somehow without realising it. The PR I'm talking about: https://github.com/Urigo/whatsapp-client-angularcli-material/pull/67

darkbasic commented 6 years ago

I didn't close any of the PRs myself :) Do you know why :automergeMajor doesn't work as expected? I still have to manually merge PRs.

rarkins commented 6 years ago

I didn't close any of the PRs myself :)

Did you delete any branches? If a PR's branch is deleted then that implicitly closes the PR too. If you are also sure on that, can you email support@github.com, send them the reference https://github.com/Urigo/whatsapp-client-angularcli-material/pull/67#event-1434087617 and ask them why it's saying your username closed a PR that you didn't? If it's a bug then it seems like an important one for them to know. Please let me know the response too.

Do you know why :automergeMajor doesn't work as expected? I still have to manually merge PRs.

Automerge doesn't work by default if tests fail. If you want to accept the risk, you can add "requiredStatusChecks": null to your config and then Renovate will merge every single branch without caring if there are tests or if tests have failed.

darkbasic commented 6 years ago

Did you delete any branches?

No, but I rebased and then force-pushed. Does it count as deleting a branch?

Automerge doesn't work by default if tests fail.

The tests did not fail. All the merged PRs in chapter 3, 4, 5 and 6 has been manually merged because the automatic merge didn't work. Chapter 7, 8 and 9 failed the tests so it's fine to not automatically merge them.

rarkins commented 6 years ago

No, but I rebased and then force-pushed. Does it count as deleting a branch?

I can't be sure of it, but if that's what you did and then GitHub says you closed it, then it's highly likely that's what caused it. GitHub closes PRs if it thinks the branch its associated with has gone away, and perhaps if the head ref it points to goes away.

Here's one that automerged: image

Here's one you merged: image

You can see GitHub reports both the creation of PR and your merging as being approximately 3 hours ago. Renovate normally needs around an hour or so to automerge, because it's not yet triggered to detect exactly when all tests pass and run again immediately. i.e. it will be caught up on the next hourly run.

Also note: it's usually only possible to automerge once per "run". Once you change the master branch by merging something into it, GitHub triggers a background process to recalculate whether all existing branches are mergeable or not (e.g. have conflicts). In the meantime its API says that those branches are all potentially unstable, so Renovate cannot safely automerge multiple in a row.

darkbasic commented 6 years ago

Oh, I didn't notice there was one which has been automatically merged.

But why do I have PRs for @types/node, jasmine-core, protractor and typescript in master-step5 while I only have PRs for @types/node in master-step3? They are all dependencies of master-step3 as well.

rarkins commented 6 years ago

https://github.com/Urigo/whatsapp-client-angularcli-material/pulls?utf8=%E2%9C%93&q=is%3Apr+sort%3Aupdated-desc+master-step3

darkbasic commented 6 years ago

I'm not sure if I understand it correctly, but for typescript and jasmine-core it basically says that "As this PR has been closed unmerged, Renovate will now ignore this update". The point is that I didn't close any PR at all. I'm starting to think that since the Tortilla workflow involves doing a rebase each time that I push something this is not going to work with Renovate, because force-push is being interpreted as closing the PR by Github. Am I right?

Also, I don't see any PR for protractor at all: https://github.com/Urigo/whatsapp-client-angularcli-material/commits/master-step3 There is this one (https://github.com/Urigo/whatsapp-client-angularcli-material/pull/39) but the branch has been rebased since then and it no longer appeared.

rarkins commented 6 years ago

The point is that I didn't close any PR at all.

It doesn't really matter, if GitHub thinks your actions should close it and then closes it on your behalf with asking you - the result is the same.

I'm starting to think that since the Tortilla workflow involves doing a rebase each time that I push something this is not going to work with Renovate, because force-push is being interpreted as closing the PR by Github. Am I right?

Your workflow seems to be incompatible with GitHub, not with Renovate. By that I mean: if you are performing git actions that result in GitHub closing PRs that you don't want closed, then it's incompatible. I suggest you email support@github.com with logs of the git commands you're using and asking why that results in GitHub closing PRs if you really disagree with their functionality or want to understand it better. I've only seen them closing PRs previously when branches are deleted, so this is new to me.

Also I suggest reviewing if you really need to use those exact git commands. If you tell me why you need to do a rebase on Renovate branches, then maybe I can suggest an alternative. For example did you know that Renovate can rebase its own PRs any time it sees the base branch has been updated? It won't happen instantly but within ~1 hour. As you get your dependencies more updated after this initial burst, you should see less concurrent updates and hence less need for rebasing.

edmorley commented 6 years ago

I've only seen them closing PRs previously when branches are deleted, so this is new to me.

I think I've seen them close PRs when a force push was made that makes the branch now identical to master (ie no commits unique to the PR), which makes sense.

darkbasic commented 6 years ago

Ok, I think I'm starting to understand what's happening here.

Also I suggest reviewing if you really need to use those exact git commands. If you tell me why you need to do a rebase on Renovate branches, then maybe I can suggest an alternative.

Tortilla is a tool to create tutorials. It allows you to create commits which you can then refer to as "diffSteps" in a template. For example this is a commit which we then refer to as "diffStep 1.3": https://github.com/Urigo/Ionic2CLI-Meteor-WhatsApp/commit/a70f9a9130e73e4de9089fda3325878008bc9505#diff-b9cfc7f2cdf78a7f4b91a753d10865a2 This is template where we refer to that commit: https://github.com/Urigo/Ionic2CLI-Meteor-WhatsApp/blob/master/.tortilla/manuals/templates/step1.tmpl This is the end result once you render the view and embed it into a website: https://angular-meteor.com/tutorials/whatsapp2/ionic/setup

The whole point of Tortilla is handling the tutorial updates. If we decide to update Typescript from 2.4.2 to 2.4.3 then the view relative to diffStep 1.3 will be obsolete, because it refers to 2.4.2. In order to compute the correct diffStep view we need to update the parent commit to use Typescript 2.4.3 and then all subsequent commits will be rebased against that change, so it will be easy to generate new diffStep views which will reflect the updates we made.

Doing so is not made using git commands by myself, on the contrary Tortilla forbids direct use of git if you don't want to "tamper" the repository. Instead we use commands like "tortilla step edit 1.3" which will internally use git to do the modifications we want and then rebase all subsequent commits. Since commit hashes will change once you rebase, Tortilla automatically appends a prefix to each commit (like "Step 1.3") to be able to find them and refer to them despite having different hashes.

The point is that since it internally does rebases, you cannot simply push the changes, because Github will reject them (the history will be different). So you have to append "--force" each time you push.

I think I've seen them close PRs when a force push was made that makes the branch now identical to master (ie no commits unique to the PR), which makes sense.

Doesn't make sense in our case. Initially we only had a single branch: master. If you wanted to checkout a specific chapter you simply had to checkout the relative commit. Then we improved Tortilla to make it compatible with Renovate, because we wanted to know when an update breaks the tutorial, thus having the possibility to manually intervene to fix it. When Renovate updates a dependency it simply appends a commit at the end of the history, while we need to amend the specific commit where we actually introduced that dependency and rebase the subsequent history. So Renovate cannot actually update the tutorial itself, but at least it can tell us if there is any breaking change on the horizon, and once we know that something breaks we can manually update the tutorial to fix it. But Renovate cannot append its commits at the end of the master branch, because we need a sane Tortilla repo and appending commits will tamper it. So we needed to create a branch just for Renovate, so that it could tamper it as much as he wanted because we still had master for our purposes. But since we also remove lots of dependencies between one chapter and another, if we run tests only at latest chapter we will miss lots of possible breaking changes in the middle. That's why we had to create a branch per chapter: to allow Renovate to catch all the possible breaking changes, including the ones in the middle of the tutorial.

@rarkins I thought about a possible solution: since we don't need to be able to ignore a specific dependency update, can we tell Renovate to always recreate PRs, even if they have been previously closed? I don't mind the "spam", especially since I want all the PRs which pass the tests to be automatically merged.

P.S. In the future we would like to create a webhook which automatically rebases the tutorial using Tortilla once Renovate creates a PR which passes the tests, but this is another story. For the moment knowing when something breaks is enough, because that allows us to trigger a manual update to fix it.

rarkins commented 6 years ago

I thought about a possible solution: since we don't need to be able to ignore a specific dependency update, can we tell Renovate to always recreate PRs

Add "recreateClosed": true to your config and that should do it. By default it's only used for cases where we need to keep recreating with the same branch name and PR title (e.g. Pin Dependencies, Lock File Maintenance, etc).

darkbasic commented 6 years ago

Hello, I fixed the tests for all chapters and I pinned all dependencies, then I force pushed at about 22.00 GMT+1. 12 hours later there are still 9 unmerged PRs (I have ":automergeMajor"). Is it normal?

rarkins commented 6 years ago

Logs are showing they were all created about 20 minutes ago, did you do anything in the repo just now? As mentioned previously, you need to wait for at least 1 run after the PRs are created to see them automerged.

darkbasic commented 6 years ago

No, I didn't do anything. I understand that they were created just 20 minutes ago, what I ask is if it's normal that after 12 hours it still didn't finish creating PRs, considering that there were no more than 5 dependencies updates on each chapter.

rarkins commented 6 years ago

It's not normal, but nothing about this repo's setup and config is normal either :) If there as an unexplained delay, it's hard to tell if it's caused by a bug in Renovate multi-branch or by the git commands that are also "fooling" GitHub into closing things. Those PRs were created without any manual intervention and I see 5/9 of them were merged and now remainder got recreated.

rarkins commented 6 years ago

@darkbasic I created https://github.com/renovateapp/renovate/issues/1430 as I think the number of autoclosed PRs there seems too high. Looks like the remaining PRs have now been merged though.

darkbasic commented 6 years ago

Thanks

darkbasic commented 6 years ago

https://github.com/Urigo/whatsapp-client-angularcli-material/blob/master/renovate.json

https://github.com/Urigo/whatsapp-client-angularcli-material/pulls?q=is%3Apr+is%3Aclosed

After pushing a new update PRs don't get created anymore. I use "recreateClosed": true and it used to work. Any idea?

rarkins commented 6 years ago

I see 8 open right now. The app defaults to maximum 10 concurrently open too, you can increase that if you want

darkbasic commented 6 years ago

There are 20 open PRs now, did you do anything in particular to make it happen? Because 23 hours had passed without not a single PR opened and now 20 of them have been opened.

rarkins commented 6 years ago

I did nothing to trigger it. But I suspect it’s because of a backend bug that is treating this repo as not onboarded. I have a scheduler that queues up all onboarded repos hourly but non-onboarded ones only every day at 12:00 GMT. I’ll look into that soon because it sounds like what you describe

darkbasic commented 6 years ago

Several days had passed and thousands of PRs have been created, but still very few PRs have been merged, for some branches none at all: https://github.com/Urigo/whatsapp-client-angularcli-material/commits/master-step6 https://github.com/Urigo/whatsapp-client-angularcli-material/pulls?page=1&q=is%3Apr+is%3Aclosed

Any idea why?

rarkins commented 6 years ago

Hi @darkbasic, sorry for the inconvenience. I suspect there's a problem with the multi baseBranches logic although it's hard to debug because the volume of branches/dependencies is huge. Perhaps can you uninstall Renovate temporarily? I will then fork it and try testing out on my fork

rarkins commented 6 years ago

Actually I can push a workaround fix soon which temporarily disables pruning for multiple base branches, which should result in PRs remaining open.

darkbasic commented 6 years ago

Thanks, let me know if you have any news!