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.

Sign up for GitHub

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

Jump to bottom

Support interactive commands in standard MINGW as well #2243

Merged
merged 3 commits into from
Dec 15, 2016
Merged

Support interactive commands in standard MINGW as well #2243

merged 3 commits into from
Dec 15, 2016

Conversation

thetrompf
Copy link
Contributor

@thetrompf thetrompf commented Dec 13, 2016 •
edited
Loading

Summary

Test for both MSYS* and MINGW* when determine to wrap
the node process in winpty.

I made this previous pull request #2230 for supporting interactive commands in
Git bash for windows.

It turns out that I only made it work for some really old git bash.
Along the way Git bash has changed its default terminal emulator, which means it
went from using a terminal that, resolved to MSYS* when invoking the uname command,
to a newer one that resolves MINGW*

This fix make it work with newer configurations and keeping support to the old version.
Github: #743

Test plan

Before this change

bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ uname
MSYS_NT-10.0
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ ./bin/yarn init
yarn init v0.19.0-0
question name (yarn):
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ uname
MINGW64_NT-10.0
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ ./bin/yarn init
yarn init v0.19.0-0
error An unexpected error occurred: "Can't answer a question unless a user TTY".
info If you think this is a bug, please open a bug report with the information provided in "C:\\Users\\bkc\\workspace\\yarn-test\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/init for documentation about this command.

After this change:

bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ uname
MSYS_NT-10.0
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ ./bin/yarn init
yarn init v0.19.0-0
question name (yarn):
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ uname
MINGW64_NT-10.0
bkc@DESKTOP-NNSLT74 ~/workspace/yarn-test (master) $ ./bin/yarn init
yarn init v0.19.0-0
question name (yarn):

@thetrompf
Support interactive commands in standard MINGW as well
cc1d7bf 
Previous fix only supported some old configuration of Git bash
provided by Git for windows, newer Git bash use standard MINGW
which uses a different `uname` but the fix is ultimately the same

Github: yarnpkg#743
@Daniel15 Daniel15 merged commit 35ca860 into yarnpkg:master Dec 15, 2016
@Daniel15
Copy link
Member

Daniel15 commented Dec 15, 2016 •
edited
Loading

Thanks! ❤️

