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

x/tools/gopls: CL 237517 breaks tempModfile at startup #39681

Closed
myitcv opened this issue Jun 18, 2020 · 4 comments
Closed

x/tools/gopls: CL 237517 breaks tempModfile at startup #39681

myitcv opened this issue Jun 18, 2020 · 4 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jun 18, 2020

What version of Go are you using (go version)?

$ go version
go version devel +d286e61b67 Mon Jun 15 23:29:23 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200618023723-47c907e25813
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20200618023723-47c907e25813

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build549147748=/tmp/go-build -gno-record-gcc-switches"

What did you do?

govim has a test to ensure that the tempModfile setting does not modify a go.mod file on startup:

https://github.com/govim/govim/blob/e2a1802131f331832d667736aa27f023636f0c3a/cmd/govim/testdata/scenario_tempmodfile/modfile_changes.txt

CL 237517 appears to have broken this test:

--- FAIL: TestScripts (1.19s)
    main_test.go:77: using gopls at "/tmp/gobin-gopls-installdir036629073/gopls"
    --- FAIL: TestScripts/scenario_tempmodfile (0.03s)
        --- FAIL: TestScripts/scenario_tempmodfile/modfile_changes (5.38s)
            testscript.go:348:
                # A simple test that verifies we get a diagnostic for imports
                # that are missing from our go.mod file (0.000s)
                # Open main.go and save to ensure we are actually doing something via gopls (0.168s)
                # Sleep because we don't have an event we are waiting for (5.001s)
                # Verify that there have been no changes to go.mod (0.005s)
                > cmp go.mod go.mod.golden
                [diff -go.mod +go.mod.golden]
                 module mod.com

                -go 1.15

                FAIL: testdata/scenario_tempmodfile/modfile_changes.txt:16: go.mod and go.mod.golden differ

i.e. with that CL applied, the resulting go.mod file after opening and saving main.go has been modified with go 1.15 added (in my local tests, that's the version of Go I am using)

Caught in the daily govim vs gopls@master tests:

https://github.com/govim/govim/actions/runs/139442828

What did you expect to see?

The test continuing to pass.

What did you see instead?

As above.


cc @stamblerre @findleyr

FYI @leitzler

@gopherbot gopherbot added the Tools label Jun 18, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jun 18, 2020
@findleyr
Copy link
Contributor

@findleyr findleyr commented Jun 18, 2020

Hmm, looks like this test is not -short, which is why we didn't catch it in our govim integration.

I can switch back to running all tests, not just -short, though I may need to extend our timeout to 20m, which is quite long. Is there a good way to skip just the installer tests?

@myitcv
Copy link
Member Author

@myitcv myitcv commented Jun 18, 2020

The install scripts will only run if GOVIM_RUN_INSTALL_TESTSCRIPTS=true

https://github.com/govim/govim/blob/e2a1802131f331832d667736aa27f023636f0c3a/cmd/govim/main_test.go#L258

So you should be fine by default.

@findleyr
Copy link
Contributor

@findleyr findleyr commented Jun 18, 2020

The install scripts will only run if GOVIM_RUN_INSTALL_TESTSCRIPTS=true

Ah right, I forgot that. Thanks.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 18, 2020

Change https://golang.org/cl/238737 mentions this issue: internal/lsp: use -modfile for import organization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.