code review 5794057: build, misc/dist: rebuild correct cmd/dist for binary d...

build: build correct cmd/dist matching GOHOSTARCH
        Fix for issue 3210 comment #1.

[minux1, 2012-03-11 20:01:22]

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

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

[adg, 2012-03-12 02:39:55]

http://codereview.appspot.com/5794057/diff/4001/src/make.bash
File src/make.bash (right):

http://codereview.appspot.com/5794057/diff/4001/src/make.bash#newcode127
src/make.bash:127: # When doing binary distribution, we have to rebuild cmd/dist
using GOGCCFLAGS to
what problem does this solve?

[minux1, 2012-03-12 03:37:22]

On 2012/03/12 02:39:55, adg wrote:
> http://codereview.appspot.com/5794057/diff/4001/src/make.bash
> File src/make.bash (right):
> 
> http://codereview.appspot.com/5794057/diff/4001/src/make.bash#newcode127
> src/make.bash:127: # When doing binary distribution, we have to rebuild
cmd/dist
> using GOGCCFLAGS to
> what problem does this solve?
On 64-bit hosts, plain gcc default to build 64-bit version. So if you don't
rebuild
cmd/dist again with GOGCCFLAGS, you will get 64-bit version for 32-bit
distribution.

[adg, 2012-03-12 03:47:43]

On 12 March 2012 14:37,  <minux.ma@gmail.com> wrote:
> On 2012/03/12 02:39:55, adg wrote:
>>
>> http://codereview.appspot.com/5794057/diff/4001/src/make.bash
>> File src/make.bash (right):
>
>
>
> http://codereview.appspot.com/5794057/diff/4001/src/make.bash#newcode127
>>
>> src/make.bash:127: # When doing binary distribution, we have to
>
> rebuild cmd/dist
>>
>> using GOGCCFLAGS to
>> what problem does this solve?
>
> On 64-bit hosts, plain gcc default to build 64-bit version. So if you
> don't rebuild
> cmd/dist again with GOGCCFLAGS, you will get 64-bit version for 32-bit
> distribution.

I thought this was what GOHOSTARCH was for?

[minux1, 2012-03-12 03:58:26]

On Mon, Mar 12, 2012 at 11:47 AM, Andrew Gerrand <adg@golang.org> wrote:

> >> src/make.bash:127: # When doing binary distribution, we have to rebuild
> cmd/dist
> >>
> >> using GOGCCFLAGS to
> >> what problem does this solve?
> >
> > On 64-bit hosts, plain gcc default to build 64-bit version. So if you
> > don't rebuild
> > cmd/dist again with GOGCCFLAGS, you will get 64-bit version for 32-bit
> > distribution.
>
> I thought this was what GOHOSTARCH was for?
>
Yes, but but this problem is slightly different, I didn't find a way to
test whether the compiled
dist matches the GOHOSTARCH, do you have any suggestions?

[rsc, 2012-03-12 16:30:33]

How about this instead?

http://codereview.appspot.com/5794057/diff/4001/misc/dist/bindist.go
File misc/dist/bindist.go (right):

http://codereview.appspot.com/5794057/diff/4001/misc/dist/bindist.go#newcode118
misc/dist/bindist.go:118: _, err = b.run(src, "bash", "make.bash", "--bindist")
Revert.

http://codereview.appspot.com/5794057/diff/4001/src/make.bash
File src/make.bash (right):

http://codereview.appspot.com/5794057/diff/4001/src/make.bash#newcode89
src/make.bash:89: DIST_CFLAGS="-O2 -Wall -Werror -ggdb"
Let's just get it right from the beginning.

mflag=""
case "$GOHOSTARCH" in
386) mflag=-m32;;
amd64) mflag=-m64;;
esac

gcc $mflag -O2 -Wall -Werror -ggdb -o cmd/dist/dist -Icmd/dist "$DEFGOROOT"
cmd/dist/*.c

http://codereview.appspot.com/5794057/diff/4001/src/make.bash#newcode126
src/make.bash:126: if [ "$1" == "--bindist" ]; then
This isn't right.  GOGCCFLAGS is for building Go packages, not standalone C
programs.

[minux1, 2012-03-12 19:29:10]

PTAL.

[rsc, 2012-03-12 19:30:49]

LGTM

[minux1, 2012-03-12 19:34:31]

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

build: build correct cmd/dist matching GOHOSTARCH
        Fix for issue 3210 comment #1.

R=adg, rsc
CC=golang-dev
http://codereview.appspot.com/5794057