@jmathew jmathew mentioned this pull request Sep 18, 2017
Open
@thetrompf thetrompf mentioned this pull request Sep 28, 2017
Merged
BYK pushed a commit that referenced this pull request Sep 29, 2017
@thetrompf @BYK
fix(windows): Detect when winpty is present and if the current termin…
75bc450 
…al is a TTY (#4577)

**Summary**

This addresses some of the windows issues regarding running yarn in gitbash and friends envrionment.
with this fix I keep the behavior introduced 5 months ago in #3245, but try to do a better job detecting when to use winpty out of the box, in order make `piping` of output work with yarn.

Before this fix:
```shell
$ yarn --version
1.1.0
$ yarn --version | cat
1.1.0
$ yarn init
yarn init v1.1.0
error An unexpected error occurred: "Can't answer a question unless a user TTY".
info If you think this is a bug, please open a bug report with the information provided in "D:\\workspace\\yarn\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/init for documentation about this command.
```

Piping works for simple commands, but interactive commands only work with an environment set to something. This prohibits scripts/tools around yarn that uses pipe, which is quite common to do in an unix like environment, but theses tools cannot work in windows' unix like environment.

WinPTY seems to be the savior here, but we need to only run yarn through winpty when a tty actually needs to be allocated. 

Previous attempts to solve this problem like: 
- #2230
- #2243

Did not address the use cases of piping, so they essentially broke that behavior.
Then #3245 fixed that, but now you have to use `YARN_FORCE_WINPTY=1` environment variable in order for `yarn init` and `yarn upgrade-interactive` to work and that's alright, but if you export that variable then piping is broken yet again because the variable will also be set in the piped command, and we haven't solved any problem.

I suggest we keep the environment variable behavior but open up for better detection when to use winpty out of the box.

This fix detects if the winpty binary is in path, and only use it if stdin is in fact a TTY: `test -t 1`. 

**Test plan**

The output of running:
- `yarn init`
- `yarn upgrade-interactive`
- `yarn --version | cat`

Without having the `YARN_FORCE_WINPTY=1` environment variable set.

```shell
$ ./bin/yarn init
yarn init v1.1.0
question name (yarn):

$ ./bin/yarn upgrade-interactive
yarn upgrade-interactive v1.1.0
info Color legend :
 "<red>"    : Major Update backward-incompatible updates
 "<yellow>" : Minor Update backward-compatible features
 "<green>"  : Patch Update backward-compatible bug fixes
? Choose which packages to update. (Press <space> to select, <a> to toggle all, <i> to inverse selection)
 devDependencies
   name                         range                      from               to             url
>( ) babel-core                   ^6.24.1                    6.24.1          ❯  6.26.0         https://babeljs.io/
 ( ) babylon                      ^6.5.0                     6.17.1          ❯  6.18.0         https://babeljs.io/
 ( ) eslint                       ^4.3.0                     4.3.0           ❯  4.7.2          http://eslint.org
 ( ) eslint-config-fb-strict      ^20.1.0-delta.3            20.1.0-delta.3  ❯  20.1.0-echo.1  https://github.com/facebook/jest#readme
 ( ) eslint-plugin-babel          ^4.0.0                     4.1.1           ❯  4.1.2          https://github.com/babel/eslint-plugin-babel#readme
 ( ) eslint-plugin-flowtype       ^2.35.0                    2.35.0          ❯  2.36.0         https://github.com/gajus/eslint-plugin-flowtype#readme
 ( ) eslint-plugin-jasmine        ^2.6.2                     2.6.2           ❯  2.8.4          https://github.com/tlvince/eslint-plugin-jasmine
 ( ) eslint-plugin-prettier       ^2.1.2                     2.1.2           ❯  2.3.1          https://github.com/prettier/eslint-plugin-prettier#readme
 ( ) eslint-plugin-react          ^7.1.0                     7.1.0           ❯  7.4.0          https://github.com/yannickcr/eslint-plugin-react
 ( ) eslint-plugin-yarn-internal  file:scripts/eslint-rules  0.0.0           ❯  exotic         file:scripts/eslint-rules
 ( ) gulp-sourcemaps              ^2.2.0                     2.6.0           ❯  2.6.1          http://github.com/gulp-sourcemaps/gulp-sourcemaps
 ( ) prettier                     ^1.5.2                     1.5.2           ❯  1.7.2          https://prettier.io
 ( ) webpack                      ^2.1.0-beta.25             2.6.0           ❯  2.7.0          https://github.com/webpack/webpack

 dependencies
   name                         range                      from               to             url
 ( ) babel-runtime                ^6.0.0                     6.23.0          ❯  6.26.0         https://github.com/babel/babel/tree/master/packages/babel-runtime
 ( ) commander                    ^2.9.0                     2.9.0           ❯  2.11.0         https://github.com/tj/commander.js#readme
 ( ) debug                        ^2.2.0                     2.6.8           ❯  2.6.9          https://github.com/visionmedia/debug#readme
 ( ) gunzip-maybe                 ^1.4.0                     1.4.0           ❯  1.4.1          https://github.com/mafintosh/gunzip-maybe
 ( ) inquirer                     ^3.0.1                     3.0.6           ❯  3.3.0          https://github.com/SBoudrias/Inquirer.js#readme
 ( ) node-emoji                   ^1.6.1                     1.6.1           ❯  1.8.1          https://github.com/omnidan/node-emoji#readme
 ( ) request                      ^2.81.0                    2.81.0          ❯  2.83.0         https://github.com/request/request#readme
 ( ) rimraf                       ^2.5.0                     2.6.1           ❯  2.6.2          https://github.com/isaacs/rimraf#readme
 ( ) semver                       ^5.1.0                     5.3.0           ❯  5.4.1          https://github.com/npm/node-semver#readme
 ( ) tar-fs                       ^1.15.1                    1.15.2          ❯  1.15.3         https://github.com/mafintosh/tar-fs
 ( ) uuid                         ^3.0.1                     3.0.1           ❯  3.1.0          https://github.com/kelektiv/node-uuid#readme

$ ./bin/yarn --version | cat
1.1.0
$
```

And importantly when running the interactive commands through a pipe, it will correctly fail by saying you not are running the interactive commands in a TTY:

```shell
 $ ./bin/yarn init | cat
yarn init v1.1.0
error An unexpected error occurred: "Can't answer a question unless a user TTY".
info If you think this is a bug, please open a bug report with the information provided in "D:\\workspace\\yarn\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/init for documentation about this command.

 $ ./bin/yarn upgrade-interactive | cat
yarn upgrade-interactive v1.1.0
info Color legend :
 "<red>"    : Major Update backward-incompatible updates
 "<yellow>" : Minor Update backward-compatible features
 "<green>"  : Patch Update backward-compatible bug fixes
Done in 1.43s.
Error: Can't answer a question unless a user TTY
    at D:\workspace\yarn\lib\reporters\console\console-reporter.js:487:31
    at Generator.next (<anonymous>)
    at step (D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:17:30)
    at D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:35:14
    at Promise (<anonymous>)
    at F (D:\workspace\yarn\node_modules\core-js\library\modules\_export.js:35:28)
    at D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:14:12
    at ConsoleReporter.prompt (D:\workspace\yarn\lib\reporters\console\console-reporter.js:518:7)
    at Object.<anonymous> (D:\workspace\yarn\lib\cli\commands\upgrade-interactive.js:116:38)
    at Generator.next (<anonymous>)
```
@GulajavaMinistudio GulajavaMinistudio mentioned this pull request Sep 30, 2017
Merged
joaolucasl pushed a commit to joaolucasl/yarn that referenced this pull request Oct 27, 2017
@thetrompf @joaolucasl
fix(windows): Detect when winpty is present and if the current termin…
add0aed 
…al is a TTY (yarnpkg#4577)

**Summary**

This addresses some of the windows issues regarding running yarn in gitbash and friends envrionment.
with this fix I keep the behavior introduced 5 months ago in yarnpkg#3245, but try to do a better job detecting when to use winpty out of the box, in order make `piping` of output work with yarn.

Before this fix:
```shell
$ yarn --version
1.1.0
$ yarn --version | cat
1.1.0
$ yarn init
yarn init v1.1.0
error An unexpected error occurred: "Can't answer a question unless a user TTY".
info If you think this is a bug, please open a bug report with the information provided in "D:\\workspace\\yarn\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/init for documentation about this command.
```

Piping works for simple commands, but interactive commands only work with an environment set to something. This prohibits scripts/tools around yarn that uses pipe, which is quite common to do in an unix like environment, but theses tools cannot work in windows' unix like environment.

WinPTY seems to be the savior here, but we need to only run yarn through winpty when a tty actually needs to be allocated. 

Previous attempts to solve this problem like: 
- yarnpkg#2230
- yarnpkg#2243

Did not address the use cases of piping, so they essentially broke that behavior.
Then yarnpkg#3245 fixed that, but now you have to use `YARN_FORCE_WINPTY=1` environment variable in order for `yarn init` and `yarn upgrade-interactive` to work and that's alright, but if you export that variable then piping is broken yet again because the variable will also be set in the piped command, and we haven't solved any problem.

I suggest we keep the environment variable behavior but open up for better detection when to use winpty out of the box.

This fix detects if the winpty binary is in path, and only use it if stdin is in fact a TTY: `test -t 1`. 

**Test plan**

The output of running:
- `yarn init`
- `yarn upgrade-interactive`
- `yarn --version | cat`

Without having the `YARN_FORCE_WINPTY=1` environment variable set.

```shell
$ ./bin/yarn init
yarn init v1.1.0
question name (yarn):

$ ./bin/yarn upgrade-interactive
yarn upgrade-interactive v1.1.0
info Color legend :
 "<red>"    : Major Update backward-incompatible updates
 "<yellow>" : Minor Update backward-compatible features
 "<green>"  : Patch Update backward-compatible bug fixes
? Choose which packages to update. (Press <space> to select, <a> to toggle all, <i> to inverse selection)
 devDependencies
   name                         range                      from               to             url
>( ) babel-core                   ^6.24.1                    6.24.1          ❯  6.26.0         https://babeljs.io/
 ( ) babylon                      ^6.5.0                     6.17.1          ❯  6.18.0         https://babeljs.io/
 ( ) eslint                       ^4.3.0                     4.3.0           ❯  4.7.2          http://eslint.org
 ( ) eslint-config-fb-strict      ^20.1.0-delta.3            20.1.0-delta.3  ❯  20.1.0-echo.1  https://github.com/facebook/jest#readme
 ( ) eslint-plugin-babel          ^4.0.0                     4.1.1           ❯  4.1.2          https://github.com/babel/eslint-plugin-babel#readme
 ( ) eslint-plugin-flowtype       ^2.35.0                    2.35.0          ❯  2.36.0         https://github.com/gajus/eslint-plugin-flowtype#readme
 ( ) eslint-plugin-jasmine        ^2.6.2                     2.6.2           ❯  2.8.4          https://github.com/tlvince/eslint-plugin-jasmine
 ( ) eslint-plugin-prettier       ^2.1.2                     2.1.2           ❯  2.3.1          https://github.com/prettier/eslint-plugin-prettier#readme
 ( ) eslint-plugin-react          ^7.1.0                     7.1.0           ❯  7.4.0          https://github.com/yannickcr/eslint-plugin-react
 ( ) eslint-plugin-yarn-internal  file:scripts/eslint-rules  0.0.0           ❯  exotic         file:scripts/eslint-rules
 ( ) gulp-sourcemaps              ^2.2.0                     2.6.0           ❯  2.6.1          http://github.com/gulp-sourcemaps/gulp-sourcemaps
 ( ) prettier                     ^1.5.2                     1.5.2           ❯  1.7.2          https://prettier.io
 ( ) webpack                      ^2.1.0-beta.25             2.6.0           ❯  2.7.0          https://github.com/webpack/webpack

 dependencies
   name                         range                      from               to             url
 ( ) babel-runtime                ^6.0.0                     6.23.0          ❯  6.26.0         https://github.com/babel/babel/tree/master/packages/babel-runtime
 ( ) commander                    ^2.9.0                     2.9.0           ❯  2.11.0         https://github.com/tj/commander.js#readme
 ( ) debug                        ^2.2.0                     2.6.8           ❯  2.6.9          https://github.com/visionmedia/debug#readme
 ( ) gunzip-maybe                 ^1.4.0                     1.4.0           ❯  1.4.1          https://github.com/mafintosh/gunzip-maybe
 ( ) inquirer                     ^3.0.1                     3.0.6           ❯  3.3.0          https://github.com/SBoudrias/Inquirer.js#readme
 ( ) node-emoji                   ^1.6.1                     1.6.1           ❯  1.8.1          https://github.com/omnidan/node-emoji#readme
 ( ) request                      ^2.81.0                    2.81.0          ❯  2.83.0         https://github.com/request/request#readme
 ( ) rimraf                       ^2.5.0                     2.6.1           ❯  2.6.2          https://github.com/isaacs/rimraf#readme
 ( ) semver                       ^5.1.0                     5.3.0           ❯  5.4.1          https://github.com/npm/node-semver#readme
 ( ) tar-fs                       ^1.15.1                    1.15.2          ❯  1.15.3         https://github.com/mafintosh/tar-fs
 ( ) uuid                         ^3.0.1                     3.0.1           ❯  3.1.0          https://github.com/kelektiv/node-uuid#readme

$ ./bin/yarn --version | cat
1.1.0
$
```

And importantly when running the interactive commands through a pipe, it will correctly fail by saying you not are running the interactive commands in a TTY:

```shell
 $ ./bin/yarn init | cat
yarn init v1.1.0
error An unexpected error occurred: "Can't answer a question unless a user TTY".
info If you think this is a bug, please open a bug report with the information provided in "D:\\workspace\\yarn\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/init for documentation about this command.

 $ ./bin/yarn upgrade-interactive | cat
yarn upgrade-interactive v1.1.0
info Color legend :
 "<red>"    : Major Update backward-incompatible updates
 "<yellow>" : Minor Update backward-compatible features
 "<green>"  : Patch Update backward-compatible bug fixes
Done in 1.43s.
Error: Can't answer a question unless a user TTY
    at D:\workspace\yarn\lib\reporters\console\console-reporter.js:487:31
    at Generator.next (<anonymous>)
    at step (D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:17:30)
    at D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:35:14
    at Promise (<anonymous>)
    at F (D:\workspace\yarn\node_modules\core-js\library\modules\_export.js:35:28)
    at D:\workspace\yarn\node_modules\babel-runtime\helpers\asyncToGenerator.js:14:12
    at ConsoleReporter.prompt (D:\workspace\yarn\lib\reporters\console\console-reporter.js:518:7)
    at Object.<anonymous> (D:\workspace\yarn\lib\cli\commands\upgrade-interactive.js:116:38)
    at Generator.next (<anonymous>)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Daniel15 Daniel15 Daniel15 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thetrompf @Daniel15