code review 5847068: misc/cgo/stdio: make it work on Windows

misc/cgo/stdio: make it work on Windows and also test it
use a function to get stdout and stderr, instead of depending
on a specific libc implementation.
also make test/run.go replace \r\n by \n before comparing
output.

        Fixes issue 2121.
        Part of issue 1741.

[minux1, 2012-03-19 19:04:07]

Hello alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[minux1, 2012-03-19 19:08:13]

PTAL. Forget to add run.bat.

[brainman, 2012-03-20 00:21:56]

http://codereview.appspot.com/5847068/diff/5/misc/cgo/stdio/file.go
File misc/cgo/stdio/file.go (right):

http://codereview.appspot.com/5847068/diff/5/misc/cgo/stdio/file.go#newcode22
misc/cgo/stdio/file.go:22: // which cgo doesn't like, so write a wrapper
function to get them.
Do you know why "cgo doesn't like" that define? I am not stuck on it, but I
would be concerned that cgo does not provide direct access to "stdout" variable
for one reason or other.

http://codereview.appspot.com/5847068/diff/5/src/run.bat
File src/run.bat (right):

http://codereview.appspot.com/5847068/diff/5/src/run.bat#newcode41
src/run.bat:41: echo # ../misc/cgo/stdio
Should not you check for CGO_ENABLED, like run.bat does?

http://codereview.appspot.com/5847068/diff/5/src/run.bat#newcode42
src/run.bat:42: go run ../misc/cgo/stdio/hello.go >NUL
I think this is not good enough. If you look in misc/cgo/stdio/test.bash, it
does many other things. We should do full set of tests. Perhaps rewrite
test.bash into Go?

[minux1, 2012-03-20 17:05:08]

On 2012/03/20 00:21:56, brainman wrote:
> http://codereview.appspot.com/5847068/diff/5/misc/cgo/stdio/file.go#newcode22
> misc/cgo/stdio/file.go:22: // which cgo doesn't like, so write a wrapper
> function to get them.
> Do you know why "cgo doesn't like" that define? I am not stuck on it, but I
> would be concerned that cgo does not provide direct access to "stdout"
variable
> for one reason or other.
You can try this program:
package main
/*
#define A 1
#define ARR arr[A]
int arr[2] = {1, 2};
*/
import "C"
func main() {
	println(C.ARR)
}

The reason is that cgo now only support simple renames and simple expression
for macro expansion text. Support arbitrary macro definitions would be
difficult,
although I think we can make simple cases like arr[A] work, but they might need
to wait until after Go 1. Also, I think this code serve as documentation for
people
dealing with macros not that simple (for now).

Should I file an issue for this problem?

> http://codereview.appspot.com/5847068/diff/5/src/run.bat#newcode41
> src/run.bat:41: echo # ../misc/cgo/stdio
> Should not you check for CGO_ENABLED, like run.bat does?
Done. I originally thought cgo is always enabled on windows, but it turned
out it wrong.

> http://codereview.appspot.com/5847068/diff/5/src/run.bat#newcode42
> src/run.bat:42: go run ../misc/cgo/stdio/hello.go >NUL
> I think this is not good enough. If you look in misc/cgo/stdio/test.bash, it
> does many other things. We should do full set of tests. Perhaps rewrite
> test.bash into Go?
What we need is diff(1). But what I think we need is a shell. Write a
specialized
go program for each of the test cases is painful. Maybe we could implement
a simple shell and a diff(1) utility for this kind of things, and then we can
convert
all test cases to use it, and drop dependency for bash on tests.

[brainman, 2012-03-21 03:18:17]

On 2012/03/20 17:05:08, minux wrote:
> 
> ... Support arbitrary macro definitions would be
> difficult,
> although I think we can make simple cases like arr[A] work, but they might
need
> to wait until after Go 1. Also, I think this code serve as documentation for
> people
> dealing with macros not that simple (for now).
> 
> Should I file an issue for this problem?

I do not know to what extend cgo is expected to work. It just the fact that it
does not handle something as common as stdout, makes me uneasy. Lets wait for
Russ to decide what to do.

> What we need is diff(1). 

I disagree. We need to know if 2 files are the same or not. Byte by byte. diff
is much more.

> ... Write a
> specialized
> go program for each of the test cases is painful. ...

I disagree again. Small go program that executes bunch of action and then
compares output with "golden" (byte by byte) will do. I am happy to write one
yourself. But maybe Russ has other plans, lets wait for him.

> ... Maybe we could implement
> a simple shell and a diff(1) utility for this kind of things, and then we can
> convert
> all test cases to use it, and drop dependency for bash on tests.

Again, we could copy the above mentioned Go program. These would be very similar
and easy to change.

Alex

[minux1, 2012-03-21 07:49:24]

On Wed, Mar 21, 2012 at 11:18 AM, <alex.brainman@gmail.com> wrote:
>
> Again, we could copy the above mentioned Go program. These would be very
> similar and easy to change.
>
What about complex ones like /doc/articles/wiki/test.bash?

For simple tests like comparing output with the golden one, I think we can
reuse /test/run.go (with some trivial extension) here?

[brainman, 2012-03-21 23:25:55]

On 2012/03/21 07:49:24, minux wrote:
> >
> What about complex ones like /doc/articles/wiki/test.bash?

I would do the simple ones first to see how complicated it gets.

> For simple tests like comparing output with the golden one, I think we can
> reuse /test/run.go (with some trivial extension) here?

These are just two arrays of bytes. I would just compare them byte by byte. It
might be not as informative as seeing output of diff when comparison fails. I
might actually try to run diff, if comparison fails, and display diff output, if
diff execution succeeds.

