Skip to content

Commit

Permalink
internal/lsp: recover from a view initialization failure
Browse files Browse the repository at this point in the history
If an orphaned file is used to recover a workspace package, we should
remove the initialization error and treat the view as correctly
initialized.

Also, stop caching metadata for packages with no files. We have no way
to invalidate it, and it's useless, so just re-load those files as
needed.

Fixes golang/go#36795.
Fixes golang/go#36671.
Fixes golang/go#36772.

Change-Id: I0aee5a43401517b6073d27136cca533160effef2
  • Loading branch information
stamblerre committed Jan 28, 2020
1 parent 345141a commit 3b4920f
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 13 deletions.
13 changes: 7 additions & 6 deletions internal/lsp/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,15 @@ func (s *snapshot) updateMetadata(ctx context.Context, scopes []interface{}, pkg
}
}
if !containsDir || s.view.Options().VerboseOutput {
log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
log.Print(ctx, "go/packages.Load", tag.Of("snapshot", s.ID()), tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
}
// golang/go#36292: Ignore packages with no sources and no errors.
if len(pkg.GoFiles) == 0 && len(pkg.CompiledGoFiles) == 0 && len(pkg.Errors) == 0 {
// Ignore packages with no sources, since we will never be able to
// correctly invalidate that metadata.
if len(pkg.GoFiles) == 0 && len(pkg.CompiledGoFiles) == 0 {
continue
}
// Skip test main packages.
if s.view.isTestMain(ctx, pkg) {
if isTestMain(ctx, pkg, s.view.gocache) {
continue
}
// Set the metadata for this package.
Expand Down Expand Up @@ -234,7 +235,7 @@ func (s *snapshot) updateImports(ctx context.Context, pkgPath packagePath, pkg *
return nil
}

func (v *view) isTestMain(ctx context.Context, pkg *packages.Package) bool {
func isTestMain(ctx context.Context, pkg *packages.Package, gocache string) bool {
// Test mains must have an import path that ends with ".test".
if !strings.HasSuffix(pkg.PkgPath, ".test") {
return false
Expand All @@ -247,7 +248,7 @@ func (v *view) isTestMain(ctx context.Context, pkg *packages.Package) bool {
if len(pkg.GoFiles) > 1 {
return false
}
if !strings.HasPrefix(pkg.GoFiles[0], v.gocache) {
if !strings.HasPrefix(pkg.GoFiles[0], gocache) {
return false
}
return true
Expand Down
7 changes: 7 additions & 0 deletions internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,13 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
}
s.mu.Unlock()
}
// If we have found new workspace packages from orphaned files,
// make sure to clear out the view initialization error.
if len(m) > 0 {
s.view.initializationErrorMu.Lock()
s.view.initializationError = nil
s.view.initializationErrorMu.Unlock()
}
for _, m := range m {
s.setWorkspacePackage(ctx, m)
}
Expand Down
11 changes: 8 additions & 3 deletions internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ type view struct {
// On initialization, the view's workspace packages are loaded.
// All of the fields below are set as part of initialization.
// If we failed to load, we don't re-try to avoid too many go/packages calls.
initializeOnce sync.Once
initialized chan struct{}
initializationError error
initializeOnce sync.Once
initialized chan struct{}
initializationErrorMu sync.Mutex
initializationError error

// builtin pins the AST and package for builtin.go in memory.
builtin *builtinPackageHandle
Expand Down Expand Up @@ -553,6 +554,8 @@ func (v *view) initialize(ctx context.Context, s *snapshot) {
v.initializeOnce.Do(func() {
defer close(v.initialized)

v.initializationErrorMu.Lock()
defer v.initializationErrorMu.Unlock()
v.initializationError = func() error {
// Do not cancel the call to go/packages.Load for the entire workspace.
meta, err := s.load(ctx, viewLoadScope("LOAD_VIEW"), packagePath("builtin"))
Expand Down Expand Up @@ -590,6 +593,8 @@ func (v *view) awaitInitialized(ctx context.Context) error {
return ctx.Err()
case <-v.initialized:
}
v.initializationErrorMu.Lock()
defer v.initializationErrorMu.Unlock()
return v.initializationError
}

Expand Down
6 changes: 2 additions & 4 deletions internal/lsp/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,14 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) {
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
defer done()

ctx = telemetry.Snapshot.With(ctx, snapshot.ID())

// Diagnose all of the packages in the workspace.
go func() {
wsPackages, err := snapshot.WorkspacePackages(ctx)
if ctx.Err() != nil {
return
}
if err != nil {
log.Error(ctx, "diagnose: no workspace packages", err, telemetry.Directory.Of(snapshot.View().Folder))
log.Error(ctx, "diagnose: no workspace packages", err, telemetry.Snapshot.Of(snapshot.ID()), telemetry.Directory.Of(snapshot.View().Folder))
return
}
for _, ph := range wsPackages {
Expand All @@ -70,7 +68,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot) {
return
}
if err != nil {
log.Error(ctx, "diagnose: could not generate diagnostics for package", err, telemetry.Package.Of(ph.ID()))
log.Error(ctx, "diagnose: could not generate diagnostics for package", err, telemetry.Snapshot.Of(snapshot.ID()), telemetry.Package.Of(ph.ID()))
return
}
s.publishReports(ctx, snapshot, reports, withAnalyses)
Expand Down

0 comments on commit 3b4920f

Please sign in to comment.