Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

winpty does not work well with MSYS_NO_PATHCONV=1 #738

Closed
1 task done
ghost opened this issue Apr 20, 2016 · 13 comments
Closed
1 task done

winpty does not work well with MSYS_NO_PATHCONV=1 #738

ghost opened this issue Apr 20, 2016 · 13 comments

Comments

@ghost
Copy link

ghost commented Apr 20, 2016

  • I was not able to find an open
    or closed issue
    matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? 32-bit or 64-bit? Include the
    output of git version as well.

Git for Windows 64bit on Windows 10

$ git --version
git version 2.8.0.windows.1
  • Which version of Windows are you running? 32-bit or 64-bit?

Windows 10, Pro, 64-bit

  • What options did you set as part of the installation? Or did you choose the
    defaults?

default

  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

I've svn, docker

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

bash

$ unset MSYS_NO_PATHCONV; MSYS_NO_PATHCONV=1 cmd /K dir 'c:\Windows'
$ unset MSYS_NO_PATHCONV; MSYS_NO_PATHCONV=1 winpty -- cmd /K dir 'c:\Windows'
$ unset MSYS_NO_PATHCONV; winpty -- cmd /K dir 'c:\Windows'

  • What did you expect to occur after running these commands?

both 1st and 2nd command should run "dir" on C:\Windows

  • What actually happened instead?

The 1st command, will run dir and stay in the cmd console, however, cursor will broken (up/down keys)
The 2nd command, will skip the "dir" but have cmd console, cursor work as expected.
The 3nd command, same as the 2nd.

It does matter not only doing for cmd.exe, but also many useful interactive shells for devs, e.g. mysql-sh, sqlcmd and sqlplus.

  • If the problem was occurring with a specific repository, can you provide the
    URL to that repository to help us with testing?

N/A

@dscho
Copy link
Member

dscho commented Apr 20, 2016

Yeah, MSYS_NO_PATHCONV was never meant for large invocations, just for simple command-line calls.

@dscho
Copy link
Member

dscho commented Apr 20, 2016

So the resolution is not to use MSYS_NO_PATHCONV with such invocations. This works Just Fine For Me in Git Bash:

 cmd //k 'dir c:\windows'

@dscho dscho closed this as completed Apr 20, 2016
@ghost
Copy link
Author

ghost commented Apr 21, 2016

Thanks,
It turns out that nothing wrong of MSYS_NO_PATHCONV but winpty itself.

$ /oracle/sqlplus.exe 'system/oracle@boot2docker/XE'

works fine, but

$ winpty /oracle/sqlplus.exe 'system/oracle@boot2docker/XE'

will execute
/oracle/sqlplus.exe 'system\oracle@boot2docker\XE' (checked in process explorer)
and it just destroy the login auth.

Minimal, Complete, and Verifiable example
If docker-machine + sath89/oracle-xe-11g image + oracle's instantclient-sqlplus-windows.x64 is minimum, (Just kidding, OK, it is not that minimum)

@dscho
Copy link
Member

dscho commented Apr 21, 2016

It turns out that nothing wrong of MSYS_NO_PATHCONV but winpty itself.

That is not correct. It is MSYS2's runtime's POSIX->Windows path conversion that does this, and it does the expected thing because at some point the quotes are lost and MSYS2 has to interpret the argument as if it were a relative path.

Take-home lesson: never use winpty if not needed. (There are actually more reasons for that, but the problem discussed here should be convincing enough already.)

@seishun
Copy link

seishun commented Apr 21, 2016

It is MSYS2's runtime's POSIX->Windows path conversion that does this, and it does the expected thing because at some point the quotes are lost and MSYS2 has to interpret the argument as if it were a relative path.

This statement is incorrect, as shown by the following experiment:

args.c -> args.exe

#include <stdio.h>

int main(int argc, char* argv[]) {
    for (int i = 0; i < argc; i++) {
        puts(argv[i]);
    }
}

Now observe:

Nikolai@DESKTOP-BF4LIO9 MINGW64 ~
$ ~/Desktop/args.exe /asdf
C:\Users\Nikolai\Desktop\args.exe
C:/Program Files/Git/asdf

Nikolai@DESKTOP-BF4LIO9 MINGW64 ~
$ ~/Desktop/args.exe asdf/uoid
C:\Users\Nikolai\Desktop\args.exe
asdf/uoid

Nikolai@DESKTOP-BF4LIO9 MINGW64 ~
$ ~/Desktop/args.exe 'asdf/uoid'
C:\Users\Nikolai\Desktop\args.exe
asdf/uoid

As you can see, MSYS2 doesn't touch your arguments if it doesn't start with a slash. So yes, it's winpty doing the replacement.

Take-home lesson: never use winpty if not needed. (There are actually more reasons for that, but the problem discussed here should be convincing enough already.)

You don't have choice when the program you're using is called node, php, php5, psql or python2.7, since in those cases winpty is forced on you. Which results in issues like #740.

@dscho
Copy link
Member

dscho commented Apr 22, 2016

You don't have choice when the program you're using is called node, php, php5, psql or python2.7, since in those cases winpty is forced on you.

Yes, you have a choice. Just call them as node.exe, php.exe, etc.

If you have any splendid idea how to solve the interactive console problem in a better way, you know, I am all ears.

@seishun
Copy link

seishun commented Apr 22, 2016

If you have any splendid idea how to solve the interactive console problem in a better way, you know, I am all ears.

Sure. Just have the users call winpty node etc explicitly.

@dscho
Copy link
Member

dscho commented Apr 22, 2016

Just have the users call winpty node etc explicitly.

Umm. Is this a joke? How is that a better idea? How does that prevent users flooding me (not you, of course) with bug reports?

Sorry, I do not have time for this.

@renatosilva
Copy link

renatosilva commented Jun 21, 2016

It turns out that nothing wrong of MSYS_NO_PATHCONV but winpty itself.

That is not correct. It is MSYS2's runtime's POSIX->Windows path conversion that does this...

No, that is correct. The MSYS2 runtime does not perform path conversion for its own programs. This is a very rudimentary conversion performed by winpty that seems to be causing problems all over the place. This should be fixed now, see this commit.

@coderextreme
Copy link

I got directed here from the nodejs forum. This also fails under cygwin:

PROMPT$ yes | xargs node.exe fillbuffer.js | tee xxx
0
0
[hangs... It should continue until the file system is full in xxx]

PROMPT$ cat fillbuffer.js
console.log(0);

@dscho
Copy link
Member

dscho commented Feb 24, 2017

@coderextreme Does it work if you call "C:\Program Files\Git\usr\bin\bash.exe" -l -i from a cmd window and execute that command-line there?

@coderextreme
Copy link

coderextreme commented Feb 24, 2017 via email

@seishun
Copy link

seishun commented Feb 25, 2017

@coderextreme It seems you either didn't read or didn't understand my comment. Please read it again. There is no evidence that it's a Node.js bug.

Your problem also doesn't have anything to do with winpty (as it was pointed out to you), so I'm not sure why you brought it up in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants