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.

Sign up for GitHub

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

Jump to bottom

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
Backlog

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
Contributor

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
@rsc
build: set GOPATH consistently in run.bash, run.bat, run.rc
bb6efb9 
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 <[email protected]>
Trust: Jason A. Donenfeld <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Reviewed-by: Jason A. Donenfeld <[email protected]>
@gopherbot
Copy link
Contributor

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
Contributor

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
@rsc @dmitshur
[release-branch.go1.15] build: set GOPATH consistently in run.bash, r…
6df5d3a 
…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 <[email protected]>
Trust: Jason A. Donenfeld <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Reviewed-by: Jason A. Donenfeld <[email protected]>
(cherry picked from commit bb6efb9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304773
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Trust: Carlos Amedee <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 29, 2021
@rsc @dmitshur
[release-branch.go1.16] build: set GOPATH consistently in run.bash, r…
1d967ab 
…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 <[email protected]>
Trust: Jason A. Donenfeld <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Reviewed-by: Jason A. Donenfeld <[email protected]>
(cherry picked from commit bb6efb9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304772
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Trust: Carlos Amedee <[email protected]>
@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
@rail
build: update go to 1.16.5
788c0d0 
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
  bazel-contrib/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 rail mentioned this issue Jun 18, 2021
Merged
12 tasks
rail added a commit to rail/cockroach that referenced this issue Jun 22, 2021
@rail
build: update go to 1.16.5
e3d35e0 
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
  bazel-contrib/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
@rail
Merge #66505
ebaee33 
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
  bazel-contrib/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 <[email protected]>
@thomaseizinger thomaseizinger mentioned this issue Jul 22, 2021
Merged
@bcmills bcmills self-assigned this Sep 21, 2021
@gopherbot
Copy link
Contributor

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
cmd/go: proceed with GOPATH unset if the command doesn't use it
5ee32ff 
For #43938

Change-Id: I0937b9bb6de3d29d7242ee61f053d4803277dc0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/351329
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
@gopherbot
Copy link
Contributor

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

@bcmills bcmills mentioned this issue Sep 22, 2021
Closed
@mtojek mtojek mentioned this issue Oct 12, 2021
Merged
9 tasks
@gopherbot
Copy link
Contributor

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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@rsc @dmitshur @bcmills @gopherbot