Remove GOBIN in PATH dependency; don't assume cwd is $G...

Remove GOBIN in PATH dependency; don't assume cwd is $GOROOT/src

This change removes the necessity to have GOBIN in $PATH,
and also doesn't assume that the build is being run from
$GOROOT/src. This is a minimal set of necessary changes
to get Go to build happily from the FreeBSD ports
collection.

[rsc, 2009-12-09 22:00:36]

I don't set $GOBIN.
Please don't break me.

http://codereview.appspot.com/171044/diff/1/4
File src/Make.arm (right):

http://codereview.appspot.com/171044/diff/1/4#newcode6
src/Make.arm:6: AS="${GOBIN}"/${O}a
This is not the right place for this change.
Among other things, $GOBIN isn't even set here
necessarily, and this should be QUOTED_GOBIN
anyway.

I think you should delete the lines here and move
them into Make.pkg and Make.cmd.

http://codereview.appspot.com/171044/diff/1/5
File src/Make.conf (right):

http://codereview.appspot.com/171044/diff/1/5#newcode15
src/Make.conf:15: CC=${QUOTED_GOBIN}/quietgcc
we're using $() not ${} in makefiles.

http://codereview.appspot.com/171044/diff/1/7
File src/Makefile (right):

http://codereview.appspot.com/171044/diff/1/7#newcode1
src/Makefile:1: all: build run
This (dropping all.bash for a Makefile)
should be done in a separate CL.

[dho, 2009-12-09 23:34:52]

On 2009/12/09 22:00:36, rsc wrote:
> I don't set $GOBIN.
> Please don't break me.

Just tested this with no GOBIN set. Introduces some extra patches to make this
work -- sorry about that.

> http://codereview.appspot.com/171044/diff/1/4
> File src/Make.arm (right):
> 
> http://codereview.appspot.com/171044/diff/1/4#newcode6
> src/Make.arm:6: AS="${GOBIN}"/${O}a
> This is not the right place for this change.
> Among other things, $GOBIN isn't even set here
> necessarily, and this should be QUOTED_GOBIN
> anyway.
> 
> I think you should delete the lines here and move
> them into Make.pkg and Make.cmd.

Done

> http://codereview.appspot.com/171044/diff/1/5
> File src/Make.conf (right):
> 
> http://codereview.appspot.com/171044/diff/1/5#newcode15
> src/Make.conf:15: CC=${QUOTED_GOBIN}/quietgcc
> we're using $() not ${} in makefiles.

Yep; done. Found that I put these in a couple other places as well -- oops.

> http://codereview.appspot.com/171044/diff/1/7
> File src/Makefile (right):
> 
> http://codereview.appspot.com/171044/diff/1/7#newcode1
> src/Makefile:1: all: build run
> This (dropping all.bash for a Makefile)
> should be done in a separate CL.

Put that in 171048, I believe.

[rsc, 2009-12-11 19:07:20]

i'd like you to update the docs for this change too,
but wait until the other one is submitted.

src/cmd/gotest/gotest should need to have 
$GC and $GL changed.  i'm not convinced you've
run this without having 6g and 6l hiding in your
path somewhere.  ;-)

russ

http://codereview.appspot.com/171044/diff/1003/1014
File src/make-arm.bash (right):

http://codereview.appspot.com/171044/diff/1003/1014#newcode18
src/make-arm.bash:18: echo '$GOBIN is not a directory or does not exist' 1>&2
While you're here, could you change this message to 
sniff whether $GOBIN is set at all (you'll have to save
the old setting above).  Some people misinterpret this
message as thinking that $GOBIN must be set.  I'd rather
see a message that varies depending on the case,
one of:

$GOBIN is not set and $HOME/bin is not a directory or does not exist.
mkdir $HOME/bin or set $GOBIN to the directory where binaries
should be installed.

$GOBIN is not a directory or does not exist.
Set $GOBIN to the directory where binaries should be installed,
or unset it to fall back to $HOME/bin.

http://codereview.appspot.com/171044/diff/1003/1016
File src/pkg/exp/eval/test.bash (right):

http://codereview.appspot.com/171044/diff/1003/1016#newcode13
src/pkg/exp/eval/test.bash:13: 6g main.go && 6l main.6
need some GOBINs here, no?

[dho, 2009-12-11 21:28:56]

2009/12/11  <rsc@golang.org>:
> i'd like you to update the docs for this change too,
> but wait until the other one is submitted.

Done. Let me know if the wording's appropriate?

> src/cmd/gotest/gotest should need to have
> $GC and $GL changed.  i'm not convinced you've
> run this without having 6g and 6l hiding in your
> path somewhere.  ;-)

Indeed -- I thought I'd cleared that out, but apparently not. This
time I've verified that it wasn't in PATH, by explicitly setting PATH
and finding some additional pieces I missed :)

> russ
>
>
>
> http://codereview.appspot.com/171044/diff/1003/1014
> File src/make-arm.bash (right):
>
> http://codereview.appspot.com/171044/diff/1003/1014#newcode18
> src/make-arm.bash:18: echo '$GOBIN is not a directory or does not exist'
> 1>&2
> While you're here, could you change this message to
> sniff whether $GOBIN is set at all (you'll have to save
> the old setting above).  Some people misinterpret this
> message as thinking that $GOBIN must be set.  I'd rather
> see a message that varies depending on the case,
> one of:
>
> $GOBIN is not set and $HOME/bin is not a directory or does not exist.
> mkdir $HOME/bin or set $GOBIN to the directory where binaries
> should be installed.
>
> $GOBIN is not a directory or does not exist.
> Set $GOBIN to the directory where binaries should be installed,
> or unset it to fall back to $HOME/bin.

Done.

> http://codereview.appspot.com/171044/diff/1003/1016
> File src/pkg/exp/eval/test.bash (right):
>
> http://codereview.appspot.com/171044/diff/1003/1016#newcode13
> src/pkg/exp/eval/test.bash:13: 6g main.go && 6l main.6
> need some GOBINs here, no?

Yep, and some other places too :)

--dho

[rsc, 2009-12-11 21:51:17]

http://codereview.appspot.com/171044/diff/2003/3002
File doc/install.html (right):

http://codereview.appspot.com/171044/diff/2003/3002#newcode84
doc/install.html:84: <code>$GOBIN</code> is a path to a directory under which go
compilers and tools are
This doesn't match the form of the others in the list.
A smaller change:

    The location where binaries will be installed.
    The default is <code>$HOME/bin</code>.
    After installing, you will want to arrange to add this
    directory to your <code>$PATH</code>, so you can use the tools.

http://codereview.appspot.com/171044/diff/2003/3002#newcode205
doc/install.html:205: After these requisite utilities are installed, optionally
set
too many text changes.

To build the Go distribution, run

http://codereview.appspot.com/171044/diff/2003/3003
File doc/progs/run (right):

http://codereview.appspot.com/171044/diff/2003/3003#newcode35
doc/progs/run:35: "${GOBIN}"/$GC $i
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3003#newcode39
doc/progs/run:39: "${GOBIN}"/$LD $1.$O
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3003#newcode48
doc/progs/run:48: "${GOBIN}"/$LD $1.$O
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3003#newcode77
doc/progs/run:77: "${GOBIN}"/$GC server.go
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3005
File misc/cgo/stdio/test.bash (right):

http://codereview.appspot.com/171044/diff/2003/3005#newcode8
misc/cgo/stdio/test.bash:8: ${GOBIN}/gomake hello fib chain
drop unnecessary braces; add ""

http://codereview.appspot.com/171044/diff/2003/3005#newcode16
misc/cgo/stdio/test.bash:16: ${GOBIN}/gomake clean
drop unnecessary braces; add ""

http://codereview.appspot.com/171044/diff/2003/3007
File src/Make.conf (right):

http://codereview.appspot.com/171044/diff/2003/3007#newcode7
src/Make.conf:7: GOBIN=$(HOME)/bin
should be QUOTED_HOME and moved further down, no?
in fact, it's already there

