Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: spurious missing $GOPATH error #43938

Closed
rsc opened this issue Jan 27, 2021 · 9 comments
Closed

build: spurious missing $GOPATH error #43938

rsc opened this issue Jan 27, 2021 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Jan 27, 2021

make.bash (and friends) should not need GOPATH to be set.
Right now it is being set implicitly, which is good enough.
But if we make it impossible to set implicitly, make.bash fails.
It should not.

$ HOME='' GOPATH='' ./make.bash
missing $GOPATH
$ 

/cc @jayconrod @matloob @bcmills

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 27, 2021
@rsc rsc added this to the Go1.17 milestone Jan 27, 2021
@rsc
Copy link
Contributor Author

rsc commented Jan 27, 2021

Shorter repro case:

HOME='' GOPATH='' go env

@rsc
Copy link
Contributor Author

rsc commented Jan 27, 2021

Another fun fact: run.bat (only!) clears GOPATH, and defaultGOPATH returns an empty string if the default GOPATH comes out the same as GOROOT.

So while I have no problems at all using GOROOT=$HOME/go and GOPATH=$HOME on my Unix systems, if I do the equivalent on Unix, I can't all.bat or run.bat because the cleared GOPATH drops to the default, which comes out equal to my GOROOT, so it gets cleared too, and then we hit the missing $GOPATH fatal error.

The fatal error in cmd/go should still be fixed, but now that there's a non-empty default GOPATH, the clearing in run.bat is misleading at best and can be removed too. I'll do that.

@gopherbot
Copy link

Change https://golang.org/cl/288818 mentions this issue: build: do not clear GOPATH in run.bat

