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

Jump to bottom

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
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.4.2

Comments

@myitcv
Copy link
Member

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
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Jun 18, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 18, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jun 18, 2020
@findleyr
Copy link
Contributor

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 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

The install scripts will only run if GOVIM_RUN_INSTALL_TESTSCRIPTS=true

Ah right, I forgot that. Thanks.

@gopherbot
Copy link

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

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Jun 18, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.2 Jun 20, 2020
@golang golang locked and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.4.2
Development

No branches or pull requests
4 participants
@myitcv @stamblerre @gopherbot @findleyr