x/tools/gopls: duplicate package paths in export data manifest (via telemetry)

[findleyr]

#!stacks
"bug.Reportf" && "(*typeCheckBatch).importMap.func1:+6" ||
"bug.Reportf" && "gcimporter.iimportCommon:+119"

Telemetry data at https://storage.googleapis.com/prod-telemetry-merged/2023-10-28.json clearly demonstrates a package that cannot be imported: there are in fact two bug reports, one each in type checking (#64235) and analysis (#64236).

Clearly some invariant is being violated. We should use the reportf callback passed into the importer to get a more accurate stack, as otherwise this is unlikely to be actionable (we can't see the error message in telemetry, which we probably didn't consider when adding this instrumentation).

Putting this in the v0.14.2 milestone as it would be nice to get to the bottom of this error. I'll push down the instrumentation.

CC @adonovan

Dups: BQtcLQ m5UK3A W_lHAg du4kbA PDrh5Q jZbDKQ 4elxTA JhOH0g oat8ig wr44gQ 6j0bvA 9wAdDQ 6LjVfg A3ISjw nJQxXw tQYZmg vx_wYg BQtcJQ WpB6LA WzmCgg DetyaA yimcvw y4xHuQ 0JP54Q -trCvQ fuzfjQ w2_GEQ 3_R2tA 3_R2vA iBh0rw m44AmQ CYJxZw WUTC9w nPNE0A 5I0Kxw kVgEzw

Dups for which I can't be bothered to include the stack: rPGxIQ _ZJXvw VJTMTw hCJB7Q 1xF43w nPNE0A m9MmHA SQT0sg TwtkSg Dups: BWn-Aw 8nKERA YQKCXA xOe7BQ y7CqCA 3ue-nQ cYxHdQ PQst8g Jm14RQ eFV2uQ cIxFiw lZ85Ug ovScmQ eZ-Gkw h1dggQ hViEvg DUHIGA zw0_tw

[adonovan]

This stack h1dggQ was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+6
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap:+18
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importPackage:+4
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+56
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
• golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
• go/types.(*Checker).importPackage:+28
• go/types.(*Checker).collectObjects.func1:+13
• go/types.(*Checker).walkDecl:+9
• go/types.(*Checker).walkDecls:=391
• go/types.(*Checker).collectObjects:+42
• go/types.(*Checker).checkFiles:+32
• go/types.(*Checker).Files:=341
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50

  golang.org/x/tools/gopls@v0.16.2 go1.21.3 darwin/arm64 neovim (1)

[adonovan]

This stack hViEvg was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+6
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap:+18
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importPackage:+4
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+56
• golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func1:+1
• golang.org/x/sync/errgroup.(*Group).Go.func1:+3
• runtime.goexit:+0

  golang.org/x/tools/gopls@v0.16.2 go1.23.1 darwin/arm64 vscode (1)

[adonovan]

This stack DUHIGA was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+6
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap:+18
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importPackage:+4
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+56
• golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func1:+1
• golang.org/x/sync/errgroup.(*Group).Go.func1:+3
• runtime.goexit:+0

  golang.org/x/tools/gopls@v0.16.2 go1.23.2 darwin/amd64 vscode (1)

[adonovan]

This stack zw0_tw was reported by telemetry:

• gopls/bug
• golang.org/x/tools/gopls/internal/util/bug.report:+35
• golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+6
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap.func1:+11
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importMap:+18
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).importPackage:+4
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+56
• golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
• golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
• go/types.(*Checker).importPackage:+31
• go/types.(*Checker).collectObjects.func1:+13

  golang.org/x/tools/gopls@v0.16.2 go1.23.0 darwin/arm64 vscode (1)
  golang.org/x/tools/gopls@v0.16.2 go1.23.0 darwin/arm64 other (1)

[findleyr]

I believe I know what this may be, having been looking more closely at our loading logic.

I think this can happen when there are on-disk file changes that gopls does not observe, and one branch of the package tree is invalidated while the other isn't.

I also have an idea for a fix. Will try to get this in for v0.17.0.

[gopherbot]

Change https://go.dev/cl/628495 mentions this issue: gopls/internal/cache: refine bug reports for inconsistent dep view