INFO: "rebase origin/master & push -f" workflow corrupts Pull request rarely

Committers
1

After “git rebase origin/master” and “git push -f”, many commits in the master branch are shown in the pull request some time.

This issue was happened in GH-16430. But I had seen similar issues several times.

I asked the Github support about it, and I got the answer from them:

                Stacey Burns (GitHub Developer Support)                                                            

 Oct 30, 1:18 PM UTC

Hi Indana,
We use what’s called a “three-dot diff” on a Pull Request, which is the
difference between the latest commit on the Pull Request branch and the last common ancestor commit
with the base branch. The last common ancestor is also derived at the
time the Pull Request is created, and doesn’t change throughout the Pull
Request’s lifecycle.
At the time the Pull Request was created, the base branches head commit was . The author then modified the Pull Request branch head commit to , which results in the following diff:
Comparing 9f77268f901cc3e8874e5042361520a0e482476a...e3afbbe87a49acd45170abeb4425eabe40d98010 · python/cpython · GitHub
Rebasing
an existing Pull Request can result in undesirable / unintended results
in some cases, especially when the Pull Request branch is far behind
the base. We normally encourage folks to make sure their Pull Request
branch is up to date with the base branch before creating the Pull Request. This ensures the last common ancestor is the most up to date.
I hope this is somewhat helpful! If there’s anything else I can do to help, please let me know.
Cheers

Stacey

3 Likes
2

I’ve seen this happen as well. Thanks for following up with GitHub and posting a deep analysis of the cause!

This is one of the reasons I’ve adopted a policy to never rebase a branch after creating a PR for it: I always merge rather than rebase in these cases.

This also avoids making old PR comments irrelevant and unclear, which happens when rebasing edits the commit history that they were referring to.

Should we update or create a relevant guideline in the devguide regarding this?

3

I had updated “git bootcamp” chapter in the devguide already. (pull request)

But I found another part which recommends rebase over merge.
https://devguide.python.org/pullrequest/#step-by-step-guide

1 Like
4

+1!

I find the part under “If someone else added new changesets and you get an error:” to be out of context: Merge/rebase should only ever be necessary after a PR has been created, but that part appears before the creation of a PR.

I’ll make a PR for improving that part.

1 Like
5

Thanks for figuring out the root cause of these oddities!

6

If you do get into this state, there’s a workaround: you can temporarily change the PR’s target branch, and then switch it back. This forces GitHub to recalculate the “merge base” and show the correct set of commits.

GitHub could do this recalculation automatically, but apparently they consider the broken PR display a feature that they intentionally implemented and are proud of. (Like, I’m not making a snarky comment, that’s actually what they told me.) I don’t get it. But at least there’s a workaround.

1 Like
7

Thanks for sharing the workaround.

But I don’t think this workaround is suitable for CPython. If we change the base branch temporary, there will be a huge number of commits in the pull request. We may send spam notification to many people.

8

I’ve created a PR, which hopefully will help people deal with merges. Reviews would be very welcome!

9

Update: The devguide’s step-by-step guide to making a PR now includes instructions for merging (rather than rebasing) and an new sub-section on how to handle merge conflicts.

1 Like