Merge #2735 'provider: Only call system() once, don't use Python 3 interpreter for +python, improve messages'.

4 files changed, 71 insertions, 50 deletions

diff --git a/runtime/autoload/provider/python.vim b/runtime/autoload/provider/python.vim
index 4c43c8a613..d4a509864f 100644
--- a/runtime/autoload/provider/python.vim
+++ b/runtime/autoload/provider/python.vim
@@ -10,10 +10,6 @@ endif
 let g:loaded_python_provider = 1
 
 let [s:prog, s:err] = provider#pythonx#Detect(2)
-if s:prog == ''
-  " Detection failed
-  finish
-endif
 
 function! provider#python#Prog()
   return s:prog
@@ -23,6 +19,11 @@ function! provider#python#Error()
   return s:err
 endfunction
 
+if s:prog == ''
+  " Detection failed
+  finish
+endif
+
 let s:plugin_path = expand('<sfile>:p:h').'/script_host.py'
 
 " The Python provider plugin will run in a separate instance of the Python
@@ -31,6 +32,9 @@ call remote#host#RegisterClone('legacy-python-provider', 'python')
 call remote#host#RegisterPlugin('legacy-python-provider', s:plugin_path, [])
 
 function! provider#python#Call(method, args)
+  if s:err != ''
+    return
+  endif
   if !exists('s:host')
     let s:rpcrequest = function('rpcrequest')
 
@@ -38,7 +42,10 @@ function! provider#python#Call(method, args)
     try
       let s:host = remote#host#Require('legacy-python-provider')
     catch
+      let s:err = v:exception
+      echohl WarningMsg
       echomsg v:exception
+      echohl None
       finish
     endtry
   endif
diff --git a/runtime/autoload/provider/python3.vim b/runtime/autoload/provider/python3.vim
index 1a52ade0ef..f469bf55aa 100644
--- a/runtime/autoload/provider/python3.vim
+++ b/runtime/autoload/provider/python3.vim
@@ -10,10 +10,6 @@ endif
 let g:loaded_python3_provider = 1
 
 let [s:prog, s:err] = provider#pythonx#Detect(3)
-if s:prog == ''
-  " Detection failed
-  finish
-endif
 
 function! provider#python3#Prog()
   return s:prog
@@ -23,6 +19,11 @@ function! provider#python3#Error()
   return s:err
 endfunction
 
+if s:prog == ''
+  " Detection failed
+  finish
+endif
+
 let s:plugin_path = expand('<sfile>:p:h').'/script_host.py'
 
 " The Python3 provider plugin will run in a separate instance of the Python3
@@ -31,6 +32,9 @@ call remote#host#RegisterClone('legacy-python3-provider', 'python3')
 call remote#host#RegisterPlugin('legacy-python3-provider', s:plugin_path, [])
 
 function! provider#python3#Call(method, args)
+  if s:err != ''
+    return
+  endif
   if !exists('s:host')
     let s:rpcrequest = function('rpcrequest')
 
@@ -38,10 +42,12 @@ function! provider#python3#Call(method, args)
     try
       let s:host = remote#host#Require('legacy-python3-provider')
     catch
+      let s:err = v:exception
+      echohl WarningMsg
       echomsg v:exception
+      echohl None
       finish
     endtry
   endif
-
   return call(s:rpcrequest, insert(insert(a:args, 'python_'.a:method), s:host))
 endfunction
diff --git a/runtime/autoload/provider/pythonx.vim b/runtime/autoload/provider/pythonx.vim
index 6137d16fcb..412c93d5a0 100644
--- a/runtime/autoload/provider/pythonx.vim
+++ b/runtime/autoload/provider/pythonx.vim
@@ -5,65 +5,71 @@ endif
 
 let s:loaded_pythonx_provider = 1
 
-function! provider#pythonx#Detect(ver) abort
-  let host_var = (a:ver == 2) ?
+function! provider#pythonx#Detect(major_ver) abort
+  let host_var = (a:major_ver == 2) ?
         \ 'g:python_host_prog' : 'g:python3_host_prog'
-  let skip_var = (a:ver == 2) ?
+  let skip_var = (a:major_ver == 2) ?
         \ 'g:python_host_skip_check' : 'g:python3_host_skip_check'
   let skip = exists(skip_var) ? {skip_var} : 0
   if exists(host_var)
-    " Disable auto detection
-    let [check, err] = s:check_interpreter({host_var}, a:ver, skip)
-    return check ? [{host_var}, err] : ['', err]
+    " Disable auto detection.
+    let [result, err] = s:check_interpreter({host_var}, a:major_ver, skip)
+    if result
+      return [{host_var}, err]
+    endif
+    return ['', 'provider/pythonx: Could not load Python ' . a:major_ver
+          \ . ' from ' . host_var . ': ' . err]
   endif
 
