code review 9249043: misc/cgo/test: test for issue 5337.

misc/cgo/test: test for issue 5337.
Test for CL 9226043.

[minux1, 2013-05-06 23:49:30]

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

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

[minux1, 2013-05-06 23:53:04]

this test doesn't fail on darwin even without CL 9226043,
i'm not sure why.

[dave_cheney.net, 2013-05-06 23:53:32]

On 2013/05/06 23:49:30, minux wrote:
> Hello mailto:golang-dev@googlegroups.com (cc:
mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

Very nice. This segfaulted as expected, then 3aed68ee68dc fixed it.

I am worried that this might fail on arm, as most things do the first time, but
am unable to test this at the moment.

[minux1, 2013-05-07 00:26:42]

On Tue, May 7, 2013 at 7:53 AM, <dave@cheney.net> wrote:

> I am worried that this might fail on arm, as most things do the first
> time, but am unable to test this at the moment.
>
haha, you're right.

i prepared https://codereview.appspot.com/9251043 for that,
but unfortunately, i think this means this CL couldn't go in before Go 1.1
unless CL 9251043 is accepted first.

[dave_cheney.net, 2013-05-07 00:29:25]

> haha, you're right.

I'm crying on the inside.

> i prepared https://codereview.appspot.com/9251043 for that,
> but unfortunately, i think this means this CL couldn't go in before Go 1.1
> unless CL 9251043 is accepted first.

I think it is fine to leave making this test work on arm til after 1.1.

[minux1, 2013-05-07 00:34:46]

On Tue, May 7, 2013 at 8:29 AM, Dave Cheney <dave@cheney.net> wrote:

> > haha, you're right.
> I'm crying on the inside.

sorry, my bad. i forgot about this issue after i commented on it 8 months
ago:
https://codereview.appspot.com/6498057#msg11

[iant, 2013-05-07 05:43:56]

On Mon, May 6, 2013 at 5:26 PM, minux <minux.ma@gmail.com> wrote:
>
> i prepared https://codereview.appspot.com/9251043 for that,
> but unfortunately, i think this means this CL couldn't go in before Go 1.1
> unless CL 9251043 is accepted first.

I would prefer to leave this out of Go 1.1 anyhow.  At this point I
think that a change that only adds a test is more likely to hurt the
release than to help it.  Let's commit it after the release.

Ian

[dave_cheney.net, 2013-05-07 05:46:41]

I agree. I believe we have a fix committed and everyone is going to be
super careful when working in that area of the code, so we can survive
without this test as it will probably not be perfectly stable on first
commit.

On Tue, May 7, 2013 at 3:43 PM, Ian Lance Taylor <iant@golang.org> wrote:
> On Mon, May 6, 2013 at 5:26 PM, minux <minux.ma@gmail.com> wrote:
>>
>> i prepared https://codereview.appspot.com/9251043 for that,
>> but unfortunately, i think this means this CL couldn't go in before Go 1.1
>> unless CL 9251043 is accepted first.
>
> I would prefer to leave this out of Go 1.1 anyhow.  At this point I
> think that a change that only adds a test is more likely to hurt the
> release than to help it.  Let's commit it after the release.
>
> Ian

[minux1, 2013-05-07 05:58:25]

On Tue, May 7, 2013 at 1:43 PM, Ian Lance Taylor <iant@golang.org> wrote:

> On Mon, May 6, 2013 at 5:26 PM, minux <minux.ma@gmail.com> wrote:
> >
> > i prepared https://codereview.appspot.com/9251043 for that,
> > but unfortunately, i think this means this CL couldn't go in before Go
> 1.1
> > unless CL 9251043 is accepted first.
>
> I would prefer to leave this out of Go 1.1 anyhow.  At this point I
> think that a change that only adds a test is more likely to hurt the
> release than to help it.  Let's commit it after the release.
>
now that go1.1 has branched, can we start reviewing these changes?

[iant, 2013-05-07 06:02:27]

On Mon, May 6, 2013 at 10:58 PM, minux <minux.ma@gmail.com> wrote:
>
> On Tue, May 7, 2013 at 1:43 PM, Ian Lance Taylor <iant@golang.org> wrote:
>>
>> On Mon, May 6, 2013 at 5:26 PM, minux <minux.ma@gmail.com> wrote:
>> >
>> > i prepared https://codereview.appspot.com/9251043 for that,
>> > but unfortunately, i think this means this CL couldn't go in before Go
>> > 1.1
>> > unless CL 9251043 is accepted first.
>>
>> I would prefer to leave this out of Go 1.1 anyhow.  At this point I
>> think that a change that only adds a test is more likely to hurt the
>> release than to help it.  Let's commit it after the release.
>
> now that go1.1 has branched, can we start reviewing these changes?

I would vote for giving it a few days.  We still have the option of
rebranching if it seems necessary.

Ian

[minux1, 2013-05-15 04:02:27]

gentle ping.

[bradfitz, 2013-05-15 04:12:33]

LGTM

[minux1, 2013-05-15 04:33:48]

*** Submitted as https://code.google.com/p/go/source/detail?r=3fc2a3f9584a ***

misc/cgo/test: test for issue 5337.
Test for CL 9226043.

R=golang-dev, dave, iant, bradfitz
CC=golang-dev
https://codereview.appspot.com/9249043