code review 10017045: test: do not run SIGCHLD test on Plan 9

test: do not run SIGCHLD test on Plan 9

[ality, 2013-06-05 00:10:05]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-06-05 00:11:13]

LGTM



On Tue, Jun 4, 2013 at 5:10 PM, <ality@pbrane.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> test: do not run SIGCHLD test on Plan 9
>
> Please review this at
https://codereview.appspot.**com/10017045/<https://codereview.appspot.com/100...
>
> Affected files:
>   M test/sigchld.go
>
>
> Index: test/sigchld.go
> ==============================**==============================**=======
> --- a/test/sigchld.go
> +++ b/test/sigchld.go
> @@ -1,4 +1,4 @@
> -// +build !windows
> +// +build !windows !plan9
>  // cmpout
>
>  // Copyright 2009 The Go Authors. All rights reserved.
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[ality, 2013-06-05 00:15:02]

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

test: do not run SIGCHLD test on Plan 9

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/10017045

[brainman, 2013-06-05 04:01:22]

This breaks windows build. I think the problem is in test/run.go when it reads
build tags. I don't have time to fix it now, but I will look into it unless
someone else fixes it.

Alex

[dave_cheney.net, 2013-06-05 04:36:00]

I'm on it.

On Wed, Jun 5, 2013 at 2:01 PM,  <alex.brainman@gmail.com> wrote:
> This breaks windows build. I think the problem is in test/run.go when it
> reads build tags. I don't have time to fix it now, but I will look into
> it unless someone else fixes it.
>
> Alex
>
>
> https://codereview.appspot.com/10017045/
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[brainman, 2013-06-05 04:38:27]

On 2013/06/05 04:36:00, dfc wrote:
> I'm on it.
> 

Thank you Dave. In run.go there is mini test, if you add these lines:

assertNot(shouldTest("// +build !windows !plan9", "windows", "amd64"))
assertNot(shouldTest("// +build !plan9 !windows", "windows", "amd64"))


you should be able to fix it quicker.

Alex

[dave_cheney.net, 2013-06-05 04:39:25]

Rémy pointed out that the conditions are OR'd together, not AND'd, so
!windows OR !plan9 is always true.

On Wed, Jun 5, 2013 at 2:38 PM,  <alex.brainman@gmail.com> wrote:
> On 2013/06/05 04:36:00, dfc wrote:
>>
>> I'm on it.
>
>
>
> Thank you Dave. In run.go there is mini test, if you add these lines:
>
> assertNot(shouldTest("// +build !windows !plan9", "windows", "amd64"))
> assertNot(shouldTest("// +build !plan9 !windows", "windows", "amd64"))
>
>
> you should be able to fix it quicker.
>
> Alex
>
> https://codereview.appspot.com/10017045/

[remyoudompheng, 2013-06-05 04:40:00]

2013/6/5  <alex.brainman@gmail.com>:
> On 2013/06/05 04:36:00, dfc wrote:
>>
>> I'm on it.
>
>
>
> Thank you Dave. In run.go there is mini test, if you add these lines:
>
> assertNot(shouldTest("// +build !windows !plan9", "windows", "amd64"))
> assertNot(shouldTest("// +build !plan9 !windows", "windows", "amd64"))
>
>
> you should be able to fix it quicker.
>
>
> Alex
>
> https://codereview.appspot.com/10017045/
>

http://golang.org/pkg/go/build/
" A build constraint is evaluated as the OR of space-separated options"

run.go follows that convention.

Rémy.

[brainman, 2013-06-05 04:48:04]

On 2013/06/05 04:40:00, remyoudompheng wrote:
> 
> run.go follows that convention.
> 

You are right. The +build is wrong in sigchld.go. But replacing " " with ","
still fails, so I think run.go is broken.

Alex

[bradfitz, 2013-06-05 04:51:07]

On Tue, Jun 4, 2013 at 9:48 PM, <alex.brainman@gmail.com> wrote:

> On 2013/06/05 04:40:00, remyoudompheng wrote:
>
>  run.go follows that convention.
>>
>
>
> You are right. The +build is wrong in sigchld.go. But replacing " " with
> "," still fails, so I think run.go is broken.
>

run.go isn't broken so much as just never acquired support for the full
syntax.  It's grown as needed.