EntryCaller.TrimmedPath() does not work correctly on Windows #382
Comments
|
Thank you for the excellent bug report! Yes, I'd love a PR to fix this - the simple solution you've outlined sounds correct. Please include links to the two issues you mentioned here in a comment in the code, so that future readers don't assume that we've neglected Windows support. |
moitias
added a commit
to moitias/zap
that referenced
this issue
Mar 23, 2017
Go stdlib runtime.Caller() currently returns forward slashes on Windows (see golang/go#3335) which causes EntryCaller.TrimmedPath() to return full paths instead of the expected trimmed paths on Windows. This is because EntryCaller.TrimmedPath() uses os.PathSeparator to trim the path which is '\' on Windows. According to the discussion on the Go bug, it seems like os.PathSeparator might be '' in some cases on Unix too so might cause issues on non-Windows platforms too. This PR replaces the two occurrences of os.PathSeparator with ''/' as that is what runtime.Caller() currently produces on all platforms. Fixes: uber-go#382 See also: golango/go#18151
4 tasks
akshayjshah
pushed a commit
that referenced
this issue
Mar 23, 2017
Go stdlib runtime.Caller() currently returns forward slashes on Windows (see golang/go#3335) which causes EntryCaller.TrimmedPath() to return full paths instead of the expected trimmed paths on Windows. This is because EntryCaller.TrimmedPath() uses os.PathSeparator to trim the path which is '' on Windows. According to the discussion on the Go bug, it seems like os.PathSeparator might be '' in some cases on Unix too so might cause issues on non-Windows platforms too. This PR replaces the two occurrences of os.PathSeparator with ''/' as that is what runtime.Caller() currently produces on all platforms. Did not add tests, as the existing tests for TrimmedPath() failed on Windows with master and work with this PR. Fixes: #382 See also: golang/go#18151
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When using development configuration on Windows, complete file paths are being shown instead of the trimmed ones that are expected and documented.
This is due to runtime.Caller() returning paths with forward slashes instead of os.PathSeparator (because of historic reasons, it seems. See golang/go#3335 for discussion) and TrimmedPath expecting os.PathSeparator and failing with the complete path when it can't find os.PathSeparator in the string.
It's essentially a Go bug and it's not immediately clear how this will be handled in the future but it seems like the Go team is inching towards 'forward slashes everywhere' (see golang/go#18151 for a duplicate bug with some more discussion).
The fix seems simple; just replace the two uses of os.PathSeparator in EntryCaller.TrimmedPath with '/'). This doesn't looke like a major change as it seems like the only call to NewEntryCaller (that passes the runtime.Caller() information to be stored in EntryCaller.File) is in Logger.check and as far as I can see, EntryCaller.File doesn't get modified from anywhere else either (except in testing).
Would be happy to make a PR to fix it if it seems like a reasonable fix - I'm not at all familiar with zap internals so not sure if it would have potential side-effects and/or break compatibility. I guess someone might be depending on the buggy behaviour?
The text was updated successfully, but these errors were encountered: