PlugUpdate fails when there is a forced update to plugin

[phanimahesh]

Title says it all. I discovered that when one of my plugins had a force update, nothing short of messing directly with the plugin directory allows me to update the plugin.

Allow PlugUpdate! to do a non-fast-forward update by simply checking out the new HEAD.

I tested only on neovim, but should be the same behaviour for all.

---

:version
NVIM v0.1.3-348-ge7a9c00
Build type: Dev
Compilation: /usr/local/Library/ENV/4.3/clang -Wconversion -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -O2 -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99
-Wvla -fstack-protector-strong -fdiagnostics-color=auto -DINCLUDE_GENERATED_DECLARATIONS -DHAVE_CONFIG_H -I/tmp/neovim20160315-25348-1k9rfzd/build/config -I/tmp/neovim20160315-25348
-1k9rfzd/src -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr
/include/luajit-2.0 -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/deps-bu
ild/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/deps-build/usr/include -I/usr/local/opt/gettext/include -I/usr/include -I/usr/include -I/tmp/neovim20160315-25348-1k9rfzd/build/s
rc/nvim/auto -I/tmp/neovim20160315-25348-1k9rfzd/build/include
Compiled by phanimahesh@dev.phanimahesh.me

Optional features included (+) or not (-): +acl   +iconv    +jemalloc
For differences from Vim, see :help vim-differences

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/local/Cellar/neovim/HEAD/share/nvim"

• Type:
  • Bug
  • Enhancement
  • Feature Request
  • Question
• OS:
  • All/Other
  • Linux
  • OS X
  • Windows
• Vim:
  • Terminal Vim
  • GVim
  • Neovim

[starcraftman]

@phanimahesh Hi, I have seen this once or twice before. Most responsible git devs shouldn't resort to forced updates as they cause such user problems. I'm not sure this happens frequently enough to warrant us putting code in to handle it specifically.

Note, we already have ! enabled for PlugInstall/PlugUpdate, it means to force the re running of postinstall hooks. I'm not sure if we want to further add to the bang behaviour.

@junegunn Thoughts? Have you had many forced updates? Interested in this being patched?

[junegunn]

@starcraftman Once. Literally once in all these years :) I'm concerned that it might lead to unwanted loss of local changes, little experiments, tweaks, ..., so I'm not leaning towards the idea. But it is true that there is no easy way to resolve the case, one would have to manually remove the plugin. Would it make sense to detect such cases in :PlugClean? The message says "Searching for unused plugins ...", but it also checks for clones with invalid git remotes currently.

[starcraftman]

@junegunn

But it is true that there is no easy way to resolve the case, one would have to manually remove the plugin.

Or just reset the HEAD to before the conflict to make a clean pull.

Regarding PlugClean, could work. Perhaps we should also take time to update PlugClean header message, and point out reason each repo being listed, i.e. unused, remote change or conflict.

[justinmk]

I'm concerned that it might lead to unwanted loss of local changes, little experiments, tweaks, ..., so I'm not leaning towards the idea.

I've run into it (many times) for cases where I'm lazy and patch a plugin which is then rebased upstream.

A low-risk, low-effort way to avoid data loss would be to simply save the current HEAD as a branch named something like vim-plug-bk-20160101. If someone complains about this because it's "ugly", I suggest ignoring such a complaint...

[phanimahesh]

Given that tags and branches are extremely lightweight in git (They are just a file named the same as branch and having the sha of commit they point to), I like this idea. Instead of polluting the global namespace (though not many directly use their plugin dirs for development), we can use vim-plug-backups/<date-time> as the branch/tag name.

[phanimahesh]

@starcraftman Actually, a package/plugin/dependency manager doing a git pull is unusual IMO. A checkout of newly referenced ref is simpler and more common practice.

[junegunn]

I'm also concerned about uncommitted local changes. Those can't be recovered from reflogs. The idea of backup branches sounds nice but it feels a bit overkill for vim-plug. I tend to think twice before adding anything that requires extra explanation.

Anyway, this is a preliminary patch that makes :PlugStatus and :PlugClean handle such cases.

diff --git a/plug.vim b/plug.vim
index a55af87..94c395a 100644
--- a/plug.vim
+++ b/plug.vim
@@ -1860,6 +1860,13 @@ function! s:git_validate(spec, check_branch)
         let err = printf('Invalid branch: %s (expected: %s). Try PlugUpdate.',
               \ branch, a:spec.branch)
       endif
+      if empty(err)
+        let commits = len(s:lines(s:system(printf('git rev-list origin/%s..HEAD', a:spec.branch), a:spec.dir)))
+        if commits
+          let err = join([printf('Diverged from origin/%s by %d commit(s).', a:spec.branch, commits),
+                        \ 'PlugClean required.'], "\n")
+        endif
+      endif
     endif
   else
     let err = 'Not found'
@@ -1875,14 +1882,14 @@ endfunction

 function! s:clean(force)
   call s:prepare()
-  call append(0, 'Searching for unused plugins in '.g:plug_home)
+  call append(0, 'Searching for invalid plugins in '.g:plug_home)
   call append(1, '')

   " List of valid directories
   let dirs = []
   let [cnt, total] = [0, len(g:plugs)]
   for [name, spec] in items(g:plugs)
-    if !s:is_managed(name) || empty(s:git_validate(spec, 0))
+    if !s:is_managed(name) || empty(s:git_validate(spec, 1))
       call add(dirs, spec.dir)
     endif
     let cnt += 1

[starcraftman]

@junegunn Looks good to me, follows our usual approach. People who frequently run into this could simply make an alias or binding for PlugClean! | PlugInstall to quickly put things right.

[phanimahesh]

The backup refs are an overkill, yes. Usually package managers checkout the ref, store the commit hash in a lock file, and on update, they checkout new ref and update the lock. If there are uncommitted changes a common and sensible default is to abort with a message.

But @junegunn's change looks good. It provides a solution to my original issue and is relatively simple. 👍

[junegunn]

Alright, I'll craft a test case for it. Also the output of :PlugClean can be improved.

[phanimahesh]

Thanks @junegunn!

For my future reference: c6ed41f only adds a message to PlugStatus and removes the plugin directory completely on PlugClean.

PlugInstall again will reinstall the plugin. Complete removal and re-cloning is an overkill, but works.

[shmerl]

As an alternaive, may be there can be some option or additional parameter for PlugUpdate to do rebasing.

I.e. instead of doing simple git pull, it can do git pull --rebase. That will avoid conflicts when repository had history rewritten. Or you can simply always do that for PlugUpdate to begin with.