Merge #65899

65899: util/caller: simplify, make the code compatible with bazel r=tbg,rickystewart a=knz

Fixes  #61913.

The bazel sandbox places source files into a different directory
structure than a regular `go build` or `go get`.  We need to
accommodate for this.

Additionally, this change converts the code to use
`runtime.CallersFrames`, which simplifies the extraction of the crdb
package name, which in turns greatly simplifies the computation of the
`defaultRE` regular expression.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>

pkg/util/caller/BUILD.bazel

@@ -16,6 +16,5 @@ go_test(
         "resolver_test.go",
     ],
     embed = [":caller"],
-    tags = ["broken_in_bazel"],
     deps = ["//pkg/util/log"],
 )

pkg/util/caller/resolver.go

@@ -11,6 +11,7 @@
 package caller
 
 import (
+	"fmt"
 	"path"
 	"regexp"
 	"runtime"
@@ -39,40 +40,137 @@ type CallResolver struct {
 
 var reStripNothing = regexp.MustCompile(`^$`)
 
-// defaultRE strips src/github.com/org/project/(pkg/)module/submodule/file.go
-// down to module/submodule/file.go. It falls back to stripping nothing when
-// it's unable to look up its own location via runtime.Caller().
-var defaultRE = func() *regexp.Regexp {
-	_, file, _, ok := runtime.Caller(0)
-	if !ok {
-		return reStripNothing
+// findFileAndPackageRoot identifies separately the package path to
+// the crdb source tree relative to the package build root, and the
+// package build root. This information is needed to construct
+// defaultRE below.
+//
+// For example:
+//
+//     /home/kena/src/go/src/github.com/cockroachdb/cockroach/pkg/util/caller
+//                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ crdb package path
+//     ^^^^^^^^^^^^^^^^^^^^^^ package build root
+//
+// Within a Bazel sandbox:
+//
+//     github.com/cockroachdb/cockroach/pkg/util/caller
+//     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ crdb package path
+//     (there is no package build root in this case)
+//
+//
+// The first return value is false if the paths could not be
+// determined.
+func findFileAndPackageRoot() (ok bool, crdbPath string, srcRoot string) {
+	pcs := make([]uintptr, 1)
+	if runtime.Callers(1, pcs[:]) < 1 {
+		return false, "", ""
 	}
-	const sep = "/"
-	root := path.Dir(file)
+	frame, _ := runtime.CallersFrames(pcs).Next()
+
+	// frame.Function is the name of the function prefixed by its
+	// *symbolic* package path.
+	// For example:
+	//     github.com/cockroachdb/cockroach/pkg/util/caller.findFileAndPackageRoot
+	funcName := frame.Function
+
+	crdbPath = strings.TrimSuffix(funcName, ".findFileAndPackageRoot")
+
+	// frame.File is the name of the file on the filesystem.
+	// For example:
+	//   /home/kena/src/go/src/github.com/cockroachdb/cockroach/pkg/util/caller/resolver.go
+	//
+	// (or, in a Bazel sandbox)
+	//   github.com/cockroachdb/cockroach/pkg/util/caller/resolver.go
+	//
+	// root is its immediate parent directory.
+	root := path.Dir(frame.File)
+
 	// Coverage tests report back as `[...]/util/caller/_test/_obj_test`;
 	// strip back to this package's directory.
-	for strings.Contains(root, sep) && !strings.HasSuffix(root, "caller") {
+	if !strings.HasSuffix(root, "/caller") {
+		// This trims the last component.
 		root = path.Dir(root)
 	}
-	// Strip to $GOPATH/src.
-	for i := 0; i < 6; i++ {
-		root = path.Dir(root)
+
+	if !strings.HasSuffix(root, "/caller") {
+		// If we are not finding the current package in the path, this is
+		// indicative of a bug in this code; either:
+		//
+		// - the name of the function was changed without updating the TrimSuffix
+		//   call above.
+		// - the package was renamed without updating the two HasSuffix calls
+		//   above.
+		panic(fmt.Sprintf("cannot find self package: expected .../caller, got %q", root))
+	}
+
+	if !strings.HasSuffix(root, crdbPath) {
+		// We require the logical package name to be included in the
+		// physical file name.
+		//
+		// Conceptually, this requirement could be violated if e.g. the
+		// filesystem used different path separators as the logical
+		// package paths.
+		//
+		// However, as of Go 1.16, the stack frame code normalizes
+		// paths to use the same delimiters.
+		// If this ever changes, we'll need to update this logic.
+		panic(fmt.Sprintf("cannot find crdb path (%q) inside file path (%q)", crdbPath, root))
 	}
-	qSep := regexp.QuoteMeta(sep)
-	// Part of the regexp that matches `/github.com/username/reponame/(pkg/)`.
-	pkgStrip := qSep + strings.Repeat(strings.Join([]string{"[^", "]+", ""}, qSep), 3) + "(?:pkg/)?(.*)"
-	if !strings.Contains(root, sep) {
-		// This is again the unusual case above. The actual callsites will have
-		// a "real" caller, so now we don't exactly know what to strip; going
-		// up to the rightmost "src" directory will be correct unless someone
-		// creates packages inside of a "src" directory within their GOPATH.
-		return regexp.MustCompile(".*" + qSep + "src" + pkgStrip)
+
+	// The package build root is everything before the package path.
+	srcRoot = strings.TrimSuffix(root, crdbPath)
+
+	// For the crdb package root, rewind up to the `pkg` root
+	// and one up.
+	for {
+		if strings.HasSuffix(crdbPath, "/pkg") {
+			break
+		}
+		// Sanity check.
+		if crdbPath == "." {
+			// There was no "pkg" root.
+			panic(fmt.Sprintf("caller package is not located under pkg tree: %q", root))
+		}
+		crdbPath = path.Dir(crdbPath)
 	}
-	if !strings.HasSuffix(root, sep+"src") && !strings.HasSuffix(root, sep+"vendor") &&
-		!strings.HasSuffix(root, sep+"pkg/mod") {
-		panic("unable to find base path for default call resolver, got " + root)
+	crdbPath = path.Dir(crdbPath) // Also trim /pkg.
+
+	// At this point we have simplified:
+	//
+	//     /home/kena/src/go/src/github.com/cockroachdb/cockroach/pkg/util/caller/resolver.go
+	//     crdbPath: "github.com/cockroachdb/cockroach"
+	//     srcRoot: "/home/kena/src/go/src/"
+	//
+	// Within a Bazel sandbox:
+	//
+	//     github.com/cockroachdb/cockroach/pkg/util/caller
+	//     crdbPath: "github.com/cockroachdb/cockroach"
+	//     srcRoot: ""
+	//
+	return true, crdbPath, srcRoot
+}
+
+// defaultRE strips as follows:
+//
+// - <fileroot><crdbroot>/(pkg/)?module/submodule/file.go
+//   -> module/submodule/file.go
+//
+// - <fileroot><crdbroot>/vendor/<otherpkg>/path/to/file
+//   -> vendor/<otherpkg>/path/to/file
+//
+// - <fileroot><otherpkg>/path/to/file
+//   -> <otherpkg>/path/to/file
+//
+// It falls back to stripping nothing when it's unable to look up its
+// own location via runtime.Caller().
+var defaultRE = func() *regexp.Regexp {
+	ok, crdbRoot, fileRoot := findFileAndPackageRoot()
+	if !ok {
+		return reStripNothing
 	}
-	return regexp.MustCompile(regexp.QuoteMeta(root) + pkgStrip)
+
+	pkgStrip := regexp.QuoteMeta(fileRoot) + "(?:" + regexp.QuoteMeta(crdbRoot) + "/)?(?:pkg/)?(.*)"
+	return regexp.MustCompile(pkgStrip)
 }()
 
 var defaultCallResolver = NewCallResolver(defaultRE)

pkg/util/log/BUILD.bazel

@@ -138,7 +138,6 @@ go_test(
     ],
     data = glob(["testdata/**"]),
     embed = [":log"],
-    tags = ["broken_in_bazel"],
     deps = [
         "//pkg/cli/exit",
         "//pkg/settings/cluster",