code review 5622058: cmd/dist: generate files for package runtime

cmd/dist: generate files for package runtime
goc2c moves here.
parallel builds like old makefiles (-j4).
add clean command.
add banner command.
implement Go version check.
real argument parsing (same as 6g etc)

Windows changes will be a separate CL.

[rsc, 2012-02-03 07:49:02]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2012-02-03 18:07:04]

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/a.h
File src/cmd/dist/a.h (right):

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/a.h#newcode115
src/cmd/dist/a.h:115: void	bgrunv(char *dir, int mode, Vec *argv);
need these on windows still?

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/goc2c.c
File src/cmd/dist/goc2c.c (right):

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/goc2c.c#newcode720
src/cmd/dist/goc2c.c:720: bprintf(&out, "// AUTO-GENERATED by autogen.sh; DO NOT
EDIT\n\n");
s/autogen.sh/go tool dist/ ?

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/main.c
File src/cmd/dist/main.c (right):

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/main.c#newcode39
src/cmd/dist/main.c:39: vflag += xstrlen(argv[1]) - 1;
so -verbose is like -vvvvvvv? don't really care, unless these are documented
somewhere.

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/unix.c#newcode126
src/cmd/dist/unix.c:126: #define MAXBG 4 /* maximum number of jobs to run at
once */
that's pretty arbitrary, but fine for now.

http://codereview.appspot.com/5622058/diff/8001/src/cmd/dist/unix.c#newcode226
src/cmd/dist/unix.c:226: if(bg[i].pid == pid)
should you zero this field too, now that it's matched, in case the OS re-uses
the pid quickly?

[rsc, 2012-02-03 20:27:19]

PTAL

This is actually working with all.bash now.
I will send the Windows updates in a separate CL.
For now this is not hooked into the build so it
doesn't matter that the Windows stuff is old.

[bradfitz, 2012-02-03 20:50:34]

LGTM

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c#newcode...
src/cmd/dist/build.c:1120: "install      install individual directory\n"
install [dir] ?

[iant, 2012-02-03 22:06:45]

LGTM

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c#newcode868
src/cmd/dist/build.c:868: if(hassuffix(file, "cmd/go/doc.go") || hassuffix(file,
"cmd\\go\\doc.go"))
Is this weird special case really worth it?

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c#newcode...
src/cmd/dist/build.c:1269: case 'v':
I suppose you could make -v be an option for the dist program itself rather than
an option for the various commands.

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/unix.c
File src/cmd/dist/unix.c (right):

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/unix.c#newcode43
src/cmd/dist/unix.c:43: // bpathf is the same as bprintf (on windows it turns /
into \ after the printf).
Comment should say it returns the string.

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/unix.c#newcode236
src/cmd/dist/unix.c:236: while((pid = waitpid(-1, &status, 0)) < 0) {
Might as well just call wait rather than waitpid.

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/windows.c
File src/cmd/dist/windows.c (right):

http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/windows.c#newco...
src/cmd/dist/windows.c:10: #ifdef WIN32
Where is bpathf for Windows?

[rsc, 2012-02-03 23:15:17]

On Fri, Feb 3, 2012 at 17:06,  <iant@golang.org> wrote:
> src/cmd/dist/build.c:868: if(hassuffix(file, "cmd/go/doc.go") ||
> hassuffix(file, "cmd\\go\\doc.go"))
> Is this weird special case really worth it?

Sadly, the alternatives are all worse.
I have to figure out to exclude the file or else 6g complains
that I handed a file in 'package documentation' along with
a bunch in package main.

>
http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/build.c#newcode...
> src/cmd/dist/build.c:1269: case 'v':
> I suppose you could make -v be an option for the dist program itself
> rather than an option for the various commands.

Indeed; I was trying to keep the usage simpler: flags all in one place.

> http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/unix.c#newcode43
> src/cmd/dist/unix.c:43: // bpathf is the same as bprintf (on windows it
> turns / into \ after the printf).
> Comment should say it returns the string.

Done.

>
http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/unix.c#newcode236
> src/cmd/dist/unix.c:236: while((pid = waitpid(-1, &status, 0)) < 0) {
> Might as well just call wait rather than waitpid.

Done.  Thanks.  I can never keep them straight.

>
http://codereview.appspot.com/5622058/diff/11011/src/cmd/dist/windows.c#newco...
> src/cmd/dist/windows.c:10: #ifdef WIN32
> Where is bpathf for Windows?

Windows is a future CL.

Russ

[rsc, 2012-02-03 23:16:45]

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

cmd/dist: generate files for package runtime
goc2c moves here.
parallel builds like old makefiles (-j4).
add clean command.
add banner command.
implement Go version check.
real argument parsing (same as 6g etc)

Windows changes will be a separate CL.

R=golang-dev, bradfitz, iant
CC=golang-dev
http://codereview.appspot.com/5622058