ghstack
ghstack copied to clipboard
Why can't the base of the stack be the actual main branch?
This is more of question. The README page correctly points out that merging commits from the github UI becomes impossible when using ghstack. One thing that would help with this is if the actual base of the branch the real main
and not a brach created by ghstack. I am sure there is a reason for it but why is the base branch called gh/username/1/base
, what would be the downside of basing the PR against main?
WARNING. You will NOT be able to merge these commits using the normal GitHub UI, as their branch bases won't be master. Use ghstack land $PR_URL to land a ghstack'ed pull request.
This used to be because GH didn't let you change the base, but apparently this API has started working so there is very little downside, we should do it
What I'd like to see is for the base to be the previous PRs head.
Especially as when the previous branch has been merged and deleted, GitHub will automatically update the PR's base to the branch it was merged into.
Having the base be the previous PR's head is possible to implement, but it comes at a cost: today, you can reorder PRs in a ghstack and it just works. However, if you try to reorder in this world order, it doesn't work, because e.g., PR 2 (stacked on PR 1) still has all of PR 1's history in it, and so PR 1's history will start showing up if you retarget to master. I think the only way around it is to force push in this case, which might be an acceptable tradeoff.
I'd certainly be happy with that that as a tradeoff personally. In fact that is more or less how I manage my stacks manual currently.
Yeah ok. IDK when I'll get around to implementing this but I would definitely be happy to accept a PR that adds this as an option
@ezyang Question related to this: If we made the new base for the first diff in the Ghstack main, is there anything else that would break if one were to use the Github UI to land the first diff in the stack?
If not, I would love to take a crack at this, so that I could try to support using Ghstack with the Github Merge Queues feature.
Nope, it would work fine (though I would recommend making sure you only do squash). However, the later PR wouldn't get automatically retargeted, so you'd need some integration to do that.
However, the later PR wouldn't get automatically retargeted, so you'd need some integration to do that.
On this, GitHub's behavior recently changed in that if you have a stack of PRs like b
-> a
-> main
, and merge the PR for a
and then delete branch a
, GitHub will automatically retarget b
to main
. On the PR it shows up as a message like this:
Base automatically changed from
a
tomain
(Just reproduced it here: https://github.com/robinst/github-stacked-prs-test/pulls?q=is%3Apr)
GitHub's behavior recently changed in that if you have a stack of PRs like
b
->a
->main
, and merge the PR fora
and then delete brancha
, GitHub will automatically retargetb
tomain
.
Oooh interesting! Is this documented anywhere? I couldn't find it in https://github.blog/
Oooh interesting! Is this documented anywhere? I couldn't find it in https://github.blog/
I couldn't find anything official, but it's easy to reproduce and a comment here also mentions it: https://stackoverflow.com/questions/32658070/what-happens-to-existing-pull-requests-when-the-target-branch-is-deleted#comment126821938_32658536
I spent some more time thinking about how to do this and there's another problem.
It's pretty common to do a squash to merge in commits to main. This means that the merge base for main and the retargeted second PR on the stack will be quite far into the past, and each branch will incorporate the "same" change in a way that Git does not know is the same. So if there were further changes on top of it, you'll likely merge conflict; or worse, Git might get confused and apply the diff twice (when it should have only been applied once.)
Luckily, I bet @robinst's reported behavior likely only occurs if you do an honest to goodness merge (which won't have this problem), so if you do a squash, you need a ghstack fixup step anyway, and we probably can manually setup the virtual merge in that case anyway.
Another thing to note is that this would be a change in the ghstack refspec format, which actually is a BC breaking change since there are some tools relying on its format. So this would be something like a ghstack 2.0 type affair (or more likely, ghstack submit2 or something versioning like that)
I've been dogfooding this feature on master and a few things I've noticed re: interaction with GitHub:
- You can use the regular GitHub UI to merge commits, but there are some caveats. Merges work great. However, if you squash merge, anything stacked on top of the PR will "conflict", because Git is ostensibly trying to re-merge the branch changes from the base PR and failing. If you
git pull --rebase
and thenghstack --direct
that will typically resolve things without causing anymore conflicts. There's probably a way to cook up a GitHub action that will automatically fix this - You probably want to modify GitHub settings to use PR body as the commit message, since people typically generate failure useless ghstack update messages commit-by-commit. One side effect of this is I moved the ghstack navigation links to a separate comment, so they don't get uselessly glommed into your commit message. It's possible we should change this for old-style ghstack, not sure.
- Twitter seems to think that in complicated rebase situations, we should just force push: https://twitter.com/ezyang/status/1735456825133216249 This requires extra implementation that's not done yet