Automatically handled failed updates due to rebases

[akinsho]

Describe the feature

Occasionally when updating a plugin it fails because the author has rebased their branch, so you see output that looks like

 ✗ Failed to update ray-x/lsp_signature.nvim
  Errors:
    fatal: Not possible to fast-forward, aborting.
    e4852e0

Ideally plugin makers wouldn't do that but really it's quite a helpful thing to be able to do in git and inevitable that people will. This is even more common if using a branch of a plugin that hasn't been merged.

I've tried setting --rebase=true in the update command which works but this removes the ability to inspect individual commits from other updates. I briefly looked into adding a new key binding which would allow a user to rebase manually by clicking over a plugin, but am wondering if this can't shouldn't just be automatic. The user in all likelihood just wants things to update regardless of the git workflow of the author so this seems like a good place to automate things instead.

I think a potential solution would be to inspect the error returned and if it matches a rebase error then just perform a rebase. This could potentially be configurable if for some reason someone didn't want this. It would also only apply in the cases where there has been a rebase and an update fail due to that.

Thoughts @wbthomason? also if you don't hate the idea where might I look, I've had a read through git.lua so assume it would be at some point in the updater function just not sure exactly where/if you have any ideas re. how

I guess alternatively there might be some way to update the updater so it also handles needing to rebase in the first place. I don't know what logic vim-plug uses but I don't think I've ever had to worry specifically about if there's been a rebase or not whilst using it.

[wbthomason]

I like this idea! vim-plug definitely used to fail in this way the last time I used it, but that was years ago at this point; maybe it's worth looking at how they pull updates to see if they've added something to work around this issue.

My git fu isn't strong enough to know a simple way around this, but yes, it'd be in the updater function. Ideally there's some flag we can pass to the pull command to just make this happen; otherwise, we'd be adding an or_else call, checking if the exit code reflects specifically a failure to merge unrelated histories due to rebasing (assuming there is such a unique exit code), and forcing a merge if so.

The code in git.lua is messy as hell due to the poor semantics of the jobs wrapper. I'm happy to help implement this, though I do not yet know how to make git do what we want here.

[leiserfg]

Another way could be adding an action to remove pull --rebase one plugin from the changes list or to delete it to make possible the reinstallation.

[wbthomason]

@akinsho This might be what we want? https://github.com/git/git/blob/master/Documentation/RelNotes/2.9.0.txt#L58-L68

It's probably still a good idea to check what plug does, though.

[akinsho]

@wbthomason that looks like it could be right 👍🏾 definitely worth trying. I had a read through plug's update mechanism and I find it really hard to follow tbh as far as I can tell it looks like it's doing a git fetch and then git merge --ff-only origin/<branch-name> 🤷🏾. Which is what packer is doing as well.

https://github.com/junegunn/vim-plug/blob/fc2813ef4484c7a5c080021ceaa6d1f70390d920/plug.vim#L1864

https://github.com/junegunn/vim-plug/blob/fc2813ef4484c7a5c080021ceaa6d1f70390d920/plug.vim#L1547-L1555

[akinsho]

Actually reading through the docs it occurs to me that because --allow-unrelated-histories used to be the default with git it just might be the case that this was why it "worked" in vim-plug when I used it and now it doesn't rather than it being a packer issue it's more a git issue since the default changed between versions 🤔

[akinsho]

As a bit of a stream of consciousness side note after reading through git docs again for the first time in a long while it really does seem that ultimately this really is a problem of people rebasing on public branches. Not that there's much we can do about that but really does seem like a bit of a misuse but this will work around that

[n3wborn]

Describe the feature

Occasionally when updating a plugin it fails because the author has rebased their branch, so you see output that looks like

 ✗ Failed to update ray-x/lsp_signature.nvim
  Errors:
    fatal: Not possible to fast-forward, aborting.
    e4852e0

Ideally plugin makers wouldn't do that but really it's quite a helpful thing to be able to do in git and inevitable that people will. This is even more common if using a branch of a plugin that hasn't been merged.

I've tried setting --rebase=true in the update command which works but this removes the ability to inspect individual commits from other updates. I briefly looked into adding a new key binding which would allow a user to rebase manually by clicking over a plugin, but am wondering if this can't shouldn't just be automatic. The user in all likelihood just wants things to update regardless of the git workflow of the author so this seems like a good place to automate things instead.

I think a potential solution would be to inspect the error returned and if it matches a rebase error then just perform a rebase. This could potentially be configurable if for some reason someone didn't want this. It would also only apply in the cases where there has been a rebase and an update fail due to that.

