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: runtime.Caller returns always '/' separator in file paths on Windows #3335

Open
gopherbot opened this issue Mar 16, 2012 · 16 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. OS-Windows
Milestone

Comments

@gopherbot
Copy link

by pkorotkov:

What steps will reproduce the problem?
1. Call function runtime.Caller(...)
2. Take the file path (second argument)

What is the expected output?
On Windows path separator is "\"

What do you see instead?
"/"

Which compiler are you using (5g, 6g, 8g, gccgo)?
8g

Which operating system are you using?
Windows 7, x86

Which revision are you using?  (hg identify)
0b67257c702f tip
@minux
Copy link
Member

minux commented Mar 17, 2012

Comment 1:

Windows will accept both slash and backslash as path separator, so I think
this is a non-issue.
Leave it to others to decide.

@gopherbot
Copy link
Author

Comment 2 by pkorotkov:

It's an issue if your business logic for the path is based on the assumption that one
has OS-specific path separators (os.PathSeparator); that's my case. Despite that you can
easily work around it's a bug anyway.

@dsymonds
Copy link
Contributor

Comment 3:

Labels changed: added priority-go1, packagebug, removed priority-triage.

@alexbrainman
Copy link
Member

Comment 4:

Filenames returned by runtime.Caller do have / as path delimiter. They are extacted from
symtab of compiled packages. They are written by compiler when it compiles package.
As far as I remember, compiler rewrites all \ into / because all our runtime procedures
expect / as path delimiter. I am not sure it is safe to change compiler to leave \ alone.
Alternatively, we could change runtime.Caller to rewrite all / into \ if running on
windows. I am not sure what the implications are. It could also be misleading sometimes,
if, for example, some packages where compiled on unix and then copied onto windows.
/home/my/file.go would then become \home\my\file.go.
Alex

@gopherbot
Copy link
Author

Comment 5 by pkorotkov:

Alex, your example of misleading paths is valid only in a cross-compiling case. I see
two obvious options:
1. Leave it as is and add a relevant comment to the func's spec;
2. Force all path separators to be replaced with OS-specific ones indicating such
behavior in the func's spec as well. If source code is available, it's correct 'ab
incunabulis'. If not - file was cross-compiled - you just need to do little extra work
to get right paths.
Maybe, it would be nice to have automatic src-detection in this context... don't know. 
In either case, current version of function description lacks for clarifications.

@gopherbot
Copy link
Author

Comment 6 by pkorotkov:

Alex, your example of misleading paths is valid only in cross-compiling case. I see two
obvious options:
1. Leave it "as is" and add a relevant comment to the func's spec;
2. Force all path separators to be replaced with OS-specific ones indicating such
behavior in the func's spec as well. If source code is available, it's correct 'ab
incunabulis'. If not - file was cross-compiled - you just need to do little extra work
to get the right paths.
Maybe, it would be nice to have automatic src-detection in this context... don't know. 
In either case, the current version of the function description lacks for clarifications.

@rsc
Copy link
Contributor

rsc commented Mar 26, 2012

Comment 7:

We can revisit this after Go 1.

Labels changed: added priority-later, removed priority-go1.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 8:

Labels changed: added go1.1maybe.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 9:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 10:

I think as part of Go 1.3 we will change the way line numbers are sent out of the
compilers, and they will record the full OS path. Then everything will work as expected.

Labels changed: added go1.3.

@robpike
Copy link
Contributor

robpike commented Aug 20, 2013

Comment 11:

Labels changed: removed go1.3.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 12:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 13:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 14:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@griesemer griesemer modified the milestones: Go1.9Maybe, Unplanned Dec 2, 2016
@griesemer griesemer changed the title runtime: Caller returns only Unix-specific path separator in file path runtime: runtime.Caller returns always '/' separator in file paths on Windows Dec 2, 2016
@griesemer
Copy link
Contributor

See also #18151. On Unix, the file path separator is either '/' or '' depending on how the filename got set in the first place.

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
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
@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017
@bradfitz bradfitz added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. OS-Windows labels Jul 20, 2017
@ChrisHines
Copy link
Contributor

My preference here is to simply document the current behavior of always using '/' in stack frame data. That's the way it's been implemented for years, and packages like github.com/go-stack/stack that have been stable for years expect it. Changing the behavior in a future version of Go will break working code without a clearly documented benefit. Documenting the behavior will help avoid new code from making the wrong assumptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. OS-Windows
Projects
None yet
Development

No branches or pull requests

9 participants