Awesome installer has bugs

jdevera commented 12 years ago

I'm testing the nin branch and found this issue.

These are my bundles:

Bundle 'gmarik/vundle'
Bundle 'tomtom/tlib_vim'
Bundle 'MarcWeber/vim-addon-mw-utils'
Bundle "garbas/snipmate.vim"
Bundle 'honza/snipmate-snippets'
Bundle 'tpope/vim-repeat'
Bundle 'tpope/vim-surround'
Bundle 'Align'                  
Bundle 'camelcasemotion' 
Bundle 'Color-Scheme-Explorer'
Bundle 'Conque-Shell'         
Bundle 'davidoc/todo.txt-vim'
Bundle 'godlygeek/csapprox'
Bundle 'hallison/vim-markdown'        
Bundle 'jdevera/vim-stl-syntax'
Bundle 'majutsushi/tagbar'
Bundle 'pylint.vim'       
Bundle 'python.vim--Vasiliev'  
Bundle 'scrooloose/nerdcommenter'
Bundle 'scrooloose/nerdtree'
Bundle 'ShowMarks'          
Bundle 'TaskList.vim'            
Bundle 'tpope/vim-fugitive' 
Bundle 'xolox/vim-notes'
Bundle 'tomasr/molokai'

This is the Log:

Already up-to-date.^@
Cloning into /home/jdevera/.vim/bundle/tlib_vim...^@fatal: https://github.com/vim-scripts/tlib_vim.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/vim-addon-mw-utils...^@
Cloning into /home/jdevera/.vim/bundle/snipmate.vim...^@fatal: https://github.com/vim-scripts/snipmate.vim.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/snipmate-snippets...^@
Cloning into /home/jdevera/.vim/bundle/vim-repeat...^@fatal: https://github.com/vim-scripts/vim-repeat.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/vim-surround...^@fatal: https://github.com/vim-scripts/vim-surround.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/Align...^@
Cloning into /home/jdevera/.vim/bundle/camelcasemotion...^@
Cloning into /home/jdevera/.vim/bundle/Color-Scheme-Explorer...^@
Cloning into /home/jdevera/.vim/bundle/Conque-Shell...^@
Cloning into /home/jdevera/.vim/bundle/todo.txt-vim...^@fatal: https://github.com/vim-scripts/todo.txt-vim.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/csapprox...^@fatal: https://github.com/vim-scripts/csapprox.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/vim-markdown...^@fatal: https://github.com/vim-scripts/vim-markdown.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/vim-stl-syntax...^@fatal: https://github.com/vim-scripts/vim-stl-syntax.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/tagbar...^@fatal: https://github.com/vim-scripts/tagbar.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/pylint.vim...^@
Cloning into /home/jdevera/.vim/bundle/python.vim--Vasiliev...^@
Cloning into /home/jdevera/.vim/bundle/nerdcommenter...^@fatal: https://github.com/vim-scripts/nerdcommenter.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/nerdtree...^@fatal: https://github.com/vim-scripts/nerdtree.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/ShowMarks...^@
Cloning into /home/jdevera/.vim/bundle/TaskList.vim...^@
Cloning into /home/jdevera/.vim/bundle/vim-fugitive...^@fatal: https://github.com/vim-scripts/vim-fugitive.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/vim-notes...^@fatal: https://github.com/vim-scripts/vim-notes.git/info/refs not found: did you run git update-server-info on the server?^@
Cloning into /home/jdevera/.vim/bundle/molokai...^@

So it seems the user part of the repo is getting lost and the default vim-script is being used.

gmarik commented 12 years ago

Oh, good catch. Shouldn't be too hard to fix, just have to use original URIs instead just names. I didn't notice that! Thanks you!

gmarik commented 12 years ago

Please update Vundle and try again! Should be fixed.

It's omitting options for now, so it still to be added

jdevera commented 12 years ago

Fixed indeed! Thanks!

gmarik commented 12 years ago

reopening to keep track of issues with new installer until it's pushed to master

gmarik commented 12 years ago

@taverius asked if it's possible to hide scrollbar during the installation.

Taverius commented 12 years ago

That looks laborious.

Its set by guioptions, and since you cannot set guioptions all at once - you have to do it one option at the time, and there are 4 applicable options ... You'd have to parse the option-string, trigger a remove for all applicable ones, run the script then re-add all the removed options ... sounds like a can of worms and a half, just leave that alone :)

jdevera commented 12 years ago

I found some new issues.

My HEAD is at 4218a579343e6ba5d94e0ca73a5d774de7cf5ad5

Issue 1

I run BundleInstall (same bundles as stated before, which includes ONE wrong repo spec, just to see what happens when the repo can't be found.). All seems to be fine here.

But when I run VundleLog right after, I get this:

Error detected while processing function <SNR>27_view_log:
line    3:
E37: No write since last change (add ! to override)
Press ENTER or type command to continue```

Issue 2

I create a couple of empty directories under my bundle directory and then run BundleClean, it asks for for confirmation and I say yes. then it does delete the directories but I don't get any notification of success or failure.

So I created a directory with a file in it and removed write permission from the directory: cd bundle mkdir scratch touch scratch/onefile chmod a-w scratch/onefile

And ran BundleClean again. I get this from the shell vim creates:

rm: cannot remove `/home/jdevera/.vim/bundle/pollo/file': Permission denied

shell returned 1

Perhaps it can be captured somehow.

Issue 3

All commands that open a new window leave that windows in a modified state, needs to be closed with q!. There must be some way in which this can be avoided, so one can just close it normally with q

Issue 4

Installation leaves rubbish in the search register. I close vim after installation and the next file I open has all contents highlighted. I have set hlsearch in my .vimrc and the search register (echo @/) is left with .* from the installation.

jdevera commented 12 years ago

Actually issue 1 is a consequence of issue 3. I used w! in the installation window and then VundleLog worked fine.

BTW:

Issue 5

All entries in VundleLog have ^@ athe end of the line. The fileformat is unix and I am in Linux, I guess this should not happen.

gmarik commented 12 years ago

Good review @jdevera! Thanks!

As for

1 - yeah, VundleLog needs improvements, right now it's just a quick hack
2 - will modify BundleClean work same way as BundleInstall works, stepping through each entry and marking status on gutter
3 - will look, do you have a testcase for that?
4 - that's interesting, cuz i though that was already removed.
5 - should be fixed along with 1

jdevera commented 12 years ago

For 3, I'm not doing anything special, I guess the simplest and quickest is to run the Bundles command and try attempt to exit the window with q

Taverius commented 12 years ago

Bugs noticed on nin as of commit 5d4ee2d

1) BundleInstall! is broken. Its just complains about Error installing ", once for every bundle. 2) Installer complains about the window being read-only

gmarik commented 12 years ago

@Taverius,

cannot reproduce (1), do u have more details on that?
yup, noticed that too, but not sure how to reproduce it consistently

Taverius commented 12 years ago

@gmarik, nothing special, I just ran :BundleInstall!.

:VundleLog is just fatal: could not create work tree dir 'C:\Users\Taverius\vimfiles\bundle"'.: Invalid argument, times as many bundles as I have.

Looks like a win-specific path booboo or something.

Taverius commented 12 years ago

Had a few spare minutes, the latest commits broke install period, not just global update - BundleInstall 'foo/bar' gives the same error.

gmarik commented 12 years ago

@Taverius, could you pls check if BundleInstall gmarik/golife.vim works? Installing foo/bar errors out as it's non existing repo, so that's a totally valid case. Still unable to reproduce the error above...(

gmarik commented 12 years ago

made logging more verbose, so it should show what's the cause of the issue.

Taverius commented 12 years ago

I said foo/bar as in any repo. I get the same error with gmarik/vundle:

Bundle 
$ git clone https://github.com/vim-scripts/.git "C:\Users\Taverius\vimfiles\bundle\"
> fatal: could not create work tree dir 'C:\Users\Taverius\vimfiles\bundle"'.: Invalid argument

jdevera commented 12 years ago

I wonder why the modified buffer problem doesn't show to you, it must be scared. I managed to solve by setting the buffer as not modified, see pull request at #73

Taverius commented 12 years ago

Oh it does, it complains about the buffer being modified while read-only, and do I really want to write to it? (predictable impatient answer)

jdevera commented 12 years ago

@Taverius did you try my change? I'm only one only commit ahead of gmarik's and have opened a pull request. It'd be great if you could try it before: jdevera/vundle@a56926fd5bce27a8e0b9748d63be3aa396c0d7eb or simply clone it and checkout the nin branch.

gmarik commented 12 years ago

Bundle 
$ git clone https://github.com/vim-scripts/.git "C:\Users\Taverius\vimfiles\bundle\"
> fatal: could not create work tree dir 'C:\Users\Taverius\vimfiles\bundle"'.: Invalid argument

@Taverius, never had this issue myself

As you see from the log the bundle name is missing(or blank), something is clearly wrong... Probably need to boot up my WinXP box to test it...

Taverius commented 12 years ago

@jdevera did just now, stops it from whining at me which is nice :)

@gmarik 6c6efeeee59fc64fd4402621459af2571b16a818 is when nin stops working for me.

jdevera commented 12 years ago

Tested nin today in Windows 7 64bit and it works for me, @Taverius, perhaps you could share some details of your config?, might find why it is not working for you.

Note, to make it work I had to do this:

if has('win32') || has('win64')
    " Add vundle to the runtime path
    set rtp+=~/vimfiles/bundle/vundle/
    " Call Vundle
    call vundle#rc('$HOME/vimfiles/bundle/')
else
    " Add vundle to the runtime path
    set rtp+=~/.vim/bundle/vundle/
    " Call Vundle
    call vundle#rc()
endif

Taverius commented 12 years ago

I don't have the conditionals, as I don't work on unix anymore - so for me its just:

filetype off

set runtimepath+=~/vimfiles/bundle/vundle

call vundle#rc('$HOME/vimfiles/bundle/')

Bundle 'gmarik/vundle'

If you want parts of my _vimrc you'll have to be more specific - its 1102 lines.

jdevera commented 12 years ago

@Taverius, Yes! :) I'd like to try it with your full vimrc. Maybe you can create a gist with it or use some pastebin website?

Taverius commented 12 years ago

@jdevera https://gist.github.com/1136639 enjoy :)

jdevera commented 12 years ago

@Taverius thanks, I have found some inspiration in you vimrc :) . I thought there could be some interactions between stuff defined there and stuff that Vundle uses, but could not find anything. So I tried it on my machine, under windows 7 64bit. It worked like a charm. I mean, the installer in the nin branch worked. It only complained about the bundle scroolooose/nerdtree and I think it is because it has one letter o too many. I wish I could reproduce your problem, but I can't. Anything else that we should explore? @gmarik, any suggestions?

Taverius commented 12 years ago

@jdevera you're welcome. I have the compulsion to pick through all vimrcs I find and assimilate borg-style. So far I've managed to avoid our host's ...

I would have been surprised if you did find anything setting-wise, I run VimLint regularly.

@gmarik I did, just now, find a culprit. If you have code (such as I do) that creates & loads viewfiles automagically, nin-vundle becomes very unhappy if you start it without deleting the view file.

Just this afternoon I wrote some basic exclusion tests for that - see updated vimrc - so I will include that, but:

If the 'quickfix' feature is present, vundle should :setl buftype to nofile for the install window - see :h buftype. Not only is it correct but most examples on the vim wiki use that to detect files not to auto-create/load views for.
I have set viewoptions=cursor,folds,options,slash,unix.

optionsis the likely culprit, but one must note that if having an option stored in a view file breaks the code, the code must be fixed ... not only is it doing something wrong, if a view breaks it, imagine what happens if run from inside a session, which is likely to also store some variables.

In the year and a half since I saw the light and left emacs for vim this is the first time I have trouble with viewfiles ...

gmarik commented 12 years ago

@jdevera, I believe @Taverius found the cause...

1) buftype is important as view shouldn't be written. 2) viewoptions is a tricky one... is set viewoptions=cursor,folds,options,slash,unix enough to trigger error?

And thanks for useful suggestions! )

Taverius commented 12 years ago

@gmarik having options in there is the reason.

