raven-go icon indicating copy to clipboard operation
raven-go copied to clipboard

avoid using go/build to trim prefix

Open benesch opened this issue 7 years ago • 4 comments

go/build will return gopath/goroot paths for the machine running it, which isn't what you want -- the path being stripped are from the build machine. a dumber approach, just looking for /src/ does almost the same thing without using go/build.

Submitted on behalf of @dt.

benesch avatar Jun 05 '17 20:06 benesch

Is this guaranteed to be true? I feel like there'd be cases where "/src/" doesn't exist in the path.

mattrobenolt avatar Jun 05 '17 20:06 mattrobenolt

hm pretty sure anything that was in GOPATH will have src: https://github.com/golang/go/wiki/GOPATH#directory-layout

if you somehow got Go to build something that wasn't in GOPATH, well then trimming GOPATH off it wouldn't work anyway.

That said, either way, the current code using go/build is getting the executing machine's values for GOPATH / GOROOT, not the ones from the machine used to build and thus that are in the stacks (except in the special case of running on the build machine), so I think even with its limitations, this naive heuristic is hopefully still a bit more reliable/portable than using go/build.

dt avatar Jun 05 '17 20:06 dt

Actually, thinking about this more, I think this patch makes things worse. Trimming from the last /src/ is going to break whenever a Go package uses an internal src directory, e.g. github.com/cockroachdb/cockroach/src/.

Seems to me we should just avoid trimming entirely. If users want to trim, they can do so with GOROOT_FINAL and/or

go build -gcflags=-trimpath=$GOPATH -asmflags=-trimpath=$GOPATH

(I'm not necessarily suggesting that the official go-raven library default to no-trimming, but would be nice to have it exposed as an option. Thoughts, @dt @tamird?)

benesch avatar Aug 04 '17 19:08 benesch

TIL about trimpath; indeed, path trimming is fraught. It's too bad you can't ask for the GOROOT/GOPATH that were built into the binary at runtime. +1 to removing this functionality altogether.

tamird avatar Aug 04 '17 21:08 tamird