From a4b4a6733aaeb3889123ecbd9f260a1a546080f9 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 23 Jan 2020 17:36:38 -0500 Subject: [PATCH] internal/lsp/cache: check go.mod even if tempModFile is false Missed a pretty key case in CL 216138. Also, I accidentally made it pass -modfile even when it's not supported. Change-Id: Ia0d04be7e82b77e1ec3f57ee2fee04e8c14a7c90 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216140 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/modfiles.go | 25 +++++++++++++------------ internal/lsp/cache/session.go | 5 +++-- internal/lsp/cache/view.go | 32 +++++++++++++++----------------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/internal/lsp/cache/modfiles.go b/internal/lsp/cache/modfiles.go index 0e817744d16..c33b494bc28 100644 --- a/internal/lsp/cache/modfiles.go +++ b/internal/lsp/cache/modfiles.go @@ -39,19 +39,18 @@ func (v *view) modfileFlagExists(ctx context.Context, env []string) (bool, error return lines[0] == "go1.14", nil } -func (v *view) setModuleInformation(ctx context.Context, enabled bool) error { - // The user has disabled the use of the -modfile flag. - if !enabled { - log.Print(ctx, "using the -modfile flag is disabled", telemetry.Directory.Of(v.folder)) - return nil - } - modFile := strings.TrimSpace(v.gomod) +func (v *view) setModuleInformation(ctx context.Context, gomod string, modfileFlagEnabled bool) error { + modFile := strings.TrimSpace(gomod) if modFile == os.DevNull { return nil } - v.mod = &moduleInformation{ + v.mod = moduleInformation{ realMod: span.FileURI(modFile), } + // The user has disabled the use of the -modfile flag. + if !modfileFlagEnabled { + return nil + } if modfileFlag, err := v.modfileFlagExists(ctx, v.Options().Env); err != nil { return err } else if !modfileFlag { @@ -87,7 +86,7 @@ func (v *view) setModuleInformation(ctx context.Context, enabled bool) error { if err != nil { return err } - if err := ioutil.WriteFile(v.mod.tempSumFile(), contents, stat.Mode()); err != nil { + if err := ioutil.WriteFile(tempSumFile(tempModFile.Name()), contents, stat.Mode()); err != nil { return err } return nil @@ -95,7 +94,9 @@ func (v *view) setModuleInformation(ctx context.Context, enabled bool) error { // tempSumFile returns the path to the copied temporary go.sum file. // It simply replaces the extension of the temporary go.mod file with "sum". -func (mod *moduleInformation) tempSumFile() string { - tmp := mod.tempMod.Filename() - return tmp[:len(tmp)-len("mod")] + "sum" +func tempSumFile(filename string) string { + if filename == "" { + return "" + } + return filename[:len(filename)-len("mod")] + "sum" } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 25d490a504a..03f1404d09e 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -106,11 +106,12 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI, } // Make sure to get the `go env` before continuing with initialization. - if err := v.setGoEnv(ctx, folder.Filename(), options.Env); err != nil { + gomod, err := v.setGoEnv(ctx, folder.Filename(), options.Env) + if err != nil { return nil, nil, err } // Set the module-specific information. - if err := v.setModuleInformation(ctx, v.options.TempModfile); err != nil { + if err := v.setModuleInformation(ctx, gomod, v.options.TempModfile); err != nil { return nil, nil, err } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index afe68a66950..2bacbc1869d 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -58,7 +58,7 @@ type view struct { folder span.URI // mod is the module information for this view. - mod *moduleInformation + mod moduleInformation // importsMu guards imports-related state, particularly the ProcessEnv. importsMu sync.Mutex @@ -86,7 +86,7 @@ type view struct { ignoredURIs map[span.URI]struct{} // `go env` variables that need to be tracked by the view. - gopath, gomod, gocache string + gopath, gocache string // initialized is closed when the view has been fully initialized. // On initialization, the view's workspace packages are loaded. @@ -141,10 +141,6 @@ type moduleInformation struct { } func (v *view) ModFiles() (span.URI, span.URI) { - // Don't return errors if the view is not in a module. - if v.mod == nil { - return "", "" - } return v.mod.realMod, v.mod.tempMod } @@ -258,7 +254,7 @@ func (v *view) Config(ctx context.Context) *packages.Config { // We want to run the go commands with the -modfile flag if the version of go // that we are using supports it. buildFlags := v.options.BuildFlags - if v.mod != nil { + if v.mod.tempMod != "" { buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", v.mod.tempMod.Filename())) } cfg := &packages.Config{ @@ -300,7 +296,7 @@ func (v *view) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) } // In module mode, check if the mod file has changed. - if v.mod != nil { + if v.mod.realMod != "" { mod, err := v.Snapshot().GetFile(v.mod.realMod) if err == nil && mod.Identity() != v.cachedModFileVersion { v.processEnv.GetResolver().(*imports.ModuleResolver).ClearForNewMod() @@ -489,9 +485,9 @@ func (v *view) shutdown(context.Context) { v.cancel() v.cancel = nil } - if v.mod != nil { + if v.mod.tempMod != "" { os.Remove(v.mod.tempMod.Filename()) - os.Remove(v.mod.tempSumFile()) + os.Remove(tempSumFile(v.mod.tempMod.Filename())) } debug.DropView(debugView{v}) } @@ -618,7 +614,9 @@ func (v *view) cancelBackground() { v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx) } -func (v *view) setGoEnv(ctx context.Context, dir string, env []string) error { +// setGoEnv sets the view's GOPATH and GOCACHE values. +// It also returns the view's GOMOD value, which need not be cached. +func (v *view) setGoEnv(ctx context.Context, dir string, env []string) (string, error) { var gocache, gopath bool for _, e := range env { split := strings.Split(e, "=") @@ -636,29 +634,29 @@ func (v *view) setGoEnv(ctx context.Context, dir string, env []string) error { } b, err := source.InvokeGo(ctx, dir, env, "env", "-json") if err != nil { - return err + return "", err } envMap := make(map[string]string) decoder := json.NewDecoder(b) if err := decoder.Decode(&envMap); err != nil { - return err + return "", err } if !gopath { if gopath, ok := envMap["GOPATH"]; ok { v.gopath = gopath } else { - return errors.New("unable to determine GOPATH") + return "", errors.New("unable to determine GOPATH") } } if !gocache { if gocache, ok := envMap["GOCACHE"]; ok { v.gocache = gocache } else { - return errors.New("unable to determine GOCACHE") + return "", errors.New("unable to determine GOCACHE") } } if gomod, ok := envMap["GOMOD"]; ok { - v.gomod = gomod + return gomod, nil } - return nil + return "", nil }