avoid using go/build to trim prefix #139
Conversation
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.
|
Is this guaranteed to be true? I feel like there'd be cases where "/src/" doesn't exist in the path. |
|
hm pretty sure anything that was in GOPATH will have if you somehow got Go to build something that wasn't in GOPATH, well then trimming That said, either way, the current code using |
| prefix += string(filepath.Separator) | ||
| } | ||
| trimPaths = append(trimPaths, prefix) | ||
| const src = "/src/" |
There was a problem hiding this comment.
should this use os.PathSeparator (or filepath.Join, equivalently) instead of hardcoding /?
There was a problem hiding this comment.
Welp, what if you're running a binary cross-compiled on Linux for Windows? My point is that I think neither the current behavior nor your proposed alternative is correct. Perhaps we need to look for both /src/ and \src\ on all platforms?
There was a problem hiding this comment.
Indeed. In fact, we're completely speculating on this behaviour at the moment. We probably need to experiment (or find the code) to find out how source locations are encoded into Go binaries. See golang/go#19784, for instance, which implies some confusion in this area.
There was a problem hiding this comment.
that said, even if /src/ doesn't work for some platform or cross-compilation combinations, it should still probably do better than go/build's user-specific paths, right? so does it make sense to call this an incremental improvement even if there's potentially further we could go?
| var trimPaths []string | ||
|
|
||
| // Try to trim the GOROOT or GOPATH prefix off of a filename | ||
| // trimPath tries to trim the GOROOT or GOPATH prefix off of a filename. |
There was a problem hiding this comment.
I think this comment is now irrelevant, right? Since we don't look at these values at all.
|
Actually, thinking about this more, I think this patch makes things worse. Trimming from the last Seems to me we should just avoid trimming entirely. If users want to trim, they can do so with (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?) |
|
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. |
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.