Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: inconsistent filename separators between Unix/Windows with runtime.Caller #18151

Closed
griesemer opened this issue Dec 1, 2016 · 6 comments

Comments

Projects
None yet
6 participants
@griesemer
Copy link
Contributor

commented Dec 1, 2016

While writing the test code for another issue (https://go-review.googlesource.com/#/c/33799/), I noticed that filenames defined via //line filename:line directives when returned from runtime.Caller always contained '/' (forward) slashes on Windows, even if the filename in the directive used '\' (backward) slashes. Both these line directives:

//line c:\foo\bar.go:987
//line c:/foo/bar.go:987

return "c:/foo/bar.go" when followed by a runtime.Caller(0) call on Windows. On Unix, the filename reported matches the filename in the directive ( https://play.golang.org/p/7E_bDYfhy4 ).

There's an inconsistency somewhere that should be addressed.

@griesemer griesemer added this to the Go1.9 milestone Dec 1, 2016

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Dec 1, 2016

I noticed that filenames defined via //line filename:line directives when returned from runtime.Caller always contained '/' (forward) slashes on Windows

This is purely historical.

All code written in Go always produced file paths delimited by \ and happily accepted paths delimited by both \ and /. But paths read by runtime.Caller are generated by cmd/link, and cmd/link was written in C. Originally cmd/link code was for non-windows paths, like /a/b. Windows is quite OK with these paths, so some people just put drive letter in front, like c:/a/b, and that was that.

It is, probably, quite easy to change cmd/link to use \. I just wonder who will be affected.

Alex

@griesemer

This comment has been minimized.

Copy link
Contributor Author

commented Dec 2, 2016

@alexbrainman Thanks for the background. I'm not surprised that this is simply a historical "accident".

However, it does lead to surprising (and inconsistent) behavior across platforms, which is why I think we should address it at some point.

@odeke-em

This comment has been minimized.

Copy link
Member

commented Dec 2, 2016

@griesemer duplicate of #3335 that has existed for the last 4 years.

@griesemer

This comment has been minimized.

Copy link
Contributor Author

commented Dec 2, 2016

@odeke-em Thanks for tracking this down! Closing as duplicate.

@minux

This comment has been minimized.

Copy link
Member

commented Dec 2, 2016

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2016

akshayjshah added a commit to uber-go/zap that referenced this issue Mar 23, 2017

Make caller path trimming Windows-compatible (#385)
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

@golang golang locked and limited conversation to collaborators Dec 2, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.