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
Milestone

Comments

@griesemer
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

griesemer 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
Copy link
Member

odeke-em commented Dec 2, 2016

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

@griesemer
Copy link
Contributor Author

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

@minux
Copy link
Member

minux commented Dec 2, 2016 via email

@davecheney
Copy link
Contributor

davecheney commented Dec 2, 2016 via email

akshayjshah pushed a commit to uber-go/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.

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.
Projects
None yet
Development

No branches or pull requests

6 participants