code review 35900045: lib9, libmach, cmd/dist, go/build: add support for GOOS...

lib9, libmach, cmd/dist, go/build: add support for GOOS=solaris

This change adds solaris to the list of supported operating
systems and allows cmd/dist to be built on Solaris.

This CL has to come first because we want the tools to ignore
solaris-specific files until the whole port is integrated.

[aram, 2013-12-10 20:39:30]

Hello golang-dev@googlegroups.com,

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

[jsing, 2013-12-11 13:53:51]

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:27: #include <signal.h>
This is not specific to this port and arguably should be committed separately.

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:699: if(gohostos == nil)
This seems a bit strange - there should be nothing else that sets gohostos and
having this in the #else means that we only compile this code if gohostos has
been set above. I would revert this.

https://codereview.appspot.com/35900045/diff/180001/src/libmach/solaris.c
File src/libmach/solaris.c (right):

https://codereview.appspot.com/35900045/diff/180001/src/libmach/solaris.c#new...
src/libmach/solaris.c:1: // This is stubbed out for the moment. Will revisit
when the time comes.
Given this is a clone of an existing file, it would be better to  'hg cp' it so
that we have change history.

[rsc, 2013-12-11 15:21:57]

you can use hg cp -A to update the metadata without blowing away the file.

[aram, 2013-12-11 15:24:11]

\On Wed, Dec 11, 2013 at 4:21 PM, Russ Cox <rsc@golang.org> wrote:
> you can use hg cp -A to update the metadata without blowing away the file.
>

Thanks, I already uploaded it (see patch set 13), but Rietveld won't
display the side-by-side diff, even if it displays the unified diff...

error: old chunk mismatch

Who's fault is it?

[rsc, 2013-12-11 15:32:38]

On Wed, Dec 11, 2013 at 10:23 AM, Aram Hăvărneanu <aram@mgk.ro> wrote:

> \On Wed, Dec 11, 2013 at 4:21 PM, Russ Cox <rsc@golang.org> wrote:
> > you can use hg cp -A to update the metadata without blowing away the
> file.
> >
>
> Thanks, I already uploaded it (see patch set 13), but Rietveld won't
> display the side-by-side diff, even if it displays the unified diff...
>
> error: old chunk mismatch
>
> Who's fault is it?
>

probably rietveld. hard to say. we don't really know what causes "old chunk
mismatch".

[aram, 2013-12-11 21:23:41]

Fixed rietveld problem.

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:27: #include <signal.h>
On 2013/12/11 13:53:51, jsing wrote:
> This is not specific to this port and arguably should be committed separately.

Hmm, I agree, but a file can only be in one CL at a time. Can I leave it here or
should I create yet another tree so I can split the CL?

https://codereview.appspot.com/35900045/diff/180001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:699: if(gohostos == nil)
On 2013/12/11 13:53:51, jsing wrote:
> This seems a bit strange - there should be nothing else that sets gohostos and
> having this in the #else means that we only compile this code if gohostos has
> been set above. I would revert this.

Done.

[aram, 2013-12-11 21:28:42]

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

Please take another look.

[minux1, 2013-12-13 06:56:31]

LGTM.

[jsing, 2013-12-13 07:25:12]

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:699: fatal("unknown operating system: %s", u.sysname);
At this point u will be uninitialised. Additionally, the gohostos selection
(above) is based on defines, not the u.sysname value, so I'm not sure it makes
sense to include it here.

[rsc, 2013-12-18 20:27:28]

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/build.c#newc...
src/cmd/dist/build.c:54: "solaris",
Please sort this list.

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:699: fatal("unknown operating system: %s", u.sysname);
On 2013/12/13 07:25:12, jsing wrote:
> At this point u will be uninitialised. Additionally, the gohostos selection
> (above) is based on defines, not the u.sysname value, so I'm not sure it makes
> sense to include it here.

Yes, revert this change.

[gobot, 2013-12-20 16:26:04]

Replacing golang-dev with golang-codereviews.

[aram, 2013-12-21 21:32:33]

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

https://codereview.appspot.com/35900045/diff/310001/src/cmd/dist/unix.c#newco...
src/cmd/dist/unix.c:699: fatal("unknown operating system: %s", u.sysname);
> At this point u will be uninitialised. Additionally, the gohostos selection
> (above) is based on defines, not the u.sysname value, so I'm not sure it makes
> sense to include it here.

Not enough coffee, thanks for your patience.

[aram, 2013-12-21 21:48:00]

Hello golang-codereviews@googlegroups.com, jsing@google.com, rsc@golang.org,
minux.ma@gmail.com (cc: golang-codereviews@googlegroups.com),

Please take another look.

[aram, 2013-12-29 17:44:12]

Hello golang-codereviews@googlegroups.com, jsing@google.com, rsc@golang.org,
minux.ma@gmail.com (cc: golang-codereviews@googlegroups.com),

Please take another look.

[jsing, 2014-01-07 12:04:29]

LGTM

[jsing, 2014-01-07 12:12:22]

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

lib9, libmach, cmd/dist, go/build: add support for GOOS=solaris

This change adds solaris to the list of supported operating
systems and allows cmd/dist to be built on Solaris.

This CL has to come first because we want the tools to ignore
solaris-specific files until the whole port is integrated.

R=golang-codereviews, jsing, rsc, minux.ma
CC=golang-codereviews
https://codereview.appspot.com/35900045

Committer: Joel Sing <jsing@google.com>