houseabsolute / Devel-StackTrace

An object representing a stack trace
https://metacpan.org/release/Devel-StackTrace/
Other
7 stars 13 forks source link

Filename normalization in trace object means the filename does not match perl internals #16

Open haarg opened 6 years ago

haarg commented 6 years ago

The filename in stack traces is normalized with File::Spec->canonpath. On unix systems, this will usually not change the reported file name. On Win32, it will usually change the directory separators. This makes the filename not match what perl thinks it is for the files, so it won't match things like __FILE__, %INC keys, internal error messages, or other uses of caller, including Carp. The normalization doesn't seem particularly useful to begin with in this case, so I think it may be better to remove it. There could be some backwards compatibility issues with changing it though.

autarch commented 6 years ago

Yeah, this was clearly a bad idea. According to git, I did it in 2005 but I didn't provide a reason in the commit message. Bad me.

lancew commented 6 years ago

Looking at this one for the CPAN PRC, removing $self->{filename} = File::Spec->canonpath( $self->{filename} ); This seems to work but causes one test to fail:

not ok 35 - filename is canonicalized
#   Failed test 'filename is canonicalized'
#   at t/01-basic.t line 338.
#          got: 'foo/bar///baz.pm'
#     expected: 'foo/bar/baz.pm'

This test is solely for Linux (else skipped) so is it ok to remove it?

autarch commented 6 years ago

My one concern is that this is a backwards incompatible change, which could break who knows what out there.

I think I'll have to do a dev release at the very least.

As for the test, I think it can be removed.