-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Fix issues with File.dirname #4177
Fix issues with File.dirname #4177
Conversation
@jellymann this PR nearly works but test_split for File#split uses dirname internally and these changes caused to specs to fail. Any chance you can take a look? |
@enebo I've had a look at the spec. Is there any reason why it specifically dictates that If If Removing the deviation from the It seems this deviation has been in there since the beginning of RubySpec |
@jellymann ok that is interesting. I have seen bad specs. If so we can tag it out in our tags file with Windows: (vs Fail: or Error:) then it will skip testing that spec on windows. I guess ultimately ruby/spec should get fixed then. |
We (or you) can fix the spec directly in our repo; @eregon periodically merges those changes across. |
@headius @enebo I took out the deviation in File.spec, and it is now failing because File.basename is also not taking system-specific path separators into account (it does the same as File.dirname did, just replacing all I will investigate MRI's behaviour and write tests for basename, and then fix the implementation. |
@jellymann sorry I did not merge this a long time ago. We shall see how it works. At the time you submitted this our CI was pretty messed up so I might ping you if something else breaks as part of fallout. |
This fixes #4116
e.g. The result of
File.dirname('/foo/bar\baz')
on Windows is"/foo/bar"
while on Unix it is
"/foo"
, since"bar\baz"
is a single file name (\
is not a path separator in Unix)As an added bonus, this also fixes a number of inconsistencies with MRI with regard to Windows UNC paths, notably:
A typical UNC path is made up of
"//<host>/<share>/path/to/file.ext"
, where the "root" is"//<host>/<share>"
This means that, while
File.dirname('//foo/bar/baz')
is"//foo/bar"
,File.dirname('//foo/bar')
is still"//foo/bar"
(since"//foo/bar"
is the "root").Extraneous "leading" separators are removed, e.g.
File.dirname('////foo/bar/baz')
is"//foo/bar"
This logic must not be confused with Unix paths, where there should be no more than 1 separator at the beginning of the string.
e.g.
File.dirname('/foo/bar')
should still be"/foo"
, on both Unix and Windows.For all of the above,
/
and\
are completely interchangeable at all times on Windows.Windows UNC paths are not recognised at all on Unix, e.g.
File.dirname('//foo/bar')
is still"/foo"
.