MarcWeber / vim-addon-manager

manage and install vim plugins (including their dependencies) in a sane way. If you have any trouble contact me. Usually I reply within 24 hours
Other
660 stars 59 forks source link

vim-addon-manager can not install zipped plugins on Win XP #6

Closed TuxTom closed 14 years ago

TuxTom commented 14 years ago

The installation of zipped plugins (e.g. the NERDTree) fails on Win XP, as the plugin is not unzipped into the correct plugin directory but the cwd. The problem seems to be that the concatenated commands "cd [plugin-dir] && curl -o ..." are not working as supposed to.

TuxTom commented 14 years ago

After some thinking, I think I know the problem: Windows, at least up to XP, maintains one working directory for each drive letter. The 'cd' command of the Windows shell (as opposed to the 'cd' Vim command) only changes the working directory of the given drive but does not switch to that drive.

So if the current working directory is not on the same drive as the vim-addon directory, this problem occurs.

TuxTom commented 14 years ago

I was able to solve this by replacing each "exec '!cd '.... with a script-local function s:exec_in_dir( dir, command ). If the patch is of interest for you, let me know and I'll send it.

MarcWeber commented 14 years ago

Of course it is. If this fixes something for you (and all Windows XP users) I should apply it. Because unzipping and running tasks is done only once I really don't care about the extra time it takes to run the function. After you found the cause. I confirm your problem. I tried :cd dir && gvim then I typed :pwd in gvim. dir din't change. I didn't even try to change the directory. Paste your patch (or the whole file) please.

TuxTom commented 14 years ago
diff --git a/autoload/scriptmanager2.vim b/autoload/scriptmanager2.vim
index dc36672..304198f 100644
--- a/autoload/scriptmanager2.vim
+++ b/autoload/scriptmanager2.vim
@@ -88,13 +88,13 @@ endf
 fun! scriptmanager2#UpdateAddon(name)
   let directory = scriptmanager#PluginDirByName(a:name)
   if isdirectory(directory.'/.git')
-    exec '!cd '.s:shellescape(directory).'&& git pull'
+    call s:exec_in_dir(directory, '!git pull')
     return !v:shell_error
   elseif isdirectory(directory.'/.svn')
-    exec '!cd '.s:shellescape(directory).'&& svn update'
+    call s:exec_in_dir(directory, '!svn update')
     return !v:shell_error
   elseif isdirectory(directory.'/.hg')
-    exec '!cd '.s:shellescape(directory).'&& hg pull'
+    call s:exec_in_dir(directory, '!hg pull')
     return !v:shell_error
   else
     echoe "Updating plugin ".a:name." not implemented yet."
@@ -206,7 +206,7 @@ fun! scriptmanager2#Checkout(targetDir, repository)
     endif
   elseif a:repository['type'] == 'svn'
     let parent = fnamemodify(a:targetDir,':h')
-    exec '!cd '.s:shellescape(parent).'&& svn checkout '.s:shellescape(a:repository['url']).' '.s:shellescape(a:targetDir)
+    call s:exec_in_dir(parent, '!svn checkout '.s:shellescape(a:repository['url']).' '.s:shellescape(a:targetDir))
     if !isdirectory(a:targetDir)
       throw "Failed checking out ".a:targetDir."!"
     endif
@@ -222,8 +222,7 @@ fun! scriptmanager2#Checkout(targetDir, repository)
     endif
     call mkdir(a:targetDir.'/'.target,'p')
     let aname = s:shellescape(a:repository['archive_name'])
-    exec '!cd '.s:shellescape(a:targetDir).'/'.target.' &&'
-       \ .'curl -o '.aname.' '.s:shellescape(a:repository['url'])
+    s:exec_in_dir(a:targetDir.'/'.target, '!curl -o '.aname.' '.s:shellescape(a:repository['url']))
     exec addVersionFile
     call scriptmanager2#Copy(a:targetDir, a:targetDir.'.backup')

@@ -232,9 +231,8 @@ fun! scriptmanager2#Checkout(targetDir, repository)
     call mkdir(a:targetDir)
     let aname = s:shellescape(a:repository['archive_name'])
     let s = get(a:repository,'strip-components',1)
-    exec '!cd '.s:shellescape(a:targetDir).' &&'
-       \ .'curl -o '.aname.' '.s:shellescape(a:repository['url']).' &&'
-       \ .'tar --strip-components='.s.' -xzf '.aname
+    call s:exec_in_dir(a:targetDir, '!curl -o '.aname.' '.s:shellescape(a:repository['url']).' &&'
+       \ .'tar --strip-components='.s.' -xzf '.aname)
     exec addVersionFile
     call scriptmanager2#Copy(a:targetDir, a:targetDir.'.backup')

@@ -243,9 +241,8 @@ fun! scriptmanager2#Checkout(targetDir, repository)
   elseif has_key(a:repository, 'archive_name') && a:repository['archive_name'] =~ '\.tar$'
     call mkdir(a:targetDir)
     let aname = s:shellescape(a:repository['archive_name'])
-    exec '!cd '.s:shellescape(a:targetDir).' &&'
-       \ .'curl -o '.aname.' '.s:shellescape(a:repository['url']).' &&'
-       \ .'tar --strip-components=1 -xf '.aname
+    call s:exec_in_dir(a:targetDir, '!curl -o '.aname.' '.s:shellescape(a:repository['url']).' &&'
+       \ .'tar --strip-components=1 -xf '.aname)
     exec addVersionFile
     call scriptmanager2#Copy(a:targetDir, a:targetDir.'.backup')

@@ -253,9 +250,8 @@ fun! scriptmanager2#Checkout(targetDir, repository)
   elseif has_key(a:repository, 'archive_name') && a:repository['archive_name'] =~ '\.zip$'
     call mkdir(a:targetDir)
     let aname = s:shellescape(a:repository['archive_name'])
-    exec '!cd '.s:shellescape(a:targetDir).' &&'
-       \ .'curl -o '.s:shellescape(a:targetDir).'/'.aname.' '.s:shellescape(a:repository['url']).' &&'
-       \ .'unzip '.aname
+    call s:exec_in_dir(a:targetDir, '!curl -o '.s:shellescape(a:targetDir).'/'.aname.' '.s:shellescape(a:repository['url']).' &&'
+       \ .'unzip '.aname)
     exec addVersionFile
     call scriptmanager2#Copy(a:targetDir, a:targetDir.'.backup')

@@ -264,8 +260,7 @@ fun! scriptmanager2#Checkout(targetDir, repository)
     call mkdir(a:targetDir)
     let a = a:repository['archive_name']
     let aname = s:shellescape(a)
-    exec '!cd '.s:shellescape(a:targetDir).' &&'
-       \ .'curl -o '.aname.' '.s:shellescape(a:repository['url'])
+    call s:exec_in_dir(a:targetDir, '!curl -o '.aname.' '.s:shellescape(a:repository['url']))
     if a =~ '\.gz'
       " manually unzip .vba.gz as .gz isn't unpacked yet for some reason
       exec '!gunzip '.a:targetDir.'/'.a
@@ -284,6 +279,13 @@ fun! s:shellescape(s)
   return shellescape(a:s,1)
 endf

+fun! s:exec_in_dir(dir, command)
+  let old_cwd = getcwd()
+  exec 'lcd '.a:dir
+  exec a:command
+  exec 'lcd '.old_cwd
+endf
+
 " is there a library providing an OS abstraction? This breaks Winndows
 " xcopy or copy should be used there..
 fun! scriptmanager2#Copy(f,t)
MarcWeber commented 14 years ago

Hi TuxTom

I pushed a patch which is based on your implementation. I changed the implementation a little bit. I abstracted over running shell commands because I fear that yet someone else appears saying "it does not work on my OS" :-) Also your exec 'lcd '.old_cwd Does not always do what you expect. Eg if you :cd somewhere else the buffer which was used previously still will be set to old_cmd. lcd can be reset to global setting by not passing a path. However I didn't knew how to find out whether lcd was used on a buffer. So I just made Vim open a new one before running the commands which can be quit again. Can you test whether this works for you as expected?

Thank you for your help

TuxTom commented 14 years ago

Hi Marc,

I tested your patch and after some tweaking, it does install the plugins. I had to correct some things though, patch follows (seems I missed the 'call' before 's:exec_in_dir' in one case):

diff --git a/autoload/scriptmanager2.vim b/autoload/scriptmanager2.vim
index d956476..7ced36c 100644
--- a/autoload/scriptmanager2.vim
+++ b/autoload/scriptmanager2.vim
@@ -88,13 +88,13 @@ endf
 fun! scriptmanager2#UpdateAddon(name)
   let directory = scriptmanager#PluginDirByName(a:name)
   if isdirectory(directory.'/.git')
-    call s:exec_in_dir([{'d': a:directory, 'c': 'git pull'}])
+    call s:exec_in_dir([{'d': directory, 'c': 'git pull'}])
     return !v:shell_error
   elseif isdirectory(directory.'/.svn')
-    call s:exec_in_dir([{'d': a:[directory, 'c': 'svn update'}])
+    call s:exec_in_dir([{'d': directory, 'c': 'svn update'}])
     return !v:shell_error
   elseif isdirectory(directory.'/.hg')
-    call s:exec_in_dir([{'d': a:directory, 'c': 'hg pull'}])
+    call s:exec_in_dir([{'d': directory, 'c': 'hg pull'}])
     return !v:shell_error
   else
     echoe "Updating plugin ".a:name." not implemented yet."
@@ -222,7 +222,7 @@ fun! scriptmanager2#Checkout(targetDir, repository)
     endif
     call mkdir(a:targetDir.'/'.target,'p')
     let aname = s:shellescape(a:repository['archive_name'])
-    s:exec_in_dir([{'d':  a:targetDir.'/'.target, 'c': 'curl -o '.aname.' '.s:shellescape(a:repository['url']}]))
+    call s:exec_in_dir([{'d':  a:targetDir.'/'.target, 'c': 'curl -o '.aname.' '.s:shellescape(a:repository['url']}]))
     exec addVersionFile
     call scriptmanager2#Copy(a:targetDir, a:targetDir.'.backup')

@@ -281,7 +281,6 @@ endf

 " cmds = list of {'d':  dir to run command in, 'c': the command line to be run }
 fun! s:exec_in_dir(cmds)
-  let iswin = 
   if has('win16') || has('win32') || has('win64')
     " set different lcd in extra buffer:
     split

What I noticed so far is that ":ActivateAddon" with a zipped plugin deletes the current buffer and changes the cwd to the directory of the installed plugin. It works with plugins that are pulled from git.

Btw.: thanks for your help and the quick response!

MarcWeber commented 14 years ago

I applied and uploaded your patch. Let me know when yout think that I can close this bug.

TuxTom commented 14 years ago

I finally found some time to check, why the current buffer is deleted. The problem is that 'split' does not create a new buffer but a new window but 'bw!' deletes the buffer. Thus the previous window is also closed. The solution is to replace 'split' with 'new', patch:

diff --git a/autoload/vcs_checkouts.vim b/autoload/vcs_checkouts.vim
index 8635eab..601f781 100644
--- a/autoload/vcs_checkouts.vim
+++ b/autoload/vcs_checkouts.vim
@@ -48,7 +48,7 @@ endf
 fun! vcs_checkouts#ExecIndir(cmds)
   if has('win16') || has('win32') || has('win64')
     " set different lcd in extra buffer:
-    split
+    new
     let lcd=""
     for c in a:cmds
       if has_key(c, "d")
MarcWeber commented 14 years ago

Thanks! pushed. Time to close this issue?

TuxTom commented 14 years ago

I think so...