code review 5487073: os: make compatible with go/build

os: make compatible with go/build

It is probably a mistake to have these here at all -
os is supposed to be portable - but this only fixes
the build issue.

[rsc, 2011-12-14 05:09:20]

Hello golang-dev@googlegroups.com,

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

[r2, 2011-12-14 05:11:48]

you're not explaining the problem you're trying to fix here, and i can't decode
it from the CLs.

-rob

[rsc, 2011-12-14 05:28:53]

On Wed, Dec 14, 2011 at 00:11, Rob 'Commander' Pike <r@google.com> wrote:
> you're not explaining the problem you're trying to fix here, and i can't
decode it from the CLs.

Sorry.  The general problem is making the directories be
compatible with what we've laid out for goinstall to be able
to know how to build them without any extra instructions.
And by goinstall I really mean package go/build and thus
also the new go tool (same rules).

In this case, the Makefile is generating some files during
the build.  Instead of doing that, I've generated the files
and added them to the repository.

There is a separate problem - I don't think these files should
be here at all - but I've created issue 2562 for that.  This is
just about making os build with automatic tools.

Russ

[r, 2011-12-14 06:22:43]

http://codereview.appspot.com/5487073/diff/3/src/pkg/os/Makefile
File src/pkg/os/Makefile (left):

http://codereview.appspot.com/5487073/diff/3/src/pkg/os/Makefile#oldcode105
src/pkg/os/Makefile:105: ./mkunixsignals.sh $< > $@ || rm -f $@
shouldn't you keep the rules around for generating the files again? or are you
banking on the comments in the files to carry it? i'd like to see them here too

[rsc, 2011-12-14 15:29:31]

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

Please take another look.

[r, 2011-12-14 15:49:12]

http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/zsignal_darwin_386.go
File src/pkg/os/zsignal_darwin_386.go (right):

http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/zsignal_darwin_386...
src/pkg/os/zsignal_darwin_386.go:1: // ./mksignals.sh
this looks wrong.

http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/zsignal_darwin_386...
src/pkg/os/zsignal_darwin_386.go:2: // ran ./mkunixsignals.sh
../syscall/zerrors_darwin_386.go
shoud 'ran' be here?

[iant, 2011-12-14 16:00:17]

http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/mksignals.sh
File src/pkg/os/mksignals.sh (right):

http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/mksignals.sh#newcode9
src/pkg/os/mksignals.sh:9: openbsd_386 \
netbsd?  freebsd?

[rsc, 2011-12-14 16:02:00]

PTAL

freebsd added.  netbsd doesn't run because the
syscall files aren't there yet.

[rsc, 2011-12-15 16:16:56]

ping

[r2, 2011-12-15 16:18:35]

On Dec 15, 2011, at 8:16 AM, Russ Cox wrote:

> ping

i commented yesterday about the bad comments in the generated files. you didn't
respond yet.

-rob

[rsc, 2011-12-15 16:24:30]

On Wed, Dec 14, 2011 at 10:49,  <r@golang.org> wrote:
>
http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/zsignal_darwin_386...
> src/pkg/os/zsignal_darwin_386.go:1: // ./mksignals.sh
> this looks wrong.
>
>
http://codereview.appspot.com/5487073/diff/2005/src/pkg/os/zsignal_darwin_386...
> src/pkg/os/zsignal_darwin_386.go:2: // ran ./mkunixsignals.sh
> ../syscall/zerrors_darwin_386.go
> shoud 'ran' be here?

I was trying to indicate that the first program ran the second.
If you are regenerating, the thing to do is to run mksignals.sh
which builds all of them.

[rsc, 2011-12-15 16:24:41]

On Thu, Dec 15, 2011 at 11:18, Rob 'Commander' Pike <r@google.com> wrote:
> i commented yesterday about the bad comments in the generated files. you
didn't respond yet.

sorry, missed that behind ian's.

[r, 2011-12-15 17:14:52]

http://codereview.appspot.com/5487073/diff/7002/src/pkg/os/zsignal_freebsd_38...
File src/pkg/os/zsignal_freebsd_386.go (right):

http://codereview.appspot.com/5487073/diff/7002/src/pkg/os/zsignal_freebsd_38...
src/pkg/os/zsignal_freebsd_386.go:3: // MACHINE GENERATED BY THE COMMAND ABOVE;
DO NOT EDIT
i still find this confusing even after your explanation.

// MACHINE GENERATED; DO NOT EDIT.
// To regenerate, run:
// ./mksignals.sh
// which for this file will in turn run:
// ./mkunixsignals.sh ../syscall/zerrors_freebsd_386.go

[rsc, 2011-12-15 17:17:42]

PTAL

[r, 2011-12-15 17:23:12]

LGTM
much clearer

[rsc, 2011-12-15 17:33:39]

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

os: make compatible with go/build

It is probably a mistake to have these here at all -
os is supposed to be portable - but this only fixes
the build issue.

R=golang-dev, r, r, iant
CC=golang-dev
http://codereview.appspot.com/5487073