Please note, that all this is just a speculation on my part. I have no say in
this matter.

Alex

[minux1, 2012-04-25 08:40:24]

any more thoughts on how to do tests on Windows?
should I just write a small diff like utility in misc/tools/ or I
write a test.go for each test that require output validation?

[rsc, 2012-05-03 21:49:37]

On Wed, Apr 25, 2012 at 4:40 AM,  <minux.ma@gmail.com> wrote:
> any more thoughts on how to do tests on Windows?
> should I just write a small diff like utility in misc/tools/ or I
> write a test.go for each test that require output validation?

You could use 'fc', which is Windows's cmp.
http://ss64.com/nt/fc.html

Russ

[minux1, 2012-05-11 18:05:13]

PTAL.

If this approach is ok, I will try to enable other cgo tests on Windows.

[brainman, 2012-05-14 04:27:15]

On 2012/05/11 18:05:13, minux wrote:
> PTAL.

LGTM. But please wait for rsc.

> 
> If this approach is ok, I will try to enable other cgo tests on Windows.

I have small problem with this approach. We are creating duplicate programs that
do same things on different OS. I believe, people will change one, but not the
other. I still prefer single program written in Go. But I do not want to push
for it unless others agree.

Alex

[r, 2012-05-14 18:46:20]

I share your discomfort.

[minux1, 2012-05-14 18:57:29]

On Tue, May 15, 2012 at 2:46 AM, <r@golang.org> wrote:

> I share your discomfort.
>
could we write a mini language in Go to do all these testing?
then we won't even depend on bash.

or, maybe there is a way to mechanically translate a small but useful
subset of bash script to dos batch file?

[rsc, 2012-05-17 01:41:51]

Can we reuse test/run.go?

[minux1, 2012-05-18 16:25:16]

On Thu, May 17, 2012 at 9:41 AM, Russ Cox <rsc@golang.org> wrote:

> Can we reuse test/run.go?
>
Sure. I've prepared two CLs to replace custom bash script under misc/cgo/*
and doc/progs
CL 6220049 and CL 6218048.

[minux1, 2012-08-10 03:59:07]

now that CL 6220049 has been submitted, please take another look at this CL.

note: run.bat includes changes from CL 6218048, as it will be submitted after
that one.

[minux1, 2012-08-10 04:45:26]

PTAL.

i have to make test/run.go replace \r\n by \n before comparing output,
otherwise, the stdio test will fail on Windows.

[brainman, 2012-08-10 05:29:02]

On 2012/08/10 04:45:26, minux wrote:
> PTAL.
> 
> i have to make test/run.go replace \r\n by \n before comparing output,
> otherwise, the stdio test will fail on Windows.

I am concerned about you making this change. These comparison rules are becoming
complicated.

Apart from that everything looks good to me.

Alex

[minux1, 2012-08-10 11:11:54]

On Fri, Aug 10, 2012 at 1:29 PM, <alex.brainman@gmail.com> wrote:

> On 2012/08/10 04:45:26, minux wrote:
>>
>> i have to make test/run.go replace \r\n by \n before comparing output,
>> otherwise, the stdio test will fail on Windows.
>
> I am concerned about you making this change. These comparison rules are
> becoming complicated.
>
yes, it is.

if we define cmpout as comparing textual output, then my change is
reasonable,
if we allow cmpout to compare arbitrary binary, then it's not; and if it's
the case,
perhaps we need a cmptext?

[brainman, 2012-08-12 08:22:26]

On 2012/08/10 11:11:54, minux wrote:
> 
> if we define cmpout as comparing textual output, then my change is
> reasonable,

Fair enough. But, these might be unsuitable for other tests. Lets wait for rsc
to voice in. As far as your other changes are concerned, they LGTM.

Alex

[remyoudompheng, 2012-08-16 06:52:53]

Can we modify the test so that it opens stdout in binary mode on Windows?

[brainman, 2012-08-16 06:55:50]

On 2012/08/16 06:52:53, remyoudompheng wrote:
> Can we modify the test so that it opens stdout in binary mode on Windows?

I do not know much about C library involved.

On the other hand, I am not sure, if it is OK for us to change the test. Cgo is
suppose to work transparently everywhere, isn't it?

Alex

[minux1, 2012-08-16 07:10:36]

On Thu, Aug 16, 2012 at 2:52 PM, <remyoudompheng@gmail.com> wrote:

> Can we modify the test so that it opens stdout in binary mode on
> Windows?
>
i'd rather not. because it has two purposes.
it's a test for using stdio, and it's also a cgo example referenced
by tip.golang.org/cmd/cgo (and I think the latter purpose is more important,
the test merely serves to make sure we don't break our own examples)

we already have to use wrapper functions to get stdout/stderr to workaround
cmd/cgo limitations, i don't want to complicate it just to satisfy our test
program.

[minux1, 2012-09-03 16:28:13]

ping.

i need a final say on the test/run.go change.

[rsc, 2012-09-10 15:57:28]

LGTM

[minux1, 2012-09-19 16:28:25]

*** Submitted as http://code.google.com/p/go/source/detail?r=80ab25ffe4ca ***

misc/cgo/stdio: make it work on Windows and also test it
use a function to get stdout and stderr, instead of depending
on a specific libc implementation.
also make test/run.go replace \r\n by \n before comparing
output.

        Fixes issue 2121.
        Part of issue 1741.

R=alex.brainman, rsc, r, remyoudompheng
CC=golang-dev
http://codereview.appspot.com/5847068