code review 6397048: cmd/go: go get should fail if GOPATH is not set

cmd/go: go get should fail if GOPATH is not set

Correctly exporting GOPATH is an issue that comes up on IRC
frequently. This proposal causes go get to abort if GOPATH
is not set.

[dave_cheney.net, 2012-07-17 01:14:50]

Hello golang-dev@googlegroups.com,

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

[r2, 2012-07-17 02:07:10]

I'm unsure about this generally and in particular. I'd like some time to
think about the consequences.


On Mon, Jul 16, 2012 at 6:14 PM, <dave@cheney.net> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> cmd/go: go get should fail if GOPATH is not set
>
> Correctly exporting GOPATH is an issue that comes up on IRC
> frequently. This proposal causes go get to abort if GOPATH
> is not set.
>
> Please review this at
http://codereview.appspot.com/**6397048/<http://codereview.appspot.com/6397048/>
>
> Affected files:
>   M src/cmd/go/doc.go
>   M src/cmd/go/get.go
>   M src/cmd/go/help.go
>   M src/cmd/go/test.bash
>
>
> Index: src/cmd/go/doc.go
> ==============================**==============================**=======
> --- a/src/cmd/go/doc.go
> +++ b/src/cmd/go/doc.go
> @@ -445,7 +445,7 @@
>  On Windows, the value is a semicolon-separated string.
>  On Plan 9, the value is a list.
>
> -GOPATH must be set to build and install packages outside the
> +GOPATH must be set to get, build and install packages outside the
>  standard Go tree.
>
>  Each directory listed in GOPATH must have a prescribed structure:
> Index: src/cmd/go/get.go
> ==============================**==============================**=======
> --- a/src/cmd/go/get.go
> +++ b/src/cmd/go/get.go
> @@ -62,6 +62,12 @@
>  }
>
>  func runGet(cmd *Command, args []string) {
> +       // Check GOPATH is valid
> +       if gopath := os.Getenv("GOPATH"); gopath == "" {
> +               errorf("GOPATH is not set. Consult go help gopath for more
> details.")
> +       }
> +       exitIfErrors()
> +
>         // Phase 1.  Download/update.
>         var stk importStack
>         for _, arg := range downloadPaths(args) {
> Index: src/cmd/go/help.go
> ==============================**==============================**=======
> --- a/src/cmd/go/help.go
> +++ b/src/cmd/go/help.go
> @@ -186,7 +186,7 @@
>  On Windows, the value is a semicolon-separated string.
>  On Plan 9, the value is a list.
>
> -GOPATH must be set to build and install packages outside the
> +GOPATH must be set to get, build and install packages outside the
>  standard Go tree.
>
>  Each directory listed in GOPATH must have a prescribed structure:
> Index: src/cmd/go/test.bash
> ==============================**==============================**=======
> --- a/src/cmd/go/test.bash
> +++ b/src/cmd/go/test.bash
> @@ -119,6 +119,15 @@
>         ok=false
>  fi
>
> +# Test that without GOPATH set, go get should fail
> +d=$(mktemp -d)
> +mkdir -p $d/src/pkg
> +if ( unset GOPATH ; export GOROOT=$d ; ./testgo get -d
>
code.google.com/p/go.**codereview/cmd/hgpatch<http://code.google.com/p/go.codereview/cmd/hgpatch>2>&1
| grep -v -q 'GOPATH is not set' ); then
> +       echo "go get get
code.google.com/p/go.**codereview/cmd/hgpatch<http://code.google.com/p/go.codereview/cmd/hgpatch>should
not succeed with GOPATH unset"
> +       ok=false
> +fi
> +rm -rf $d
> +
>  if $ok; then
>         echo PASS
>  else
>
>
>

[dave_cheney.net, 2012-07-17 02:10:35]

On 2012/07/17 02:07:10, r2 wrote:
> I'm unsure about this generally and in particular. I'd like some time to
> think about the consequences.

Understood. For the record here is the link to the proposal,
https://groups.google.com/d/topic/golang-dev/RA1epLmPeaQ/discussion

[rsc, 2012-07-29 23:01:36]

I replied to the other thread. I don't like this proposal. I would
rather see something like a warning printed when the Go command finds
multiple possible sources for a given package path.

Russ

[dave_cheney.net, 2012-07-30 02:32:56]

Fair enough, thanks for your review.

[dave_cheney.net, 2012-07-30 02:35:19]

*** Abandoned ***

[dave_cheney.net, 2012-09-29 02:45:27]

At the risk of beating a dead horse the problem of failing to set
GOPATH correct (or at all) continues to be a stumbling block for new
Go users. This then leads them into more destructive behaviour like
chmod a+w /usr/local/go, or running go get as root. While this
solution may not be correct, could I ask that we reopen the discussion
of the logic of allowing go get to work without GOPATH set.

Cheers

Dave

On Mon, Jul 30, 2012 at 12:35 PM,  <dave@cheney.net> wrote:
> *** Abandoned ***
>
> http://codereview.appspot.com/6397048/

[rsc, 2012-10-01 19:11:13]

Maybe we should. Please file an issue so we remember. Thanks.