Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(294)

Issue 18535002: Fix stdin_sync_test on Windows. (Closed)

Created:
7 years, 5 months ago by Anders Johnsen
Modified:
7 years, 5 months ago
Reviewers:
Bill Hesse
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fix stdin_sync_test on Windows. BUG= Committed: https://code.google.com/p/dart/source/detail?r=24686

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -2 lines) Patch
M tests/standalone/io/stdin_sync_script.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/io/stdin_sync_test.dart View 1 chunk +6 lines, -1 line 1 comment Download

Messages

Total messages: 4 (0 generated)
Anders Johnsen
7 years, 5 months ago (2013-07-02 13:57:33 UTC) #1
Anders Johnsen
TBR
7 years, 5 months ago (2013-07-02 13:57:46 UTC) #2
Anders Johnsen
Committed patchset #1 manually as r24686 (presubmit successful).
7 years, 5 months ago (2013-07-02 13:58:14 UTC) #3
Bill Hesse
7 years, 5 months ago (2013-07-02 14:49:43 UTC) #4
Message was sent while issue was closed.
LGTM, with a bug filed.

https://codereview.chromium.org/18535002/diff/1/tests/standalone/io/stdin_syn...
File tests/standalone/io/stdin_sync_test.dart (right):

https://codereview.chromium.org/18535002/diff/1/tests/standalone/io/stdin_syn...
tests/standalone/io/stdin_sync_test.dart:44: test("hej\r\r\nhej\r\nhej\r",
['hej\r', 'hej', 'hej\r']);
This indicates an error in the newline handling with subprocesses.

I'm pretty sure that the standard for Windows is that all file handles and
descriptors for stdin, stdout, and stderr are in text mode, which means that
"\r\n" on the file/pipe/stream are converted to "\n" before being exposed to C
code, by stdlib.  So our Dart code, and our C++ implementation of Dart IO, is
only seeing \n.

I think we need to maintain this, so that interoperability with other languages
works with Process.
If Dart has a Python or C subprocess, then \n and \r\n sent from Dart should be
received as \n and \r\n by Python or C.  If Python or C start a Dart subprocess,
then \n and \r\n should also be communicated correctly.  Similarly when Dart has
its stdin and stdout redirected to a file.

It may be that the only problem is the Handle/file descriptors created in Dart
when Process.start is called may not be in text mode.  That would explain the
problem here.

We should create a bug, and a series of tests.

Powered by Google App Engine
This is Rietveld 408576698