gopherbot pushed a commit that referenced this issue Feb 19, 2021
We used to clear GOPATH in all the build scripts.
Clearing GOPATH is misleading at best, since you just end up
with the default GOPATH (%USERPROFILE%\go on Windows).
Unless that's your GOROOT, in which case you end up with a
fatal error from the go command (#43938).

run.bash changed to setting GOPATH=/dev/null, which has no
clear analogue on Windows.

run.rc still clears GOPATH.

Change them all to set GOPATH to a non-existent directory
/nonexist-gopath or c:\nonexist-gopath.

Change-Id: I51edd66d37ff6a891b0d0541d91ecba97fbbb03d
Reviewed-on: https://go-review.googlesource.com/c/go/+/288818
Trust: Russ Cox <rsc@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
@gopherbot
Copy link

Change https://golang.org/cl/304772 mentions this issue: [release-branch.go1.16] build: set GOPATH consistently in run.bash, run.bat, run.rc

@gopherbot
Copy link

Change https://golang.org/cl/304773 mentions this issue: [release-branch.go1.15] build: set GOPATH consistently in run.bash, run.bat, run.rc

gopherbot pushed a commit that referenced this issue Mar 29, 2021
…un.bat, run.rc

We used to clear GOPATH in all the build scripts.
Clearing GOPATH is misleading at best, since you just end up
with the default GOPATH (%USERPROFILE%\go on Windows).
Unless that's your GOROOT, in which case you end up with a
fatal error from the go command (#43938).

run.bash changed to setting GOPATH=/dev/null, which has no
clear analogue on Windows.

run.rc still clears GOPATH.

Change them all to set GOPATH to a non-existent directory
/nonexist-gopath or c:\nonexist-gopath.

For #45238.
Fixes #45239.

Change-Id: I51edd66d37ff6a891b0d0541d91ecba97fbbb03d
Reviewed-on: https://go-review.googlesource.com/c/go/+/288818
Trust: Russ Cox <rsc@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
(cherry picked from commit bb6efb9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304773
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Trust: Carlos Amedee <carlos@golang.org>
gopherbot pushed a commit that referenced this issue Mar 29, 2021
…un.bat, run.rc

We used to clear GOPATH in all the build scripts.
Clearing GOPATH is misleading at best, since you just end up
with the default GOPATH (%USERPROFILE%\go on Windows).
Unless that's your GOROOT, in which case you end up with a
fatal error from the go command (#43938).

run.bash changed to setting GOPATH=/dev/null, which has no
clear analogue on Windows.

run.rc still clears GOPATH.

Change them all to set GOPATH to a non-existent directory
/nonexist-gopath or c:\nonexist-gopath.

For #45238.
Fixes #45240.

Change-Id: I51edd66d37ff6a891b0d0541d91ecba97fbbb03d
Reviewed-on: https://go-review.googlesource.com/c/go/+/288818
Trust: Russ Cox <rsc@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
(cherry picked from commit bb6efb9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304772
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Trust: Carlos Amedee <carlos@golang.org>
@dmitshur
Copy link
Contributor

CL 288818 has made an improvement in this area, but the bug in the original report still reproduces.

This issue doesn't seem critical and doesn't have an assignee, so moving to Backlog. If someone still has time to fix this or thinks it's important for 1.17, please update accordingly.

@dmitshur dmitshur modified the milestones: Go1.17, Backlog May 21, 2021
rail added a commit to rail/cockroach that referenced this issue Jun 18, 2021
Fixes cockroachdb#66404

* Upgrade gazelle to latest version. Go 1.16 updates `go.sum` when
  gazelle calls `go download`. The new version fixes this issue. See
  bazelbuild/bazel-gazelle#1015.
* Remove line references in `build/README.md`, because they are not
  stable.
* Set `GOPATH` in various places for bazel based builds and tests in
  order to work round the case when `go env` fails when `GOPATH` and
  `HOME` are unset. See golang/go#43938 for
  the details.
* Set `GO111MODULE=off` in `pkg/acceptance/compose/gss/psql/Dockerfile`.
  Go 1.16 requires `go.mod` in order to build and test the module
  without `GO111MODULE=off`.

Checklist:

* [x] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [X] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [X] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [x] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [ ] Adjust `GO_VERSION` in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
  and ask the Developer Infrastructure team to deploy new images.

Release note: None
rail added a commit to rail/cockroach that referenced this issue Jun 22, 2021
Fixes cockroachdb#66404

* Upgrade gazelle to latest version. Go 1.16 updates `go.sum` when
  gazelle calls `go download`. The new version fixes this issue. See
  bazelbuild/bazel-gazelle#1015.
* Remove line references in `build/README.md`, because they are not
  stable.
* Set `GOPATH` in various places for bazel based builds and tests in
  order to work round the case when `go env` fails when `GOPATH` and
  `HOME` are unset. See golang/go#43938 for
  the details.
* Set `GO111MODULE=off` in `pkg/acceptance/compose/gss/psql/Dockerfile`.
  Go 1.16 requires `go.mod` in order to build and test the module
  without `GO111MODULE=off`.

Checklist:

* [x] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Adjust version in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [x] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [ ] Ask the Developer Infrastructure team to deploy new TeamCity agent images according to [packer/README.md](./packer/README.md)

Release note: None
craig bot pushed a commit to cockroachdb/cockroach that referenced this issue Jun 23, 2021
66505: build: update go to 1.16.5 r=rickystewart a=rail

Fixes #66404

* Upgrade gazelle to latest version. Go 1.16 updates `go.sum` when
  gazelle calls `go download`. The new version fixes this issue. See
  bazelbuild/bazel-gazelle#1015.
* Remove line references in `build/README.md`, because they are not
  stable.
* Set `GOPATH` in various places for bazel based builds and tests in
  order to work round the case when `go env` fails when `GOPATH` and
  `HOME` are unset. See golang/go#43938 for
  the details.
* Set `GO111MODULE=off` in `pkg/acceptance/compose/gss/psql/Dockerfile`.
  Go 1.16 requires `go.mod` in order to build and test the module
  without `GO111MODULE=off`.

Checklist:

* [x] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Adjust version in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [x] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [ ] Ask the Developer Infrastructure team to deploy new TeamCity agent images according to [packer/README.md](./packer/README.md)

Release note: None

Co-authored-by: Rail Aliiev <rail@iqchoice.com>
@bcmills bcmills self-assigned this Sep 21, 2021
@gopherbot
Copy link

Change https://golang.org/cl/351329 mentions this issue: cmd/go: proceed with GOPATH unset if the command doesn't use it

gopherbot pushed a commit that referenced this issue Sep 22, 2021
For #43938

Change-Id: I0937b9bb6de3d29d7242ee61f053d4803277dc0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/351329
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/351529 mentions this issue: cmd/compile/internal/ssa: remove workarounds for #43938

@gopherbot
Copy link

Change https://golang.org/cl/361774 mentions this issue: [dev.boringcrypto] all: merge master (fa16efb) into dev.boringcrypto

@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants