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

all: run.{bash,bat,rc} sets GOPATH inconsistently #45238

Closed
dmitshur opened this issue Mar 25, 2021 · 5 comments
Closed

all: run.{bash,bat,rc} sets GOPATH inconsistently #45238

dmitshur opened this issue Mar 25, 2021 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

The run scripts have diverged in their normalization of the GOPATH environment variable. They used to all clear it, but that stopped being effective when Go 1.8 introduced a default GOPATH value (see https://golang.org/doc/go1.8#gopath).

run.bash has started to set it to /dev/null, while run.bat and run.rc were never updated.

Setting GOPATH to /dev/null has no analogue on Windows, and causes problems for some cmd/go tests (which are skipped in short mode, so they only run on longtest builders or when run explicitly):

$ GOPATH=/dev/null go test -count=1 cmd/go/internal/modload
[...]
--- FAIL: TestQueryImport (9.81s)
    --- FAIL: TestQueryImport/golang.org_x_net_context (2.47s)
        import_test.go:81: queryImport(_, "golang.org/x/net/context"): golang.org/x/[email protected]: verifying module: golang.org/x/[email protected]: initializing sumdb.Client: open /dev/null/pkg/sumdb/sum.golang.org/latest: not a directory
[...]
FAIL	cmd/go/internal/modload	14.294s

# (There are many more instances of this.)

CC @bcmills, @rsc, @golang/release.

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

Fixed by CL 288818.

@dmitshur
Copy link
Contributor Author

@gopherbot Please consider this for backport.

This should be a test-only fix, since releases use make.bash (all.bash is used only during release testing), and is needed to be able to run long tests for future 1.16 and 1.15 minor releases (issue #29252).

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #45239 (for 1.15), #45240 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@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
…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
…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]>
@golang golang locked and limited conversation to collaborators Mar 25, 2022
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

2 participants