avoid call to stdin on Windows

[MiguelAlho]

Summary

As seen in other repos (namely https://github.com/evantahler/actionhero/pull/360) IISNode has no notion of stdin, so accessing ´stdin` in base-reporter throws an exception.

While developing an extension for Visual Studio's Task Runner Explorer component, that would allow devs to call yarn commands (and maybe run scripts) from this window. This is similar to existing extensions for other task-enabled engines like gulp, grunt, and npm. These tasks can also be binded to some of the IDEs events such as "project open". This is great for my current use case - when a yarn enabled project is open, run yarn install automatically.

Unfortunately, when executing yarn through the extension (which is really calling cmd.exe /C yarn install) the following error appears:

C:\repo> cmd.exe /c yarn install
C:\Users\AlhoM\AppData\Roaming\npm\node_modules\yarn\bin\yarn.js:48
      throw err;
      ^
Error: Implement me. Unknown stdin file type!
    at process.getStdin [as stdin] (internal/process/stdio.js:82:15)
    at ConsoleReporter.BaseReporter (C:\Users\AlhoM\AppData\Roaming\npm\node_modules\yarn\lib\reporters\base-reporter.js:60:37)
    at ConsoleReporter (C:\Users\AlhoM\AppData\Roaming\npm\node_modules\yarn\lib\reporters\console\console-reporter.js:58:5)
    at Object.<anonymous> (C:\Users\AlhoM\AppData\Roaming\npm\node_modules\yarn\lib\cli\index.js:192:18)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
Process terminated with code 1.

Hacking around a bit and using the info in the link above, the repose to the command returns the desired output:

 C:\repo> cmd.exe /c yarn install
yarn install v0.15.1
warning app@1.0.0: No license field
success Already up-to-date.
Done in 0.13s.
Process terminated with code 0..

Test plan

A clean clone did not produce a valid code run (I checked on discord and the info available was that some tests where not running.

With visual studio installed, the extension I created can be installed and used to test the change.

I don't have experience with testing in node, so I , unfortunately, have no idea where to start, especially for something system / platform specific...

[Daniel15]

Hmm, I wonder why the reporter even needs stdin? I wonder if it could lazily grab stdin as needed rather than touching it in the constructor.

While developing an extension for Visual Studio's Task Runner Explorer component, that would allow devs to call yarn commands (and maybe run scripts) from this window.

This is an awesome idea! 😄

[MiguelAlho]

I've uploaded the extension where the behaviour is visible to the Visual Studio Gallery; code for the extension is at https://github.com/MiguelAlho/YarnTaskRunner

[bestander]

@MiguelAlho can you rebase please.
@Daniel15, should we merge this or investigate stdin usages?

[MiguelAlho]

@Daniel15 @bestander I've been unable to get this working correctly - or better yet - unable to make the tests pass. There are a pair of tests that timeout, which might be an indication that some point is waiting for input. Unfortunately, i haven't learned enough yet to actually debug it, and integrating it with broken tests is undesirable.

All I was trying to do here is "stub" an stdin implementation, in a Null-Object pattern approach. The implementation though is for some reason incorrect.

Also, I made a bad mistake of working on Master on my fork so the commit history is all mixed up. Would it make sense to close this and reopen based on a clean master and a feature branch?

[bestander]

@MiguelAlho yeah, go ahead, let's close and open a new one.
Just link this PR from the new one

[nelsonmorais]

Hi,

@bestander Is there a new PR with a fix to the reported problem?
I still see the reported issue while running yarn version 0.19.1, so using the Yarn Task Runner in Visual Studio 2015 still fails after installing the version 0.19.1 of yarn from the yarn web site.
I see that @MiguelAlho has reported a Hack to have it running at https://github.com/MiguelAlho/YarnTaskRunner but it seems that the mentioned hack is not part of the 0.19.1 version of yarn.
Is this issue being addressed in some way?

[MiguelAlho]

@nelsonmorais I never got back to reimplementing the "fix". Basically, every attempt I made to fix it on the codebase made tests fail, (mostly failures with reporters) and I never figured out how to get around them. I'm a bit limited in developing with node, so I blocked.

Unfortunately I never got the time to go back to it. Maybe it's easier with the most recent versions, or maybe the same failures will appear - i haven't tried yet. The only thing I tried to do was create a substitute for stdin when on windows:

`
/* @flow */

const Readable = require('stream').Readable;

//StandinStdin serves as a stub for stdin, to be used
//in environments where ther is no stdin, such as IISNode
export class StandinStdIn extends Readable {
constructor(options: any) {
super(options);
}

_read(size: number) {
this.push(null);
}
}
`
and in base-reporter.js try to define stdin based on env conditions:

//The IISNode process, in Windows, lacks a stdin if (process.platform == 'win32' || process.env.IISNODE_VERSION) { this.stdin = new StandinStdIn(); } else { this.stdin = opts.stdin || process.stdin; }
I can't back to this now, so if any one wants to give it a try, (or if it still makes sense) please feel free to .

[nelsonmorais]

Hi @MiguelAlho,

Thanks for the quick response.
I've also posted an issue VS Extention Git repo to see if this is something than can be fixed at that level: madskristensen/NpmTaskRunner#46

[scottaddie]

@Daniel15 I'm joining this thread a bit late, but this issue is still preventing the Visual Studio extension from working. You questioned why stdin is needed by the reporter at all. Could this be fixed by simply removing stdin references from base-reporter? I suppose we'd also have to figure out what to do about the couple of stdin references in console-reporter.

[MiguelAlho]

@scottaddie thanks for taking care of this!

[scottaddie]

@MiguelAlho Thank you for all the time you spent on this. That definitely expedited the process for me. Now to wait for an official Yarn release to test this out.

[nelsonmorais]

@MiguelAlho @Daniel15 Thank you both for your effort.