Skip to content

winpty does not work well with MSYS_NO_PATHCONV=1 #738

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

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.

mjcheetham added a commit to mjcheetham/git that referenced this issue Apr 7, 2025
We should standardise on a location (and naming convention) to store
secrets used as part of the build process for `microsoft/git`. Azure Key
Vault is an approved store for secrets inside of Microsoft, and so we
should migrate any existing secrets from GitHub environment secrets to
AKV. Access to the AKV is via Managed Identity and [federated
authentication through GitHub
Actions](https://learn.microsoft.com/en-us/azure/developer/github/connect-from-azure-openid-connect).

The names of the secrets, and other configuration for accessing the Key
Vault, remain in GitHub environment secrets. Forks of the project can
therefore define the same set of environment secrets pointing at their
own AKV to utilise the same build process.

For reference, the set of secrets that must be defined for the workflow
are as follows:

Secret Name|Description
-|-
`AZURE_VAULT`|Name of the Azure Key Vault containing build secrets.
`AZURE_CLIENT_ID`|Client ID of the Managed Identity with access to the
Key Vault.
`AZURE_TENANT_ID`|Tenant ID where the Managed Identity resides.
`AZURE_SUBSCRIPTION_ID`|Subscription ID where the Key Vault resides.
`WIN_CODESIGN_CERT_SECRET_NAME`|Name of the AKV secret containing the
base64 encoded Windows Authenticode signing certificate.
`WIN_CODESIGN_PASS_SECRET_NAME`|Name of the AKV secret containing the
password for the Windows Authenticode signing certificate.
`WIN_GPG_PRIVATE_SECRET_NAME`|Name of the AKV secret containing the GPG
private key used to sign the Windows package.
`WIN_GPG_KEYGRIP_SECRET_NAME`|Name of the AKV secret containing the
keygrip for the GPG key used to sign the Windows package.
`WIN_GPG_PASS_SECRET_NAME`|Name of the AKV secret containing the
passphrase for the GPG private key used to sign the Windows package.
`APPLE_APPCERT_SECRET_NAME`|Name of the AKV secret containing the base64
encoded Apple Application Certificate for macOS.
`APPLE_APPCERT_PASS_SECRET_NAME`|Name of the AKV secret containing the
password for the Apple Application Certificate.
`APPLE_INSTCERT_SECRET_NAME`|Name of the AKV secret containing the
base64 encode Apple InstallationCertificate for macOS.
`APPLE_INSTCERT_PASS_SECRET_NAME`|Name of the AKV secret containing the
password for the Apple Installation Certificate.
`APPLE_TEAM_ID_SECRET_NAME`|Name of the AKV secret containing the Apple
Developer Team ID.
`APPLE_DEVELOPER_ID_SECRET_NAME`|Name of the AKV secret containing the
Apple Developer ID account email address for developer signing.
`APPLE_DEVELOPER_PASSWORD_SECRET_NAME`|Name of the AKV secret containing
the Apple Developer ID account password for developer signing.
`APPLE_APPSIGN_ID_SECRET_NAME`|Name of the AKV secret containing the
Apple Application Signing ID.
`APPLE_INSTSIGN_ID_SECRET_NAME`|Name of the AKV secret containing the
Apple Installation Signing ID.
`LINUX_GPG_PUBLIC_SECRET_NAME`|Name of the AKV secret containing the
base64 encoded GPG public key used for Debian package signing.
`LINUX_GPG_PRIVATE_SECRET_NAME`|Name of the AKV secret containing the
base64 encoded GPG private key used for Debian package signing.
`LINUX_GPG_PASS_SECRET_NAME`|Name of the AKV secret containing the
password for the GPG signing key used for the Debian package.
`LINUX_GPG_KEYGRIP_SECRET_NAME`|Name of the AKV secret containing the
keygrip of the GPG signing key used for the Debian package.
mjcheetham added a commit to mjcheetham/git that referenced this issue Apr 29, 2025
There are several issues that have been uncovered with the changes made
in git-for-windows#738. Let's fix them!

* Check out `akv-secret` action before it is used.
* Log in to Azure before accessing the Key Vault.
* Don't mask empty lines.
* Use a buffer to fix encoding issues when writing binary data.
* Correctly mask multi-line secret values.
* Add missing `require('path')` statement.
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