gopls/internal/telemetry/cmd/stacks: fix two bugs
1. There were three early returns in the frame -> URL
computation, though it was hard to see, and their
formatting logic differed. This CL factors them,
extracting the frameURL function.
2. When git clone fails (e.g. due to no SSO cert),
we failed to clean up the empty dir, causing a
persistently stuck failure state. Now we attempt
to clean up the directory. (This won't help when
the program is terminated from without.)
Change-Id: I0f7e2dd26b95899ec85b3e4666def374dc8caadd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/613076
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/telemetry/cmd/stacks/stacks.go b/gopls/internal/telemetry/cmd/stacks/stacks.go
index 5d4727a..dc33380 100644
--- a/gopls/internal/telemetry/cmd/stacks/stacks.go
+++ b/gopls/internal/telemetry/cmd/stacks/stacks.go
@@ -316,75 +316,11 @@
}
// Parse the stack and get the symbol names out.
- for _, line := range strings.Split(stack, "\n") {
- // e.g. "golang.org/x/tools/gopls/foo.(*Type).Method.inlined.func3:+5"
- symbol, offset, ok := strings.Cut(line, ":")
- if !ok {
- // Not a symbol (perhaps stack counter title: "gopls/bug"?)
- fmt.Fprintf(body, "`%s`\n", line)
- continue
- }
- fileline, ok := pclntab[symbol]
- if !ok {
- // objdump reports ELF symbol names, which in
- // rare cases may be the Go symbols of
- // runtime.CallersFrames mangled by (e.g.) the
- // addition of .abi0 suffix; see
- // https://github.com/golang/go/issues/69390#issuecomment-2343795920
- // So this should not be a hard error.
- log.Printf("no pclntab info for symbol: %s", symbol)
- fmt.Fprintf(body, "`%s`\n", line)
- continue
- }
- if offset == "" {
- log.Fatalf("missing line offset: %s", line)
- }
- offsetNum, err := strconv.Atoi(offset[1:])
- if err != nil {
- log.Fatalf("invalid line offset: %s", line)
- }
- linenum := fileline.line
- switch offset[0] {
- case '-':
- linenum -= offsetNum
- case '+':
- linenum += offsetNum
- case '=':
- linenum = offsetNum
- }
-
- // Construct CodeSearch URL.
- var url string
- if firstSegment, _, _ := strings.Cut(fileline.file, "/"); !strings.Contains(firstSegment, ".") {
- // std
- // (First segment is a dir beneath GOROOT/src, not a module domain name.)
- url = fmt.Sprintf("https://cs.opensource.google/go/go/+/%s:src/%s;l=%d",
- info.GoVersion, fileline.file, linenum)
-
- } else if rest, ok := strings.CutPrefix(fileline.file, "golang.org/x/tools"); ok {
- // in x/tools repo (tools or gopls module)
- if rest[0] == '/' {
- // "golang.org/x/tools/gopls" -> "gopls"
- rest = rest[1:]
- } else if rest[0] == '@' {
- // "golang.org/x/tools@version/dir/file.go" -> "dir/file.go"
- rest = rest[strings.Index(rest, "/")+1:]
- }
-
- url = fmt.Sprintf("https://cs.opensource.google/go/x/tools/+/%s:%s;l=%d",
- "gopls/"+info.Version, rest, linenum)
-
+ for _, frame := range strings.Split(stack, "\n") {
+ if url := frameURL(pclntab, info, frame); url != "" {
+ fmt.Fprintf(body, "- [`%s`](%s)\n", frame, url)
} else {
- // TODO(adonovan): support other module dependencies of gopls.
- log.Printf("no CodeSearch URL for %q (%s:%d)",
- symbol, fileline.file, linenum)
- }
-
- // Emit stack frame.
- if url != "" {
- fmt.Fprintf(body, "- [`%s`](%s)\n", line, url)
- } else {
- fmt.Fprintf(body, "- `%s`\n", line)
+ fmt.Fprintf(body, "- `%s`\n", frame)
}
}
@@ -411,6 +347,73 @@
return title
}
+// frameURL returns the CodeSearch URL for the stack frame, if known.
+func frameURL(pclntab map[string]FileLine, info Info, frame string) string {
+ // e.g. "golang.org/x/tools/gopls/foo.(*Type).Method.inlined.func3:+5"
+ symbol, offset, ok := strings.Cut(frame, ":")
+ if !ok {
+ // Not a symbol (perhaps stack counter title: "gopls/bug"?)
+ return ""
+ }
+
+ fileline, ok := pclntab[symbol]
+ if !ok {
+ // objdump reports ELF symbol names, which in
+ // rare cases may be the Go symbols of
+ // runtime.CallersFrames mangled by (e.g.) the
+ // addition of .abi0 suffix; see
+ // https://github.com/golang/go/issues/69390#issuecomment-2343795920
+ // So this should not be a hard error.
+ log.Printf("no pclntab info for symbol: %s", symbol)
+ return ""
+ }
+
+ if offset == "" {
+ log.Fatalf("missing line offset: %s", frame)
+ }
+ offsetNum, err := strconv.Atoi(offset[1:])
+ if err != nil {
+ log.Fatalf("invalid line offset: %s", frame)
+ }
+ linenum := fileline.line
+ switch offset[0] {
+ case '-':
+ linenum -= offsetNum
+ case '+':
+ linenum += offsetNum
+ case '=':
+ linenum = offsetNum
+ }
+
+ // Construct CodeSearch URL.
+ firstSegment, _, _ := strings.Cut(fileline.file, "/")
+ if !strings.Contains(firstSegment, ".") {
+ // std
+ // (First segment is a dir beneath GOROOT/src, not a module domain name.)
+ return fmt.Sprintf("https://cs.opensource.google/go/go/+/%s:src/%s;l=%d",
+ info.GoVersion, fileline.file, linenum)
+ }
+
+ if rest, ok := strings.CutPrefix(fileline.file, "golang.org/x/tools"); ok {
+ // in x/tools repo (tools or gopls module)
+ if rest[0] == '/' {
+ // "golang.org/x/tools/gopls" -> "gopls"
+ rest = rest[1:]
+ } else if rest[0] == '@' {
+ // "golang.org/x/tools@version/dir/file.go" -> "dir/file.go"
+ rest = rest[strings.Index(rest, "/")+1:]
+ }
+
+ return fmt.Sprintf("https://cs.opensource.google/go/x/tools/+/%s:%s;l=%d",
+ "gopls/"+info.Version, rest, linenum)
+ }
+
+ // TODO(adonovan): support other module dependencies of gopls.
+ log.Printf("no CodeSearch URL for %q (%s:%d)",
+ symbol, fileline.file, linenum)
+ return ""
+}
+
// -- GitHub search --
// searchIssues queries the GitHub issue tracker.
@@ -504,6 +507,7 @@
if !fileExists(revDir) {
log.Printf("cloning tools@gopls/%s", info.Version)
if err := shallowClone(revDir, "https://go.googlesource.com/tools", "gopls/"+info.Version); err != nil {
+ _ = os.RemoveAll(revDir) // ignore errors
return nil, fmt.Errorf("clone: %v", err)
}
}
