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

Can I re-enable force pushing, or should I change my workflow? #3437

Open
jakubledl opened this issue Mar 26, 2024 · 15 comments · May be fixed by #3528
Open

Can I re-enable force pushing, or should I change my workflow? #3437

jakubledl opened this issue Mar 26, 2024 · 15 comments · May be fixed by #3528

Comments

@jakubledl
Copy link

jakubledl commented Mar 26, 2024

When developing on a branch feature-asdf, I'm used to having the upstream of the branch set to origin/main and have push.default set to current. This means that whenever someone else merges something to origin/main, I immediately see it as ↑m↓n, I can pull and rebase onto origin/main with a simple p and I could push the rebased branch with just P and Enter to confirm the force push.

After upgrading to 0.41.0, #3387 of course broke this workflow, since now it's impossible to force push the branch after the rebase. Is there a way I can get the old behavior back, or should I perhaps do something else entirely? I switched to lazygit from Magit when I stopped using Emacs a few months ago, so it's possible I'm just trying to recreate something that I'm used to and that lazygit has a different/better way to do.

@stefanhaller
Copy link
Collaborator

Sorry for breaking your workflow. This is very interesting, I hadn't expected people to work this way. You seem to find it more important to see the divergence from origin/main than to see the divergence from your own upstream branch, which I guess makes sense if you are the only one working on your branch (and you know you only push to it from one machine).

I'm a little unsure what to do here. While I see how your workflow is convenient, it still feels very weird to me, and I'm not sure how hard we should try to support it. Happy to discuss this more though.

That said, I have always missed an easy way to see the divergence of my branch from origin/main; both a simple ↑2↓3 overview, and the more detailed version similar to what you see against your upstream using u enter. I'd be interested in discussing ways to make this information easily accessible somehow, I don't have great ideas myself. Would that be enough to get you to change your workflow to the "normal" one? I do realize that pressing p to rebase onto main would still be missing.

@jakubledl
Copy link
Author

jakubledl commented Mar 28, 2024

Hi @stefanhaller,

many thanks for reply and sorry for my delayed response 🙂 It’s exactly as you describe – I work mostly in a team where everyone is pushing their work into their branches on origin and those then get merged into origin/main. Merges happen frequently, so it’s valuable to see how far behind origin/main your branch is (e.g. to remind you to update your branch now rather than face a ton of merge conflicts later). On the other pretty much the only time two people are going to push into the same branch is during pair-programming sessions, so I’m virtually always sure that I can just wipe anything on the remote branch and replace it with whatever I have on the local branch (and I pretty much never need to pull from the remote branch).

I absolutely understand that it’s a nonstandard workflow and I plan to continue use lazygit even without support for it, it’s a great tool whose benefits for me definitely outweigh the current slight inconvenience 🙂

That said, regarding your last paragraph, as a former user of Magit, I find it’s way of dealing with this really quite convenient. When you check out <name>, it basically let’s you work with both with branch.<name>.pushRemote and branch.<name>.merge. It displays divergence information for both (I think). When you push, it lets you pick whether to push to the push remote (the more convenient key), the merge branch or elsewhere. The same thing works when pulling, with the more convenient key assigned to pulling from the merge branch, I think. When the branch’s push remote or merge branch is unset, it let’s you “set and pull/push” instead. I’ll attach some screenshots later when I’m at my computer.

Perhaps something along those lines might be adapted to work with lazygit and I’d be happy to discuss this further. If there eventually is an idea that you guys might want to include, I’ll be happy to take a shot at implementing it 🙂

@linsui
Copy link

linsui commented Apr 14, 2024

I work on a repo that fast-forward only merge is enabled. So I need to rebase and force-push the branch before merging. Force push is used very frequently in my workflow. Can I override the P keybinding to mimic the old behavior?

@stefanhaller
Copy link
Collaborator

@linsui Sorry for the late response. Is your setup similar to Jakub's, where you pull from a different branch than you push to? If not, you'll have to explain more what the problem is, because I don't see how being restricted to fast-forward-only merges is related.

@jakubledl I'm curious what your current work-around is. Did you change your workflow, or are you using a custom command to force-push, or something else? (I hope you didn't stop using lazygit because of this. 😄)

I made a quick proof-of-concept draft PR (#3528) with a change that should support your workflow correctly, so pressing shift-P should prompt you to force-push in the appropriate cases. Would be cool if you could try it out.

@linsui
Copy link

linsui commented Apr 26, 2024

You are correct. I didn't notice that --force-with-lease is passed by default and in fact the push fails because this problem. https://stackoverflow.com/questions/56191415/why-is-git-push-force-with-lease-failing-with-rejected-stale-info-even

@stefanhaller
Copy link
Collaborator

Did you have the problem with a stale remote tracking branch that was mentioned in the stackoverflow answers, or was it some other reason why it didn't work? Usually, --force-with-lease should work fine in lazygit, triangular workflow or not.

@linsui
Copy link

linsui commented Apr 26, 2024

I thought my push sometimes fails due to the stale remote tracking branch issue. I usually working with the same local branch and the remote branch is deleted automatically when my MR is merged. So lazygit can't fetch the remote branch. I guess so...

@stefanhaller
Copy link
Collaborator

I see. In that case I recommend to do git config --global fetch.prune true, so that remote branches are deleted automatically when you fetch (you'll see "upstream gone" in lazygit), so next time you push it will ask you to create the upstream branch again.

All of that seems unrelated to this issue though.

@linsui
Copy link

linsui commented Apr 26, 2024

Good idea, thanks for your help! Yep, I thought this is an unrelated issue.

@jakubledl
Copy link
Author

@stefanhaller So currently, I don't really have a workaround 🙂 I have the local feature branch's upstream set to the remote feature branch and I use the local main to see whether there are updates; if I see main ↓3, I checkout main, pull, checkout the feature branch and rebase it onto local main. Since I usually work on just a single feature branch, this works, although it is a bit less convenient.

Thanks for the draft PR, I'll be happy to give it a try in the next few days! I'll also look into custom commands, since that's a topic I haven't really explored yet.

@mark2185
Copy link
Collaborator

@jakubledl you don't have to checkout a branch to pull it, you can just fast forward it with f.

@jakubledl
Copy link
Author

@mark2185 Thanks for the suggestion, that's definitely quicker! On the other hand, if I checkout main and pull, I automatically get the one-line log overview of what's new in the Commits window (I find that useful in projects where merges happen often and a single workday may bring something like 15 new merges into main).

@stefanhaller
Copy link
Collaborator

I like to do u enter on main in the branches panel for that. Still no need to check out the branch. 😄 Note that #3537 improves this considerably by enabling the commit graph in that view, which makes it easier to discern the individual merges.

Jakub, I'm also interested in your thoughts on #3536, I personally think this will help a lot, and I intend to start working on it very soon.

@jakubledl
Copy link
Author

Hi @stefanhaller,

I gave the support-triangular-workflow branch a try and as far as I can tell, it works great! One thing I'm unsure about, I also tried emulating a fork-based workflow (feature has upstream set to upstream/main, the repo has remote.pushDefault set to origin, so pushing on feature should push to origin/feature). In this case, I think it once happened that pressing p rebased feature correctly onto upstream/main, but then pressing P didn't request force push, but just errored out. But once I quit lazygit and started it again, it worked (e.g. asked to confirm the force push). I can investigate this further, if you want.

Will look into #3536, thanks!

@stefanhaller
Copy link
Collaborator

but then pressing P didn't request force push, but just errored out.

I think this can always happen, no matter which workflow, when your clone's remote branches are not up to date. Lazygit makes the decision whether to ask for force-push only based on the divergence of feature vs. the local copy of the remote tracking branch; so if that one is out of date because something has been pushed to the other side since you last fetched, you won't get the force-push prompt and it will error out instead.

Fetching helps in this case. The fact that it worked after you quit and restarted lazygit supports this theory, since fetching is the first thing lazygit does after starting.

Actually, this scenario is the very reason why I did #3387, since without fetching you didn't have a chance to check what you are overwriting by force-pushing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants