Neovim installer should be synchronous during vim_starting
#104
Comments
junegunn
changed the title
vim_startingvim_starting
|
I'm trying to make a Docker image for my dev environment, but I hit the wall at this point.
I tried using Thanks again for the great tool! |
This shouldn't happen. I also have my Dockerfile which does |
|
@junegunn thanks for the quick reply. I'm running Probably a good idea to have both versions anyway until NeoVim becomes more stable. Thanks for your help! |
|
It will be possible to close this after neovim/neovim#2247 is merged. |
|
I've been thinking of how jobwait() can be used to implement this. Since vim-plug has a maximum number of threads, we cant simply start and wait all curl jobs in a single pass. A solution is to use a "waiter job" that only exits after all downloads are finished. Something like this: let plugins = GetPlugins() " collect all plugins and commands that must be executed
let jobs = {}
function! s:StartPending()
if empty(plugins)
return 0
endif
let plugin = remove(plugins, 0)
let jobs[jobstart(plugin.install_argv, {'on_exit': 's:OnExit'}))] = 1
return 1
endfunction
function! s:OnExit(id, status)
call s:StartPending()
call remove(jobs, id)
if empty(jobs)
" last job, quit the waiter job
call jobclose(g:waiter, 'stdin')
endif
endfunction
" Start the initial batch of jobs
for i in range(max_threads)
if !s:StartPending()
break
endif
endwhile
if !empty(jobs)
" Start a waiter job so the installation will be synchronous
let waiter = jobstart(['cat', '-'])
call jobwait([waiter])
endif |
|
@tarruda I tried the following code just to see how it works: diff --git a/plug.vim b/plug.vim
index a03c1da..00dcf4e 100644
--- a/plug.vim
+++ b/plug.vim
@@ -934,6 +934,10 @@ function! s:update_vim()
call s:bar()
call s:tick()
+ if s:nvim
+ let waiter = jobstart(['cat', '-'])
+ call jobwait([waiter])
+ endif
endfunction
function! s:tick()but I'm seeing a bunch of errors: or Looks like job_handler function is having trouble updating the buffer and the probability of error seems to increase as the number of concurrent tasks increases. (not sure) e.g. |
|
Hmm, I forgot to mention that diff --git a/plug.vim b/plug.vim
index a03c1da..0565523 100644
--- a/plug.vim
+++ b/plug.vim
@@ -929,11 +929,19 @@ function! s:log(bullet, name, lines)
endif
endfunction
+function! s:stub()
+ " Need to pass one callback to `cat` or its stdout will be closed
+endfunction
+
function! s:update_vim()
let s:jobs = {}
call s:bar()
call s:tick()
+ if s:nvim
+ let waiter = jobstart(['cat', '-'], {'on_stdout': 's:stub'})
+ call jobwait([waiter])
+ endif
endfunction
function! s:tick() |
|
@tarruda I just applied your patch without any modification. On the first run, neovim crashed with segmentation fault after some errors. Since then I can't reproduce segv, but I still see errors. |
|
Ok I will investigate |
|
@junegunn diff --git a/plug.vim b/plug.vim
index 888ab36..2ed3ad5 100644
--- a/plug.vim
+++ b/plug.vim
@@ -125,10 +125,6 @@ function! s:to_a(v)
return type(a:v) == s:TYPE.list ? a:v : [a:v]
endfunction
-function! s:to_s(v)
- return type(a:v) == s:TYPE.string ? a:v : join(a:v, "\n") . "\n"
-endfunction
-
function! s:source(from, ...)
for pattern in a:000
for vim in s:lines(globpath(a:from, pattern))
@@ -835,12 +831,18 @@ function! s:job_handler(job_id, data, event) abort
endif
if a:event == 'stdout'
- let self.result .= substitute(s:to_s(a:data), '[\r\n]', '', 'g') . "\n"
- " To reduce the number of buffer updates
- let self.tick = get(self, 'tick', -1) + 1
- if self.tick % len(s:jobs) == 0
- call s:log(self.new ? '+' : '*', self.name, self.result)
+ if self.error
+ return
+ endif
+ if a:data[0] != "" && !empty(self.result)
+ " continue previous line
+ let self.result .= a:data[0]
+ call remove(a:data, 0)
endif
+ if !empty(a:data)
+ let self.result .= "\n" . join(a:data, "\n")
+ endif
+ call s:log(self.new ? '+' : '*', self.name, self.result)
elseif a:event == 'exit'
let self.running = 0
if a:data != 0
@@ -849,6 +851,7 @@ function! s:job_handler(job_id, data, event) abort
call s:reap(self.name)
call s:tick()
endif
+ redraw
endfunction
function! s:spawn(name, cmd, opts)
@@ -892,6 +895,10 @@ function! s:reap(name)
call s:bar()
call remove(s:jobs, a:name)
+ if empty(s:jobs) && exists('s:waiter')
+ call jobclose(s:waiter, 'stdin')
+ unlet s:waiter
+ endif
endfunction
function! s:bar()
@@ -929,11 +936,27 @@ function! s:log(bullet, name, lines)
endif
endfunction
+function! s:stub()
+ " Need to pass one callback to `cat` or its stdout will be closed
+endfunction
+
function! s:update_vim()
let s:jobs = {}
call s:bar()
call s:tick()
+ if s:nvim
+ let s:waiter = jobstart(['cat', '-'], {'on_stdout': 's:stub'})
+ if jobwait([s:waiter])[0] == -2
+ " interrupted
+ for [name, j] in items(s:jobs)
+ silent! call jobstop(j.jobid)
+ let j.error = 1
+ let j.result = 'Interrupted!'
+ endfor
+ unlet s:waiter
+ endif
+ endif
endfunction
function! s:tick() |
|
@tarruda Will do. By the way, we were planning to apply the waiting only when |
True, I removed that when testing and forgot to add it back |
|
The |
|
@tarruda Yes, it's working! |
|
@tarruda I haven't checked if it's a bug in your patch or in jobwait, but I see some errors when I |
|
@junegunn be sure that vim-plug has the cursor in the right buffer. I was getting something similar when tinkering with |
|
@junegunn Figured I'd give this a go too. I couldn't reproduce your particular bug. Instead I got my own set. Used latest from PlugUpgrade with @tarruda 's patch.
|
|
@starcraftman Yes, I also tried the much simpler patch @tarruda first posted again to make sure that the cause of the errors is not in the vimscript from plug.vim, and I'm still running into the similar errors. It's clear that there are some issues with |
|
We could just wait for jobwait() to be fixed but for the moment I have an idea. We could fall back to python installer which is blocking if @starcraftman I tried the following patch, but the current python installer on master branch doesn't seem to work on neovim. IIRC, you were working on making it work on Neovim. Is it ready? We could try your fix if it doesn't require you extra work. Thanks. diff --git a/plug.vim b/plug.vim
index d0e2dad..7079dd5 100644
--- a/plug.vim
+++ b/plug.vim
@@ -72,8 +72,6 @@ let s:plug_tab = get(s:, 'plug_tab', -1)
let s:plug_buf = get(s:, 'plug_buf', -1)
let s:mac_gui = has('gui_macvim') && has('gui_running')
let s:is_win = has('win32') || has('win64')
-let s:py2 = has('python') && !has('nvim') && !s:is_win && !has('win32unix')
-let s:ruby = has('ruby') && !has('nvim') && (v:version >= 703 || v:version == 702 && has('patch374'))
let s:nvim = has('nvim') && exists('*jobwait') && !s:is_win
let s:me = resolve(expand('<sfile>:p'))
let s:base_spec = { 'branch': 'master', 'frozen': 0 }
@@ -746,6 +744,9 @@ function! s:update_impl(pull, force, args) abort
echohl None
endif
+ let py2 = has('python') && (!s:nvim || has('vim_starting')) && !s:is_win && !has('win32unix')
+ let ruby = has('ruby') && !s:nvim && (v:version >= 703 || v:version == 702 && has('patch374'))
+
let s:update = {
\ 'start': reltime(),
\ 'all': todo,
@@ -754,7 +755,7 @@ function! s:update_impl(pull, force, args) abort
\ 'pull': a:pull,
\ 'force': a:force,
\ 'new': {},
- \ 'threads': (s:py2 || s:ruby || s:nvim) ? min([len(todo), threads]) : 1,
+ \ 'threads': (py2 || ruby || s:nvim) ? min([len(todo), threads]) : 1,
\ 'bar': '',
\ 'fin': 0
\ }
@@ -767,20 +768,20 @@ function! s:update_impl(pull, force, args) abort
\ '--depth 1' . (s:git_version_requirement(1, 7, 10) ? ' --no-single-branch' : '') : ''
" Python version requirement (>= 2.7)
- if s:py2 && !s:ruby && !s:nvim && s:update.threads > 1
+ if py2 && !ruby && s:update.threads > 1
redir => pyv
silent python import platform; print(platform.python_version())
redir END
- let s:py2 = s:version_requirement(
+ let py2 = s:version_requirement(
\ map(split(split(pyv)[0], '\.'), 'str2nr(v:val)'), [2, 6])
endif
- if (s:py2 || s:ruby) && !s:nvim && s:update.threads > 1
+ if (py2 || ruby) && s:update.threads > 1
try
let imd = &imd
if s:mac_gui
set noimd
endif
- if s:ruby
+ if ruby
call s:update_ruby()
else
call s:update_python() |
|
@junegunn I did manage to get the python installer working on neovim, I found putting all the buffer control into the python You can test the your patch against my vim-plug fork on branch |
|
@starcraftman Thanks. What do you think about the suggestion? It allows us to "fix" the issue right away but it seems like a stopgap method since I think it's better that we do not require python when |
|
@junegunn The idea is fine but there may be some hiccups. The main one I see is that not all nvim users will have the python lib installed. Nvim doesn't honor the Do you plan on documenting this temporary dependence? Or flag it via an error? If the latter, maybe you could check for it via something like: function! PyExe()
try
python3 1 + 1
return 'python3'
catch
try
python 1 + 1
return 'python'
catch
return ''
endtry
endtry
endfunction
Edit: I might have been optimistic at the state of Windows build we can probably ignore that case for now. |
|
@junegunn See my fork at https://github.com/starcraftman/vim-plug. The branch Edit: Ha! Managed to segfault vim with the py3 test somehow. |
Hmm, I'm getting 1, am I missing something? |
|
@junegunn Guess I made a typo, restored back to way it was. So I guess I'll just review the starting branch tomorrow and make a PR. |
Use Python installer on Neovim during `vim_starting` (#104)
|
FWIW, testing for |
|
@mhinz Ya, I mistakenly thought the has method wouldn't work anymore. All taken care of in the patch I made beginning of may. This issue remains open because the neovim installer shouldn't depend on my py installer for synchronous startup install. I haven't retested to see if the jobwait has been fixed. |
Vim was giving me static: every time I opened a file, I had to Ctrl-C before text would appear. Then when I tried to save, it would warn me "file already exists". Then when I tried to open another file in a tab or split, it just wouldn't work. Installing the neovim python package is currently necessary to make `nvim +PlugInstall` work right: junegunn/vim-plug#104
Vim was giving me static: every time I opened a file, I had to Ctrl-C before text would appear. Then when I tried to save, it would warn me "file already exists". Then when I tried to open another file in a tab or split, it just wouldn't work. Installing the neovim python package is currently necessary to make `nvim +PlugInstall` work right: junegunn/vim-plug#104
- Remove vim-plug from this repository, install it and all plugins on first run. Will be further improved when junegunn/vim-plug#104 is resolved. - Change runtimepath to include $XDG_DATA_HOME, move autoload there. - Separate plugin configuration from init.vim. The end result of this is that nvim's configuration directory is essentially static, which is a huge improvement over .gitignoring parts of it. One more step towards being able to have site-specific config.
See upstream issue: junegunn/vim-plug#104
|
Any progress on this? Just hit this issue myself. |
|
I just realized that Vimscript relinquishes control on sleep on the latest versions of Vim and Neovim. @blueyed mentioned that it was a recent change. Anyway, so it's now trivial to implement this. diff --git a/plug.vim b/plug.vim
index f63c79a..e269dda 100644
--- a/plug.vim
+++ b/plug.vim
@@ -948,7 +948,7 @@ function! s:update_impl(pull, force, args) abort
call s:warn('echom', '[vim-plug] Update Neovim for parallel installer')
endif
- let python = (has('python') || has('python3')) && (!s:nvim || has('vim_starting'))
+ let python = (has('python') || has('python3')) && !s:nvim
let ruby = has('ruby') && !s:nvim && (v:version >= 703 || v:version == 702 && has('patch374')) && !(s:is_win && has('gui_running')) && s:check_ruby()
let s:update = {
@@ -1014,6 +1014,12 @@ function! s:update_impl(pull, force, args) abort
endtry
else
call s:update_vim()
+ while s:nvim && has('vim_starting')
+ sleep 100m
+ if s:update.fin
+ break
+ endif
+ endwhile
endif
endfunction |
Since the new parallel installer for Neovim is asynchronous, we cannot do
nvim +PlugUpdate +qaas before. We can either add an option toPlugUpdatecommand to quit Vim when it's complete or wait for the addition ofjobwait()function suggested by @tarruda. Or we can simply guide the users to use the original Vim instead in this case.See:
The text was updated successfully, but these errors were encountered: