Skip to content

Loading…

Staging changes doesn't work with Cygwin git and shellslash on. #212

Closed
vadz opened this Issue · 14 comments

2 participants

@vadz

Staging changes with either :Gwrite or dp in :Gdiff mode doesn't work when using Cygwin git and shellslash Vim option is set: although no errors appear, changes are not really staged neither. In the latter case nothing seems to be done at all. In the former, an entire new file with the path c:/foo/bar/repo.git/real/file is staged while the real file real/file remains unstaged.

Unsetting shellpath allows this to work but I'm really used to having this option on under Windows so it'd be great if it could be fixed to make it work even with it (perhaps by setting it locally/temporarily in the plugin?). TIA!

@tpope tpope added a commit that referenced this issue
@tpope Override 'shellslash' for external Windows command
With 'shellslash' set, tempname() returns a filename with forward
slashes, which trips up the type command if we don't translate to
backslashes first.

Fixes half of #212.
49c6be3
@tpope
Owner

The latter was simple enough. The former I can't reproduce using msysgit, and I am loathe to go to the trouble of setting up cygwin Git. Is your repo actually named "repo.git"? Is your 'shell' cmd.exe, or is it bash (or whatever) from Cygwin?

@vadz

Thanks for the fix to the first problem, I can confirm that it works.

As for the second one, the repo is not named like this, it was just an example. And my shell option in Vim is cmd and although my $SHELL is /usr/bin/zsh I don't think it matters.

What I tried to explain with my example was that the file is written to the index with the wrong name. After doing :Gwrite and git diff --cached --stat from Cygwin shell I see that the file in the index has the name c:/path/to/git/repo/realfile, i.e. it's prefixed by the full path to the repository while normally (i.e. if I do git add or, indeed, use the incredibly convenient :Gdiff with dp -- that's really great, can't thank you enough for implementing this and making a screencast explaining how to use it, simply incomparably better than plain git add -p and especially so under Cygwin where the latter is very slow), just the "realfile" part of the path should appear. It looks like fugitive (or Vim?) constructs an absolute path for some reason but for things to interoperate between Windows Vim and Cygwin only relative paths should be used.

Is this enough to allow you to diagnose the problem or would you like me to provide some other information? Thanks again!

@tpope
Owner

I do think it matters. Setting 'shell' drastically affects how :! and system() work, and fugitive uses those like crazy.

There are three factors at play:

  1. cygwin git/msysgit
  2. shell=cmd/shell=bash
  3. shellslash/noshellslash

That's 8 combinations I have to support. I can't say I'm particularly enthused about supporting 8 combinations for a minority platform. I'd rather prioritize the two most sensible ones:

  1. Windows mode: msysgit/shell=cmd/noshellslash
  2. Cygwin mode: cygwin git/shell=bash/shellslash

The last time I seriously used Windows (many years ago), my experience was that mixing Cygwin commands and native commands was just asking for trouble. Either go whole hog and point 'shell' at bash, or retreat to the safety of msysgit.

As such, the first step is to set your shell to bash, make sure you can get the basics like :!git working, and see if things work in fugitive.

@vadz

I understand that you don't want to support all these different combinations and, frankly, :Gwrite is not that important anyhow -- I might be missing something but I don't see any advantages to it compared to just :!git add %. It's :Gdiff that is the real life changer for me so please feel free to close this issue if you think it's not worth pursuing further.

But, just in case you'd like to look at it, I can report that even with all Unix-ish options set, i.e. after :set shell=bash shellcmdglag=-c shellquote=\" it still doesn't work for me as it appears to run git with invalid arguments, so I get the error message with git help output -- but unfortunately without the command it actually tries to run. Once again, doing :git add % works so it must be doing something else. I might try to debug it further later but, again, this is not a big deal anyhow.

@tpope
Owner

There's several other options like 'shellquote' and 'shellxescape'. Make sure you set 'shellslash' correctly at Vim startup and it will set the others for you automatically.

There's been a lot of changes in 2 years so it would behoove you to try again regardless.

@vadz

Thanks for returning to this!

