sass
Ensure onStdinClose only completes for non-tty streams
First, thank you so much for all the work to get #1411 merged! I realized only too late that there was a small typo in my initial version that has led to a bit of a gnarly bug. 😳
The initial stdin commit 144cd35 inverted the TTY conditional
b/w the io/node and io/vm implementations of onStdinClose.
The node implementation incorrectly checked for the presence
of TTY to complete the stream, while the vm implementation
correctly checked for its absence. The commit that landed
upstream c7ab426 normalized the incorrect behaviour, which
means that sass was still not closing when stdin was closed,
unless stdin was a TTY.
Unfortunately that created a "worst of both worlds" situation because programs that start sass and then close unexpectedly will still leave zombie sass processes running in the background, and wrapper scripts designed to mitigate this exact problem will stop working because moving the process to the background now incorrectly causes the job to stop.
This change ensures we only complete the CancelableOperation onStdinClose for non-tty standard input streams.
Thanks! Are you able to figure out why the Windows tests are failing? It seems unlikely to be a flake.
@nex3 I can test Windows in a VM which is not ideal but it works :) I agree there's an issue and I will try to figure it out– right now running dart test from PowerShell hangs forever on a --watch test.
@mcrumm If you don't have time to debug this for now, maybe we should roll back #1411 for now?
@nex3 Yes I think that's likely the best course. Other responsibilities have unfortunately taken precedence for the moment, but I would very much like to get back to this as time permits :)