Update by rebase strategy

[jukben]

Hey, I'd really love to use this, but our strategy is to update branch by rebase and then merge it to master. However, if I understood it correctly, the update strategy is merge, could it be a rebase instead?

[chdsbd]

I'm not sure I exactly understand your question.

Kodiak currently uses this api call to update the PR branch with the target. Basically the equivalent of the "Update Branch" button. https://developer.github.com/v3/repos/merging/

Kodiak then merges the PR into the target based on the config: "squash", "merge", "rebase". This is equivalent to the "Merge" button and uses this api call: https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button

Which of these would you like to modify?

[chdsbd]

Okay, I think I understand now. Please correct me if I'm misunderstanding.

Instead of using a merge commit to update your branch you'd like to see Kodiak rebase all the commits in the PR against master. (rebase the commits and force push to the PR)

It seems like we'd need to do something like what this npm package does: https://www.npmjs.com/package/github-rebase#step-by-step

This should be possible.

[jukben]

Exactly as you said in the second comment. I'd like to rebase all the commits in the PR against master (and thus "update" the branch). I didn't realise Kodiak uses the "Update Branch" button, make sense though.

I'm definitely interested in it, that cherry-pick trick it's crazy but if it gonna work. Why not, right. I believe updating by rebasing against master is actually quite common, so it might be great enhancement. 🙏

[JohannesRudolph]

So if I get this right this means that kodiak right now doesn’t support a „fast forwards only“ workflow? What I mean by that is that PRs are updated by kodiak to be always strictly ahead of their base branch by rebasing (linear history). Merging a PR must only be a fast forward.

This is critical to our workflow as we rely on git commit ids for avoiding unnecessary rebuilds of our CI pipeline. With a fast forward only workflow we were hoping to have the following automated by kodiak:

• PR is automatically rebased on base branch if it falls behind base branch
• CI builds successfully at PR head
• PR is merged by fast forwarding base branch to PR head
• no further build is required since the PR head has already been build

I gather from this thread however that kodiak does the following

• base branch is merged into PR head if PR falls behind base branch
• CI builds successfully at PR head
• PR is merged by rebasing (or configured strategy) PR onto base branch
• another CI build is required on base branch since the merge created new commits

[JohannesRudolph]

I’m retracting my feature request for this. It looks like we found a way to have our build pipeline do caching based on git trees instead of git commits. These can stay stable even during merges, rebase or squashes (in the way we care about anyway). That means we can use a merge based flow rather than a rebase and fast forward flow.

[robertcoopercode]

I'd still like to see this feature implemented. On a current project, we require PRs to be up-to-date with master and so we use this rebase action in combination with the Kodiak "automerge" label, but the results are not always reliable (e.g. the rebase action or Kodiak doesn't always get triggered to run).

[JohannesRudolph]

Just to be clear. I still think it’s a great idea, just want to mention that my team very likely will no longer depend on rebase to be supported by kodiak before we can incorporate kodiak into our development process... Just want to help maintainers set priorities. Great to hear from others that want to see still see this though @robertcoopercode 👍

[andrewhampton]

I'm in the same boat as Johannes. His first comment describes my situation exactly.

We've been using Kodiak for a few weeks in a small test repository. It's worked great, but CI in that project is so fast, I hadn't noticed the rebase strategy creates a new SHA for the commit in master.

CI and docker builds in our main application take around 20 minutes. So when we started experimenting with Kodiak in that application on Friday, I was disappointed to learn the commit on master is a brand new SHA. This turns that 20 minute process into a 40 minute process for every PR. I see now that's the documented behavior for the API.

As Johannes mentioned, the ideal behavior would be:

• PR is automatically rebased on base branch if it falls behind base branch
• CI builds successfully at PR head
• PR is merged by fast forwarding base branch to PR head
• no further build is required since the PR head has already been build

The two key changes here are:

• Rebasing when the base branch is updated instead of merging changes into the PR
• Doing a fast-forward merge into the base branch when the PR is ready

These two changes will ensure the same SHA that has passed CI will become the head of the base branch. It's not yet clear to me how to update branches using a rebase via GitHub's API. However, I see bulldozer uses a call to UpdateRef to do a fast-forward merge.

@chdsbd are you interested in contributions to implement these options? I see a few months ago you mentioned you might work on this. Is there an active branch in development already?

[chdsbd]

@andrewhampton Thank you for your detailed response.

Also thanks for linking those docs. I didn't realize that GitHub modifies commits on rebase merge. That update ref endpoint you referenced seems like a great way to work around GitHub's rebase merge behavior.

I think you outlined the features we need to add perfectly:

• Rebasing when the base branch is updated instead of merging changes into the PR
• Doing a fast-forward merge into the base branch when the PR is ready

I think the rebase updates need to work like the NPM package linked above: https://www.npmjs.com/package/github-rebase#step-by-step. So we need to do a little more work in this method when we want to do rebase updates.

kodiak/bot/kodiak/queries.py

Lines 993 to 999 in e8d6873

	async def update_branch(self, *, pull_number: int) -> http.Response:
	headers = await get_headers(installation_id=self.installation_id)
	async with self.throttler:
	return await self.session.put(
	f"https://api.github.com/repos/{self.owner}/{self.repo}/pulls/{pull_number}/update-branch",
	headers=headers,
	)

For fast forward merges we need to call the update ref endpoint instead of the pull request merge endpoint.

kodiak/bot/kodiak/queries.py

Lines 1019 to 1038 in 388b689

	async def merge_pull_request(
	self,
	number: int,
	merge_method: str,
	commit_title: Optional[str],
	commit_message: Optional[str],
	) -> http.Response:
	body = dict(merge_method=merge_method)
	# we must not pass the keys for commit_title or commit_message when they
	# are null because GitHub will error saying the title/message cannot be
	# null. When the keys are not passed, GitHub creates a title and
	# message.
	if commit_title is not None:
	body["commit_title"] = commit_title
	if commit_message is not None:
	body["commit_message"] = commit_message
	headers = await get_headers(installation_id=self.installation_id)
	url = f"https://api.github.com/repos/{self.owner}/{self.repo}/pulls/{number}/merge"
	async with self.throttler:
	return await self.session.put(url, headers=headers, json=body)

I'm afraid I haven't done any work on implementing this functionality but I'm all for contributions. If anyone wants help adding these features, let me know.

[andrewhampton]

I may have time to work on this within the next 3 weeks. This isn't critical to the success of the project I'm currently on, but it's definitely the highest priority "nice to have" on our list.

[gitfool]

Instead of using a merge commit to update your branch you'd like to see Kodiak rebase all the commits in the PR against master. (rebase the commits and force push to the PR)

@chdsbd I would also like to rebase the PR before merging it to master - with a merge commit, not a fast forward.

[bryanjswift]

Not sure if this is still on the radar but the folks I'm working with would like the ability to update via rebase and then perform a merge commit as well!

[chdsbd]

I'm hopeful we'll get GitHub API support for rebases soon since GitHub just added this feature to the UI: https://github.blog/changelog/2022-02-03-more-ways-to-keep-your-pull-request-branch-up-to-date/

[AllanOricil]

This is really important to automate updates in long running feature branches where PRs are merged in it. When our feature is ready, we want to merge the whole history to main and without those "merge main into 'feature'" commits.

[r0binary]

@AllanOricil For this exact use case you could manually rebase the branch & force push before merging it back. But I definitely see this as a workaround.