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

Comments

Projects
None yet
9 participants
@gopherbot

gopherbot commented Mar 16, 2012

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

This comment has been minimized.

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

This comment has been minimized.

gopherbot commented Mar 17, 2012

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

This comment has been minimized.

Member

dsymonds commented Mar 17, 2012

Comment 3:

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

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Mar 18, 2012

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

This comment has been minimized.

gopherbot commented Mar 18, 2012

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

This comment has been minimized.

gopherbot commented Mar 19, 2012

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

rsc commented Sep 12, 2012

Comment 8:

Labels changed: added go1.1maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Mar 12, 2013

Comment 9:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@rsc

This comment has been minimized.

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

This comment has been minimized.

Contributor

robpike commented Aug 20, 2013

Comment 11:

Labels changed: removed go1.3.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 27, 2013

Comment 12:

Labels changed: added go1.3maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 4, 2013

Comment 13:

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

@rsc

This comment has been minimized.

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 from runtime: Caller returns only Unix-specific path separator in file path to runtime: runtime.Caller returns always '/' separator in file paths on Windows Dec 2, 2016

@griesemer

This comment has been minimized.

Contributor

griesemer commented Dec 2, 2016

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

Use forward slashes for TrimmedPath() instead of os.PathSeparator
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

@moitias moitias referenced this issue Mar 23, 2017

Merged

Make caller path trimming Windows-compatible #385

2 of 4 tasks complete

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

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017

johnSchnake added a commit to apcera/logray that referenced this issue Oct 9, 2017

Use correct forward slashes when dealing with callers path
The filename returned from runtime.Caller is forward slash
delimited which has had an open issue for years:
golang/go#3335

This changes makes it such that the file:line is reported
properly instead of the full-path:line which is the current
behavior on Windows. When dealing with small log lines,
that long path can represent a sizable percentage of the
total size of the log file.
@ChrisHines

This comment has been minimized.

Contributor

ChrisHines commented Nov 12, 2017

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