-  let detect_versions = (a:ver == 2) ?
-        \   ['2.7', '2.6', '2', '']
-        \ : ['3.5', '3.4', '3.3', '3.2', '3', '']
+  let prog_suffixes = (a:major_ver == 2) ?
+        \   ['2', '2.7', '2.6', '']
+        \ : ['3', '3.5', '3.4', '3.3', '']
 
-  for prog in map(detect_versions, "'python' . v:val")
-    let [check, err] = s:check_interpreter(prog, a:ver, skip)
-    if check
-      let [check, err] = s:check_version(prog, a:ver, skip)
+  let errors = []
+  for prog in map(prog_suffixes, "'python' . v:val")
+    let [result, err] = s:check_interpreter(prog, a:major_ver, skip)
+    if result
       return [prog, err]
     endif
+
+    " Accumulate errors in case we don't find
+    " any suitable Python interpreter.
+    call add(errors, err)
   endfor
 
-  " No Python interpreter
-  return ['', 'Neovim module installed Python'
-        \ .a:ver.' interpreter is not found.']
+  " No suitable Python interpreter found.
+  return ['', 'provider/pythonx: Could not load Python ' . a:major_ver
+        \ . ":\n" .  join(errors, "\n")]
 endfunction
 
-function! s:check_version(prog, ver, skip) abort
-  if a:skip
-    return [1, '']
+function! s:check_interpreter(prog, major_ver, skip) abort
+  let prog_path = exepath(a:prog)
+  if prog_path == ''
+    return [0, a:prog . ' not found in search path or not executable.']
   endif
 
-  let get_version =
-        \ ' -c "import sys; sys.stdout.write(str(sys.version_info[0]) + '.
-        \ '\".\" + str(sys.version_info[1]))"'
-  let min_version = (a:ver == 2) ? '2.6' : '3.3'
-  if system(a:prog . get_version) >= min_version
+  if a:skip
     return [1, '']
   endif
-  return [0, 'Python ' . get_version . ' interpreter is not supported.']
-endfunction
 
-function! s:check_interpreter(prog, ver, skip) abort
-  if !executable(a:prog)
-    return [0, 'Python'.a:ver.' interpreter is not executable.']
+  " Try to load neovim module, and output Python version.
+  let prog_ver = system(a:prog . ' -c ' .
+        \ '''import sys; sys.stdout.write(str(sys.version_info[0]) + '.
+        \ '"." + str(sys.version_info[1])); '''.
+        \ (a:major_ver == 2 ?
+        \   '''import pkgutil; exit(pkgutil.get_loader("neovim") is None)''':
+        \   '''import importlib; exit(importlib.find_loader("neovim") is None)''')
+        \ )
+  if v:shell_error
+    return [0, prog_path . ' does have not have the neovim module installed. '
+          \ . 'See ":help nvim-python".']
   endif
 
-  if a:skip
+  let min_version = (a:major_ver == 2) ? '2.6' : '3.3'
+  if prog_ver =~ '^' . a:major_ver && prog_ver >= min_version
     return [1, '']
   endif
 
-  " Load neovim module check
-  call system(a:prog . ' -c ' .
-        \ (a:ver == 2 ?
-        \   '''import pkgutil; exit(pkgutil.get_loader("neovim") is None)''':
-        \   '''import importlib; exit(importlib.find_loader("neovim") is None)''')
-        \ )
-  return [!v:shell_error, 'Python'.a:ver.' interpreter have not neovim module.']
+  return [0, prog_path . ' is Python ' . prog_ver . ' and cannot provide Python '
+        \ . a:major_ver . '.']
 endfunction
-
diff --git a/runtime/autoload/remote/host.vim b/runtime/autoload/remote/host.vim
index 2a7cd3c43b..a8b505d096 100644
--- a/runtime/autoload/remote/host.vim
+++ b/runtime/autoload/remote/host.vim
@@ -214,10 +214,12 @@ function! s:RequirePythonHost(name)
   catch
     echomsg v:exception
   endtry
-  throw 'Failed to load python host. You can try to see what happened ' .
-    \ 'by starting Neovim with $NVIM_PYTHON_PYTHON_LOG and opening '.
+  throw 'Failed to load Python host. You can try to see what happened '.
+    \ 'by starting Neovim with the environment variable '.
+    \ '$NVIM_PYTHON_LOG_FILE set to a file and opening '.
     \ 'the generated log file. Also, the host stderr will be available '.
-    \ 'in Neovim log, so it may contain useful information.'
+    \ 'in Neovim log, so it may contain useful information. '.
+    \ 'See also ~/.nvimlog.'
 endfunction
 
 call remote#host#Register('python', function('s:RequirePythonHost'))