Then the view stores local options (anything that's been :setl) and that makes vundle all :sadpanda:.

Could be there's an option you toggle without toggling back, and then when vundle goes to work in an env where it s been toggled it goes HORRIBLEAH WRONG.

I'd like to figure out how things like TabBar and LustyExplorer & friends write to buffers and not have vim ask you if you want to save it when you close, like vim does right now for [Vundle] Installer

gmarik commented 12 years ago

@Taverius, AFAIK there's nothing that can be done about viewoptions as it's a global option(and Vundle doesn't want to modify it). To prevent buffer written to file i've used your buftype suggestion with noswapfile. Both are per-buffer options so they're safe to use. So far (with latest nin) i haven't gotten a single warning asking to save buffer.

Latest nin also contains some other mods which may break for you...just sayin ;)

Let me know if we're on same page! Thx

gmarik commented 12 years ago

also I was surprised with rm -rf nonexistingdir always returning 0. This makes :BundleClean "never fail", which sucks. Need to come up with another solution...so if you have any ideas about please let me know!

how to reproduce: $ rm -rf nonexistingdir; echo $?

Taverius commented 12 years ago

@gmarik will test new commits

Don't force the rm? :)
viewoptions yeah that's probably safest.
buftype stuff best stuff. Still need to go through the installer code to see why having localoptions persist between invocations breaks installer.

Taverius commented 12 years ago

Since this our running issue for the branch - a minor consistency niggle in calling conventions for the commands:

:BundleInstall will take an unquoted argument, eg :BundleInstall Gundo works just fine.
:Bundle does not and :Bundle Gundo will complain about undefined variables.

Is there a design reason for this? Otherwise its probably a good idea to make them consistent :)

gmarik commented 12 years ago

yup, mostly by design. :Bundle is used mostly in .vimrc, while :BundleInstall in command line. Omitting quotes when typing BundleInstall Gundo saves 2 key-presses :). But I agree, it's not consistent...

gmarik commented 12 years ago

So, ppl, how are we doing with nin branch? Are we having any major issues? Otherwise i'm addding docs and probably pushing Vundle 0.9 )

0 commented 12 years ago

@gmarik, sorry, I haven't been following the development very much. However, having switched to the nin branch just now, I'm very much impressed; it looks great!

After some typical-use-case testing, my only suggestions for improvement have to do with the log output:

Preserving the line breaks in the messages from git would make updates and errors easier to read.
Since the log buffer is reused, printing a timestamp before each run of the installer would be nice.

gmarik commented 12 years ago

@0 thanks for the review, much appreciated!

actually that's the output the system call returns. Need to replace ^@ with \r i guess.
good idea, will add that

gmarik commented 12 years ago

@Taverius:

Don't force the rm? :)

Then rm asks for confirmation, which is no go, unless i can confirm it programmatically(which i don't really know how to do)

Taverius commented 12 years ago

@gmarik: empty(glob('/path/to/dir', 1)) or len(glob('/path/to/dir', 1)) after the rm to see if its still there? :)

The 1 is to stop wildignore and suffixes option values from affecting your results.

randymorris commented 12 years ago

If you haven't run any vundle operations in the current vim session, attempting to view the log throws the following error because g:vundle_log is not defined.

Error detected while processing function <SNR>42_view_log:
line    2:
E121: Undefined variable: g:vundle_log
E116: Invalid arguments for function writefile

gmarik commented 12 years ago

@Taverius, that will work as for workaround...

@rson, yup Logging needs to be fixed... And thanks for reviewing!

gmarik commented 12 years ago

i think now it's almost a release candidate

time to update docs

jdevera commented 12 years ago

Ok I'm gonna try to give it a good run later today

randymorris commented 12 years ago

Any chance we can get an option for where to put the temp files? Another hidden directory in $HOME is less than ideal, especially when many people already have a temp dir specifically for vim-created stuff.

gmarik commented 12 years ago

@rson, yeah, was thinking about that...

vundle.log i believe should be located somewhere in known location (so CLI lovers can tail it)
as for json data file - it may easily go to tempdir

gmarik commented 12 years ago

or just use ~/.vim/bundle/.vundle

randymorris commented 12 years ago

@gmarik since you can change the name of the directory a bundle is installed to it is probably not safe to do that unless you can programatically get that info (I haven't looked at that in a long time).

gmarik commented 12 years ago

@rson yeah, g:bundle_dir is configurable and Vundle can get that info programatically, so i guess it's safe enough...

jdevera commented 12 years ago

Just tried latest in nin under Windows. It installs fine but when I try to see the log, I get a "Can't create file" error for the vundle.log. I was wondering if maybe the writefile function does not create the directory hierarchy if it doesn't already exist. I guess this will go away once the log file is stored in g:bundle_dir?

VundleVim / Vundle.vim