search

go-stack / stack

Package stack implements utilities to capture, manipulate, and format call stacks.
MIT License
395 stars 33 forks source link

Fix ancestor confusion in gopath / goroot #5

Closed Syntaxide closed 7 years ago

Syntaxide commented 7 years ago

inGoroot uses strings.HasPrefix to determine whether or not a file is inside the goroot. However, this doesn't work well if the goroot is actually a prefix of the gopath.

For example, on my machine, I use: GOROOT=/home/alex/go GOPATH=/home/alex/gopath

Since all the code within gopath has GOROOT as a prefix, TrimRuntime() produces empty stack traces. This PR fixes this by ensuring GOROOT ends in a trailing /

Testing is tricky, since Callstacks are lists which contain runtime.Func objects, which cannot be created for testing. This could be remedied by having Call contain an interface instead of a runtime.Func. I'm not sure this is worthwhile for such a small change.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 89.268% when pulling 334475e7973746e855357edcbb7236944dbc70d7 on Syntaxide:master into 100eb0c0a9c5b306ca2fb4f165df21d80ada4b82 on go-stack:master.

ChrisHines commented 7 years ago

I think there is something going on here that I don't fully understand. I have Go installed at /usr/local/go. With the current code on the master branch after the init function completes runtimePath contains /usr/local/go/src/. Note that it already ends in a / and also that it contains the additional path segment of src.

If I understand your environment correctly then for you runtimePath should contain /home/alex/go/src/, which should not be a prefix of anything in your GOPATH which should all be under /home/alex/gopath/src/.

Also note that proposed fix of adding the following code at the end of func init may not work as intended.

runtimePath = path.Join(runtimePath, "/")

path.Join calls path.Clean before returning. path.Clean removes trailing slashes if the path is not /. For example.

fmt.Println(path.Join("/home/alex/go/src/", "/"))

writes out: /home/alex/go/src. (See https://play.golang.org/p/wBS4LnY3RB).

It seems to me like there must be another explanation for why you are seeing empty stack traces and I think the fix is not likely the one you have submitted. Perhaps there is a detail I haven't understood or we need to do some more testing to figure out what is not working as expected.

The most useful next step is likely for you to add this code to the bottom of stackinternal_test.go (from master):

func TestRuntimePath(t *testing.T) {
    t.Fatal(runtimePath)
}

Then please run go test github.com/go-stack/stack and report back the output and we'll go from there.

ChrisHines commented 7 years ago

@Syntaxide, have you had any time to investigate this further?

Syntaxide commented 7 years ago

Not really. When printing out the runtimePath, I confirmed that there were other elements in the path beyond my $GOPATH var. So it seems to be more than just a prefix issue. However, moving my GOPATH resolved the issue, so I haven't spent more time investigating.