code review 5847063: build: do more during windows build

build: do more during windows build

- use GO_GCFLAGS and GO_LDFLAGS if supplied
- build misc\dashboard\builder and misc\goplay
- run tests in test\bench\go1
- check api compatibility

[brainman, 2012-03-19 05:34:31]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2012-03-19 20:47:01]

can a Windows user please take a look at this?

http://codereview.appspot.com/5847063/diff/3004/src/make.bat
File src/make.bat (right):

http://codereview.appspot.com/5847063/diff/3004/src/make.bat#newcode26
src/make.bat:26: :: in the built and installed packages and tools.
s/built/build/

not sure i can parse this sentence. please rewrite

[kardia, 2012-03-19 21:24:42]

Changes seem fine to me.  However, checking the go tool api will fail the build,
potentially masking other problems.  Maybe add it but keep it rem for now? 
Either that or just stub out the windows syslog and always return error?

As an off topic, the syslog "prefix string" name feels totally wrong in the
Windows context, as I imagine that will need to be used for the Event source
name.

On 2012/03/19 20:47:01, r wrote:
> can a Windows user please take a look at this?
> 
> http://codereview.appspot.com/5847063/diff/3004/src/make.bat
> File src/make.bat (right):
> 
> http://codereview.appspot.com/5847063/diff/3004/src/make.bat#newcode26
> src/make.bat:26: :: in the built and installed packages and tools.
> s/built/build/
> 
> not sure i can parse this sentence. please rewrite

[brainman, 2012-03-19 23:33:17]

Hello golang-dev@googlegroups.com, r@golang.org, kardianos@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[brainman, 2012-03-19 23:33:22]

http://codereview.appspot.com/5847063/diff/3004/src/make.bat
File src/make.bat (right):

http://codereview.appspot.com/5847063/diff/3004/src/make.bat#newcode26
src/make.bat:26: :: in the built and installed packages and tools.
On 2012/03/19 20:47:02, r wrote:
> s/built/build/
> 
> not sure i can parse this sentence. please rewrite

Done.

[brainman, 2012-03-19 23:34:11]

On 2012/03/19 21:24:42, kardia wrote:
> ...  However, checking the go tool api will fail the build,

It does not fail for me. Can you tell me how to reproduce the failure you are
seeing.

> potentially masking other problems. ...

Can you be more specific. I do not know about these.

> 
> As an off topic, the syslog "prefix string" name feels totally wrong in the
> Windows context, as I imagine that will need to be used for the Event source
> name.
> 

I think, it has been decided to postpone all these decision till later
(http://code.google.com/p/go/issues/detail?id=1108). Hasn't it?

Alex

[kardia, 2012-03-20 01:20:20]

> It does not fail for me. Can you tell me how to reproduce the failure
> you are seeing.

You are correct.  When tested, the go tool api command does not catch
that windows does not implement any of syslog when there is at least
one file.  The build succeeds.


> I think, it has been decided to postpone all these decision till later
> (http://code.google.com/p/go/issues/detail?id=1108). Hasn't it?
Again correct. Was off topic.  Sorry for the noise.

Patch works fine for me on Windows.

[r, 2012-03-20 02:09:15]

Brad: opinion about running the api checker on Windows?

http://codereview.appspot.com/5847063/diff/4004/src/run.bat
File src/run.bat (right):

http://codereview.appspot.com/5847063/diff/4004/src/run.bat#newcode60
src/run.bat:60: echo # Checking API compatibility.
I'm not sure you want to do this. There are likely to be some minor
incompatibilities (syslog comes to mind) that will cause trouble. But I am
unsure. Maybe Brad should weigh in.

[brainman, 2012-03-20 02:13:31]

http://codereview.appspot.com/5847063/diff/4004/src/run.bat
File src/run.bat (right):

http://codereview.appspot.com/5847063/diff/4004/src/run.bat#newcode60
src/run.bat:60: echo # Checking API compatibility.
On 2012/03/20 02:09:15, r wrote:
> ... There are likely to be some minor
> incompatibilities (syslog comes to mind) ...

This has been noticed and recorded in $GOROOT/api/go1.txt. That is what I am
really testing here.

> ... Maybe Brad should weigh in.

That is cool. Lets wait for Brad.

[bradfitz, 2012-03-20 02:14:20]

It's supposed to produce the same output on all platforms.  It never uses
its own local build context.  It always enumerates through all of them, and
then outputs different lines if the API differs per-context.

If Windows fails for some reason, that's interesting (a bug).

On Mon, Mar 19, 2012 at 7:09 PM, <r@golang.org> wrote:

> Brad: opinion about running the api checker on Windows?
>
>
>
http://codereview.appspot.com/**5847063/diff/4004/src/run.bat<http://coderevi...
> File src/run.bat (right):
>
>
http://codereview.appspot.com/**5847063/diff/4004/src/run.bat#**newcode60<htt...
> src/run.bat:60: echo # Checking API compatibility.
> I'm not sure you want to do this. There are likely to be some minor
> incompatibilities (syslog comes to mind) that will cause trouble. But I
> am unsure. Maybe Brad should weigh in.
>
>
http://codereview.appspot.com/**5847063/<http://codereview.appspot.com/5847063/>
>

[r, 2012-03-20 02:34:59]

LGTM

[brainman, 2012-03-20 03:04:26]

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

build: do more during windows build

- use GO_GCFLAGS and GO_LDFLAGS if supplied
- build misc\dashboard\builder and misc\goplay
- run tests in test\bench\go1
- check api compatibility

R=golang-dev, r, kardianos, bradfitz
CC=golang-dev
http://codereview.appspot.com/5847063

[frichter, 2012-03-20 05:14:44]

On Monday, March 19, 2012 10:14:18 PM UTC-4, Brad Fitzpatrick wrote:
>
> It's supposed to produce the same output on all platforms.  It never uses 
> its own local build context.  It always enumerates through all of them, and 
> then outputs different lines if the API differs per-context.
>
> If Windows fails for some reason, that's interesting (a bug).
>
>>
>>
> Not sure why, but I think it's related to the 
   // +build !windows,!plan9
lines in
  src\pkg\log\syslog\syslog.go
  src\pkg\log\syslog\syslog_test.go
  src\pkg\log\syslog\syslog_unix.go 

Changing it to 
   // +build !windows !plan9
(removing the ,)
seems to make it "see" the syslog pkg when api.exe is run on windows.
(and I didn't have the new  _windows stub version present either)

[brainman, 2012-03-20 05:52:39]

On Tuesday, March 20, 2012 4:14:43 PM UTC+11, Fred Richter wrote:
>
>
> Not sure why, but ..
>
> Changing it to 
>    // +build !windows !plan9
> (removing the ,)
> seems to make it "see" the syslog pkg when api.exe is run on windows.
> (and I didn't have the new  _windows stub version present either)
>
>
See "Build Constraints" in http://tip.golang.org/pkg/go/build/ for 
explanation..

Alex