Fix #4116. Make dirname aware of the file system's path separator

[jsyeo]

I removed the line that normalizes the path to be separated by / and instead make it aware of the file system's separator.

[enebo]

@jsyeo This change will break windows compatibility but fix non-windows. On windows both / and \ will work in dirname (it is how MRI works anyways) whereas on all other platforms it treats \ as literalizing character. So I think maybe the fix might be to just put an if check on the original gsub to convert \ to / if only on windows.

[headius]

@enebo I don't think this will break anything on Windows, since the backslashes are not going to be interpreted as escapes (they're literally in the string, not syntactically inside a literal string).

However it may break other things by returning backslashes in the resulting path. I think MRI normalizes everything to / internally, right? Does any core MRI File method return a backslashy path on Windows?

@jsyeo Thank you for the PR! Are you on Windows or a UNIX system?

[jsyeo]

@jsyeo Thank you for the PR! Are you on Windows or a UNIX system?

I'm on an unix system. Hmm, I kinda assumed that it would be the inverse on a windows system. Let me spin up a windows vm and install mri to try out its File.dirname behavior.

[danini-the-panini]

The MRI implementation does pretty much what it says in the documentation:

Returns all components of the filename given in file_name except the last one. The filename can be formed using both File::SEPARATOR and File::ALT_SEPARATOR as the separator when File::ALT_SEPARATOR is not nil.

File::SEPARATOR is always /. However, File::ALT_SEPARATOR is nil on Unix and \ on Windows. The net result is that on Windows, either separator can be used interchangeably, but on Unix, only / is recognised.

I started working on a solution to #4116 for a while now, but I couldn't test on windows since I had some issues with Maven on my VM. Now that they've been fixed I've managed to put together what I believe to be a solid fix for both Unix and Windows environments. You can examine my proposed changes here: #4177.

[enebo]

@jellymann thanks for checking on the details of this. You gave a clearer explanation than me of the issue. I see there is a new PR to fix this so we will likely have a golden PR now :)

@headius I only pointed this out since I tested it on windows and it returns using either \ and / as a delimeter (which fits @jellymann desciption as to why). I think we can maybe even make a nice test for this in spec which adds ALT_SEPARATOR so we can test windows behavior on non-windows ci runs too.

Duplcating as there is a new PR now.