http://codereview.appspot.com/171044/diff/2003/3008
File src/Make.pkg (right):

http://codereview.appspot.com/171044/diff/2003/3008#newcode9
src/Make.pkg:9: # GNU Make syntax:
should be quoted home like in make.conf

http://codereview.appspot.com/171044/diff/2003/3013
File src/cmd/gotest/gotest (right):

http://codereview.appspot.com/171044/diff/2003/3013#newcode102
src/cmd/gotest/gotest:102: "${GOBIN}"/$GC -o $xofile $xgofiles
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3013#newcode118
src/cmd/gotest/gotest:118: tests=$("${GOBIN}"/6nm -s _test/$importpath.a $xofile
| egrep ' T .*·'$pattern'$' | grep -v '·.*[.·]' | sed 's/.* //; s/·/./')
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3013#newcode125
src/cmd/gotest/gotest:125: benchmarks=$("${GOBIN}"/6nm -s _test/$importpath.a
$xofile | egrep ' T .*·'$pattern'$' | grep -v '·.*[.·]' | sed 's/.* //; s/·/./')
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3013#newcode164
src/cmd/gotest/gotest:164: "${GOBIN}"/$GC _testmain.go
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3013#newcode165
src/cmd/gotest/gotest:165: "${GOBIN}"/$GL _testmain.$O
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3015
File src/make-arm.bash (right):

http://codereview.appspot.com/171044/diff/2003/3015#newcode8
src/make-arm.bash:8: if test -z "${GOBIN}"; then
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3015#newcode15
src/make-arm.bash:15: fi
    GOBIN="$HOME/bin"
elif ! test -d "$GOBIN"; then
    ... message here....
fi

http://codereview.appspot.com/171044/diff/2003/3016
File src/make.bash (right):

http://codereview.appspot.com/171044/diff/2003/3016#newcode8
src/make.bash:8: if test -z "${GOBIN}"; then
same comments as in make-arm.bash

http://codereview.appspot.com/171044/diff/2003/3021
File test/bench/timing.sh (right):

http://codereview.appspot.com/171044/diff/2003/3021#newcode21
test/bench/timing.sh:21: "${GOBIN}"/$GC $1.go; "${GOBIN}"/$LD $1.$O
drop unnecessary braces

http://codereview.appspot.com/171044/diff/2003/3021#newcode25
test/bench/timing.sh:25: "${GOBIN}"/$GC -B $1.go; "${GOBIN}"/$LD $1.$O
drop unnecessary braces

[dho, 2009-12-11 22:15:36]

2009/12/11  <rsc@golang.org>:
>
> http://codereview.appspot.com/171044/diff/2003/3002
> File doc/install.html (right):
>
> http://codereview.appspot.com/171044/diff/2003/3002#newcode84
> doc/install.html:84: <code>$GOBIN</code> is a path to a directory under
> which go compilers and tools are
> This doesn't match the form of the others in the list.
> A smaller change:
>
>    The location where binaries will be installed.
>    The default is <code>$HOME/bin</code>.
>    After installing, you will want to arrange to add this
>    directory to your <code>$PATH</code>, so you can use the tools.

Done.

> http://codereview.appspot.com/171044/diff/2003/3002#newcode205
> doc/install.html:205: After these requisite utilities are installed,
> optionally set
> too many text changes.
>
> To build the Go distribution, run

Done. Sorry, thought it read a bit better.

> http://codereview.appspot.com/171044/diff/2003/3003
> File doc/progs/run (right):
>
> http://codereview.appspot.com/171044/diff/2003/3003#newcode35
> doc/progs/run:35: "${GOBIN}"/$GC $i
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3003#newcode39
> doc/progs/run:39: "${GOBIN}"/$LD $1.$O
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3003#newcode48
> doc/progs/run:48: "${GOBIN}"/$LD $1.$O
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3003#newcode77
> doc/progs/run:77: "${GOBIN}"/$GC server.go
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3005
> File misc/cgo/stdio/test.bash (right):
>
> http://codereview.appspot.com/171044/diff/2003/3005#newcode8
> misc/cgo/stdio/test.bash:8: ${GOBIN}/gomake hello fib chain
> drop unnecessary braces; add ""