Thoughts @wbthomason? also if you don't hate the idea where might I look, I've had a read through git.lua so assume it would be at some point in the updater function just not sure exactly where/if you have any ideas re. how

I guess alternatively there might be some way to update the updater so it also handles needing to rebase in the first place. I don't know what logic vim-plug uses but I don't think I've ever had to worry specifically about if there's been a rebase or not whilst using it.

I also have kind of similar problem sometimes while trying to update lsp_signature. But the problem comes from a prompt asking for Github credentials which completely freeze nvim :/ capture

[akinsho]

@n3wborn I think this is probably worth raising as a separate issue, since fixing this issue won't really fix the one you are describing. Also the link you posted doesn't show the exact error regarding credentials that you mentioned so if you raise this separately it'd be good to get that image in there as well just so it's clear what the exact error is.

[wbthomason]

Yes @n3wborn please file a separate issue for that. We've run into issues with this before (which I thought had been fixed, at least for recent versions of git) - maybe look at https://github.com/wbthomason/packer.nvim/pull/91 and verify that your version of git works as expected with the GIT_TERMINAL_PROMPT environment variable?

[ruifm]

As a (happy) user, I would prefer to have some choice on this.

I don't want to git pull --rebase when it's not possible to fast-forward.

When it's not possible to fast-forward, it means the plugin author messed-up and force-pushed on a public branch.

For me, the correct action is to just move HEAD to origin after a fetch:

git fetch && git reset --hard origin

Many things git related are configurable in packer and imo, so should be this.

[akinsho]

@ruifm my PR is still in fairly early stages but should be easy enough to just expose a retry subcommand like all the others that a user can set themselves, OOI is there a specific concern regarding rebasing vs resetting that you're thinking of? I'm asking primarily since maybe that would be a better default

[n3wborn]

Yes @n3wborn please file a separate issue for that. We've run into issues with this before (which I thought had been fixed, at least for recent versions of git) - maybe look at #91 and verify that your version of git works as expected with the GIT_TERMINAL_PROMPT environment variable?

Sorry, I didn't know about GIT_TERMINAL_PROMPT env var. Setting this fixes my "bug". Tanks :)

[ruifm]

OOI is there a specific concern regarding rebasing vs resetting that you're thinking of? I'm asking primarily since maybe that would be a better default

Rebasing will keep the commits that are local-only but not on remote on top of origin/HEAD. This can lead to unwanted "garbage" commits that will keep piling up.

Example: if the plugin maintainer rebased the last 2 commits from B,C to B', C':

local:
A ------ B ------ C (HEAD)
remote:
A ------ B' ------ C' (origin/HEAD)

after a rebase:

A ------ B' ------ C'------ B ------ C (HEAD, origin/HEAD)

Imo, the goal of a plugin manager should primary be to keep a faithful track of the remote state, which this is failing to do since commits B and C are no longer in remote.

Also, the most common rebase scenario is a fixup, which means that commit B' will be very similar to B which will most likely lead to conflicts in an automatic rebase.

Personally, both rebasing and merging should be manual actions since they can and will have conflicts.

My expected behavior is that commits B and C are discarded since they no longer exist remotely. In order to achieve that, the best way I know is a git reset --hard origin which simply moves HEAD to origin/HEAD.

[akinsho]

@ruifm 👍🏾 makes sense to use that as default then. My initial implementation was just taking advantage of the fact that I could change the --rebase=false to true in the update command which would just "work".

But that's a fair point plus it's more inline with the rest of how packer operates to allow it to be configurable. I guess if a user wanted to make changes locally (not sure why) they could change this to use rebase instead to maintain local changes which was something @.wbthomason raised at some point

[leiserfg]

Yes, the way to go should be fetch and then reset --hard @{upstream}

[ruifm]

changing the options table in packer.init() to

{
  git = {
    subcommands = { -- Format strings for git subcommands
      update         = '-C %s reset --hard @{u}'
      }
   }
 }

seems to do the trick. Maybe the fix should just be to change the default?

Off-topic: I have no idea what the option uptate_branch is supposed to do, I could not find any reference to it in the code.

[akinsho]

@ruifm when you change the update command as you did are still able to see a list of commits when a plugin updates?

I'm guessing that since you are resetting for every update then seeing what changes occurred for a plugin won't work. This is why I think this should only happen when it needs to otherwise users lose the ability to see what changed when they update plugins

[ruifm]

@akinsho you are right, my setting is not working for some reason.