code review 1784042: misc/builder: support for uploading tarballs to googlecode

misc/builder: support for uploading tarballs to googlecode

[adg, 2010-07-16 06:33:42]

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

I'd like you to review this change.

[rsc1, 2010-07-16 18:03:16]

I'd rather read the password out of a file than 
put it on the command line.  On the command line
it shows up in ps.

Also, the built binaries will have a baked in
default $GOROOT of wherever the build runs,
which isn't great.  Can we address that too?
Should be able to do something like

BAKED_GOROOT?=$(GOROOT)

in src/lib9/Makefile and then use it in the
command line for building goos.o instead
of the standard $(GOROOT).  And then the
builders can run with BAKED_GOROOT=/usr/local/go
or something generic like that.

http://codereview.appspot.com/1784042/diff/5001/6006
File src/clean.bash (right):

http://codereview.appspot.com/1784042/diff/5001/6006#newcode15
src/clean.bash:15: if [ "x$1" != "x--nopkg" ]; then
With the "" you don't need the x trick.

http://codereview.appspot.com/1784042/diff/5001/6006#newcode17
src/clean.bash:17: rm -f "$GOROOT"/lib/*.a
These can always be deleted.  They are not used by the installed code.

[adg, 2010-07-17 01:24:50]

On 17 July 2010 04:03,  <rsc@google.com> wrote:
> I'd rather read the password out of a file than
> put it on the command line.  On the command line
> it shows up in ps.
>
> Also, the built binaries will have a baked in
> default $GOROOT of wherever the build runs,
> which isn't great.  Can we address that too?
> Should be able to do something like
>
> BAKED_GOROOT?=$(GOROOT)
>
> in src/lib9/Makefile and then use it in the
> command line for building goos.o instead
> of the standard $(GOROOT).  And then the
> builders can run with BAKED_GOROOT=/usr/local/go
> or something generic like that.

I don't like the idea of forcing people to put the binaries in a
specific location. It's reasonable that someone might want to download
the binaries to a system (on which they don't have root, like a
university system) and run the toolchain from their home directory.

What if we modified the toolchain to use the environment variable
instead of baking it in? Or at least making this an option.

Andrew

[rsc, 2010-07-17 01:52:57]

On Fri, Jul 16, 2010 at 18:24, Andrew Gerrand <adg@golang.org> wrote:
> On 17 July 2010 04:03,  <rsc@google.com> wrote:
>> I'd rather read the password out of a file than
>> put it on the command line.  On the command line
>> it shows up in ps.
>>
>> Also, the built binaries will have a baked in
>> default $GOROOT of wherever the build runs,
>> which isn't great.  Can we address that too?
>> Should be able to do something like
>>
>> BAKED_GOROOT?=$(GOROOT)
>>
>> in src/lib9/Makefile and then use it in the
>> command line for building goos.o instead
>> of the standard $(GOROOT).  And then the
>> builders can run with BAKED_GOROOT=/usr/local/go
>> or something generic like that.
>
> I don't like the idea of forcing people to put the binaries in a
> specific location. It's reasonable that someone might want to download
> the binaries to a system (on which they don't have root, like a
> university system) and run the toolchain from their home directory.
>
> What if we modified the toolchain to use the environment variable
> instead of baking it in? Or at least making this an option.

It does use the environment variable.  But there's a default
in the binary for when $GOROOT is not set.  It is only that
default that I'm talking about changing.  As long as people
have $GOROOT set, they'll never notice.  But if they don't
have $GOROOT set, I would rather the error messages
be about /usr/local/go than about /home/rsc/go-linux-amd64/go/candidate
(the $GOROOT during a builder build).

Russ

[adg, 2010-07-17 01:56:14]

On 17 July 2010 11:52, Russ Cox <rsc@golang.org> wrote:
> It does use the environment variable.  But there's a default
> in the binary for when $GOROOT is not set.  It is only that
> default that I'm talking about changing.  As long as people
> have $GOROOT set, they'll never notice.  But if they don't
> have $GOROOT set, I would rather the error messages
> be about /usr/local/go than about /home/rsc/go-linux-amd64/go/candidate
> (the $GOROOT during a builder build).

In that case, SGTM. I'll implement your suggestion.

Andrew

[adg, 2010-07-19 06:11:09]

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

I'd like you to review this change.

[adg, 2010-07-21 22:51:40]

Ping. It'd be nice to get this running on some of the builders before
the next release.

On Monday, July 19, 2010,  <adg@golang.org> wrote:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> http://codereview.appspot.com/1784042/show
>

[rsc1, 2010-07-29 22:54:10]

looks good modulo the two config files.
i assume you have tested this somehow?

[rsc, 2010-07-29 23:33:22]

LGTM

but please fix the README to mention the
subkey name (.gobuildkey-whatever)

[adg, 2010-07-29 23:36:09]

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

Please take another look.

[adg, 2010-07-30 04:01:10]

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

misc/builder: support for uploading tarballs to googlecode

R=rsc
CC=golang-dev
http://codereview.appspot.com/1784042

Committer: Andrew Gerrand <adg@golang.org>