I've just updated to 8258025 and tried doing gvim foo in vim-fugitive own repository and :Gwq. This results in:

Error detected while processing function <SNR>30_Wq..<SNR>30_Write..<SNR>30_repo_translate..<SNR>30_repo_rev_parse..<SNR>30_repo_git_chomp:
line    1:
E484: Can't open file V:/tmp/VIo6B9D.tmp

V:/tmp is my temporary directory but beyond this I have no idea about what's going on here... This is with all "Unix" shell options. If I use shell=cmd, no error messages are given but nothing happens neither (i.e. the file is not added), whether shellslash is used or not. So from my point of view the latest version is unfortunately much worse than the previous one I used (61fac2f), as it seems I can't use it at all with Cygwin git any more.

I tried doing vi -u NONE -U NONE hoping to find out what exactly triggered this, but I don't know how to initialize vim-fugitive properly then (normally I just use pathogen for it and sourcing it manually doesn't seem to work). Also, would there be some (simple) way to see the commands executed by the plugin by chance?

TIA!

@tpope
Owner

Last chunk of the stack trace points at the function that called system(). You can throw an :echomsg in there and see.

Add an -N to your vi invocation to disable compatible mode and enable sourcing plugins.

@vadz

If I do

diff --git a/plugin/fugitive.vim b/plugin/fugitive.vim
index 7979a50..c316d7b 100644
--- a/plugin/fugitive.vim
+++ b/plugin/fugitive.vim
@@ -340,6 +340,7 @@ function! s:repo_git_command(...) dict abort
 endfunction

 function! s:repo_git_chomp(...) dict abort
+  echomsg call(self.git_command,a:000,self)
   return s:sub(system(call(self.git_command,a:000,self)),'\n$','')
 endfunction

I see that git is executed with the path in what Cygwin calls "mixed" format, i.e. Windows paths with slashes (c:/path/to/repo/foo), which doesn't work (if I do it from Cygwin command line it doesn't work neither).

And thanks for the tip about -N, but even if I do gvim -N -u NONE -U NONE foo and then :source plugin\fugitive.vim, I still get the same error with the temporary file (which doesn't exist BTW).

@tpope
Owner

What path? The git dir?

@vadz

P.S. Using procmon (Windows more-or-less equivalent of strace) I see that the temporary file is created, never written to, and then deleted -- and then Vim tries to open it again and fails, resulting in the error. I still have no idea why does this happen though :-(

@vadz

What path? The git dir?

Yes, sorry for not being clear.

@vadz

I guess the summary is: when launching (Windows) Vim from Cygwin shell with -u NONE, nothing works at all as shell options seem to be set incorrectly: shell is /usr/bin/zsh (coming from SHELL environment variable probably) while shellslash is not set. This must account for the temporary file error.

When using my Vim config (shell=cmd, shellslash set but unsetting it doesn't change anything), there are no errors but it doesn't work neither ("Calling" and "Returned" are my debug lines added before/after calling system()):

Calling: git --git-dir='w:/home/zeitlin/vimfiles/bundle/vim-fugitive/.git' rev-parse --verify foo
Returned: fatal: Needed a single revision
Calling: git --git-dir='w:/home/zeitlin/vimfiles/bundle/vim-fugitive/.git' rev-parse --verify foo
Returned: fatal: Needed a single revision
"foo" 1 line, 10 characters written
Calling: git --git-dir='w:/home/zeitlin/vimfiles/bundle/vim-fugitive/.git' add 'w:/home/zeitlin/vimfiles/bundle/vim-fugitive/foo'

The last command doesn't do anything, the file is not added.

@tpope tpope added a commit that closed this issue
@tpope Use relative path in :Gwrite
Closes #212.
9f9dabc
@tpope tpope closed this in 9f9dabc
@tpope
Owner

Only actual problem I spy here is the absolute path passed to git add, which I just fixed. I see no evidence the git dir is posing an issue, and if shell is /usr/bin/zsh rather than a Windows path, that's on you.

@vadz

Great! I can confirm that :Gwrite works with 750db5e now, both with and without shellslash. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.