code review 70300043: net: remove the dreg of solaris merge

net: remove the dreg of solaris merge

Also enables netgo test on solaris.

[mikio, 2014-03-01 01:51:44]

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

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

[dave_cheney.net, 2014-03-01 03:48:58]

I'm not sure if this does anything today, solaris/amd64 doesn't support cgo.


On Sat, Mar 1, 2014 at 12:51 PM, <mikioh.mikioh@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net: enable cgo and netgo tests on solaris
>
> Please review this at https://codereview.appspot.com/70300043/
>
> Affected files (+2, -2 lines):
>   M src/pkg/net/cgo_unix_test.go
>   M src/pkg/net/netgo_unix_test.go
>
>
> Index: src/pkg/net/cgo_unix_test.go
> ===================================================================
> --- a/src/pkg/net/cgo_unix_test.go
> +++ b/src/pkg/net/cgo_unix_test.go
> @@ -3,7 +3,7 @@
>  // license that can be found in the LICENSE file.
>
>  // +build cgo,!netgo
> -// +build darwin dragonfly freebsd linux netbsd openbsd
> +// +build darwin dragonfly freebsd linux netbsd openbsd solaris
>
>  package net
>
> Index: src/pkg/net/netgo_unix_test.go
> ===================================================================
> --- a/src/pkg/net/netgo_unix_test.go
> +++ b/src/pkg/net/netgo_unix_test.go
> @@ -3,7 +3,7 @@
>  // license that can be found in the LICENSE file.
>
>  // +build !cgo netgo
> -// +build darwin dragonfly freebsd linux netbsd openbsd
> +// +build darwin dragonfly freebsd linux netbsd openbsd solaris
>
>  package net
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[mikio, 2014-03-01 03:54:37]

On Sat, Mar 1, 2014 at 12:48 PM, Dave Cheney <dave@cheney.net> wrote:

> I'm not sure if this does anything today, solaris/amd64 doesn't support cgo.

??? you guys added solaris tag to cgo_xxx.go files, right?

[dave_cheney.net, 2014-03-01 03:59:59]

I know Aram is working on cgo support, but right now, cgo is not enabled on
solaris builds. http://golang.org/src/pkg/go/build/build.go#L260


On Sat, Mar 1, 2014 at 2:54 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:

> On Sat, Mar 1, 2014 at 12:48 PM, Dave Cheney <dave@cheney.net> wrote:
>
> > I'm not sure if this does anything today, solaris/amd64 doesn't support
> cgo.
>
> ??? you guys added solaris tag to cgo_xxx.go files, right?
>

[mikio, 2014-03-01 04:29:25]

On Sat, Mar 1, 2014 at 12:59 PM, Dave Cheney <dave@cheney.net> wrote:

> I know Aram is working on cgo support, but right now, cgo is not enabled on
> solaris builds. http://golang.org/src/pkg/go/build/build.go#L260

??? freebsd/arm doesn't support cgo too, but we see freebsd tag in
cgo_xxx files because we have cgo or netgo build toggle. What's your
point?

[aram, 2014-03-01 12:14:33]

I don't understand the purpose of this patch. It doesn't have any effect. Please
note that even when I implement cgo I will *not* use cgo for networking, instead
I will do what Windows does.

[mikio, 2014-03-02 00:02:03]

On Sat, Mar 1, 2014 at 9:14 PM,  <aram@mgk.ro> wrote:

> I don't understand the purpose of this patch. It doesn't have any
> effect.

Both cgo_unix_test.go and netgo_unix_test.go test three situations
where we can see; w/ cgo by CGO_ENABLED=1, w/o cgo (automatically use
the last resort netgo) by CGO_ENABLED=0 and w/ cgo + w/ netgo (see
issue golang.org/issue/4078) by CGO_ENABLED=1 and -tags netgo.

> Please note that even when I implement cgo I will *not* use cgo
> for networking, instead I will do what Windows does.

Then you should drop solaris tag from cgo_unix.go, otherwise we will
confuse; looks like solaris wants to build net's cgo stuff but it
never tests cgo/netgo toggles, why?

[mikio, 2014-03-02 01:20:41]

Aram,

On Sun, Mar 2, 2014 at 9:02 AM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:

>> Please note that even when I implement cgo I will *not* use cgo
>> for networking, instead I will do what Windows does.
>
> Then you should drop solaris tag from cgo_unix.go, otherwise we will
> confuse; looks like solaris wants to build net's cgo stuff but it
> never tests cgo/netgo toggles, why?

If you want me to drop solaris tag from cgo_unix.go in this CL, will
do so. Then other change would be just adding solaris tag to
netgo_unix_test.go.

[aram, 2014-03-02 09:02:22]

On Sun, Mar 2, 2014 at 1:02 AM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> looks like solaris wants to build net's cgo stuff

A mistake perhaps.

[aram, 2014-03-02 09:02:24]

On Sun, Mar 2, 2014 at 2:20 AM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> If you want me to drop solaris tag from cgo_unix.go in this CL, will
> do so. Then other change would be just adding solaris tag to
> netgo_unix_test.go.

Yes, this is fine, please also change the commit message accordingly.

[mikio, 2014-03-02 09:25:01]

Hello golang-codereviews@googlegroups.com, dave@cheney.net, aram@mgk.ro (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[aram, 2014-03-02 09:36:21]

LGTM

[jsing, 2014-03-02 14:59:30]

LGTM

[mikio, 2014-03-02 22:09:40]

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

net: remove the dreg of solaris merge

Also enables netgo test on solaris.

LGTM=aram, jsing
R=golang-codereviews, dave, aram, jsing
CC=golang-codereviews
https://codereview.appspot.com/70300043