Skip to content

Commit

Permalink
fix(windows): Detect when winpty is present and if the current termin…
Browse files Browse the repository at this point in the history
…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>)
```
  • Loading branch information
thetrompf authored and joaolucasl committed Oct 27, 2017
1 parent 2b20df2 commit add0aed
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion bin/yarn
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ command_exists() {
}

if command_exists node; then
if [ "$YARN_FORCE_WINPTY" = 1 ]; then
if [ "$YARN_FORCE_WINPTY" = 1 ] || command_exists winpty && test -t 1; then
winpty node "$basedir/yarn.js" "$@"
else
exec node "$basedir/yarn.js" "$@"
Expand Down

0 comments on commit add0aed

Please sign in to comment.