Done

> http://codereview.appspot.com/171044/diff/2003/3005#newcode16
> misc/cgo/stdio/test.bash:16: ${GOBIN}/gomake clean
> drop unnecessary braces; add ""

Done

> http://codereview.appspot.com/171044/diff/2003/3007
> File src/Make.conf (right):
>
> http://codereview.appspot.com/171044/diff/2003/3007#newcode7
> src/Make.conf:7: GOBIN=$(HOME)/bin
> should be QUOTED_HOME and moved further down, no?
> in fact, it's already there

Done

> http://codereview.appspot.com/171044/diff/2003/3008
> File src/Make.pkg (right):
>
> http://codereview.appspot.com/171044/diff/2003/3008#newcode9
> src/Make.pkg:9: # GNU Make syntax:
> should be quoted home like in make.conf

Done

> http://codereview.appspot.com/171044/diff/2003/3013
> File src/cmd/gotest/gotest (right):
>
> http://codereview.appspot.com/171044/diff/2003/3013#newcode102
> src/cmd/gotest/gotest:102: "${GOBIN}"/$GC -o $xofile $xgofiles
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3013#newcode118
> src/cmd/gotest/gotest:118: tests=$("${GOBIN}"/6nm -s _test/$importpath.a
> $xofile | egrep ' T .*·'$pattern'$' | grep -v '·.*[.·]' | sed 's/.* //;
> s/·/./')
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3013#newcode125
> src/cmd/gotest/gotest:125: benchmarks=$("${GOBIN}"/6nm -s
> _test/$importpath.a $xofile | egrep ' T .*·'$pattern'$' | grep -v
> '·.*[.·]' | sed 's/.* //; s/·/./')
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3013#newcode164
> src/cmd/gotest/gotest:164: "${GOBIN}"/$GC _testmain.go
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3013#newcode165
> src/cmd/gotest/gotest:165: "${GOBIN}"/$GL _testmain.$O
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3015
> File src/make-arm.bash (right):
>
> http://codereview.appspot.com/171044/diff/2003/3015#newcode8
> src/make-arm.bash:8: if test -z "${GOBIN}"; then
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3015#newcode15
> src/make-arm.bash:15: fi
>    GOBIN="$HOME/bin"
> elif ! test -d "$GOBIN"; then
>    ... message here....
> fi

Done

> http://codereview.appspot.com/171044/diff/2003/3016
> File src/make.bash (right):
>
> http://codereview.appspot.com/171044/diff/2003/3016#newcode8
> src/make.bash:8: if test -z "${GOBIN}"; then
> same comments as in make-arm.bash

Done

> http://codereview.appspot.com/171044/diff/2003/3021
> File test/bench/timing.sh (right):
>
> http://codereview.appspot.com/171044/diff/2003/3021#newcode21
> test/bench/timing.sh:21: "${GOBIN}"/$GC $1.go; "${GOBIN}"/$LD $1.$O
> drop unnecessary braces

Done

> http://codereview.appspot.com/171044/diff/2003/3021#newcode25
> test/bench/timing.sh:25: "${GOBIN}"/$GC -B $1.go; "${GOBIN}"/$LD $1.$O
> drop unnecessary braces

Done. Sorry for wasting time with reviews.

-dho

[dho, 2009-12-11 22:44:54]

Just tested this latest patchset with:

GOBIN=/home/dho/go\ bin
GOROOT=/home/dho/go\ lang/

Looks like it works; also without GOBIN set.

--dho

[rsc, 2009-12-11 23:14:11]

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

Remove GOBIN in PATH dependency; don't assume cwd is $GOROOT/src

This change removes the necessity to have GOBIN in $PATH,
and also doesn't assume that the build is being run from
$GOROOT/src. This is a minimal set of necessary changes
to get Go to build happily from the FreeBSD ports
collection.

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

Committer: Russ Cox <rsc@golang.org>