-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
path.posix.relative returns different results for *nix and Windows versions of node #13683
Comments
On Windows, Node.js versions differ: // Node.js 4.8.3 (v8 4.5.103.47) x64
../../../.././x
// Node.js 6.11.0 (v8 5.1.281.102) x64
../../../..../x
// Node.js 8.1.1 (v8 5.8.283.41) x64
../../../..../x |
I will try to fix it. |
After reading the code found |
The first solution that comes to mind is adding a check (re #13714): |
@DuanPengfei you may create a new PR to check and throw error in posix.resolve. |
👌🏻I will create a new PR. |
@refack Throwing an error instead of fixing the bug seems like a really bad idea; I think there is no reason inherent to the API why |
I am a little surprised as well, I didn't follow the conversation, but I wonder why we do not even consider fixing the bug instead of just saying "you cannot use this function on Windows". Did I miss something? Is this really too difficult to implement? How is |
@XadillaX I understand that; but that should work just fine on Windows, no? Also, there’s not always a missing part, for example if you reverse the parameters in your code snippet. |
if the correct result may be implemented fine, I think to implement is better too. |
If use different systems pwd can produce correct result, of course, that is better, I will try it again later. Create PR because I think use |
Even if it is not, that is not a reason to prevent Windows applications from dealing with posix paths and vice versa. For example, you might want to talk to a posix FTP server from a Windows machine (in which case paths shouldn't be resolved against |
So is that means |
I'm not sure which thread is better to comment in, this or #13738 🤔 P.S. The |
In that case I would recommend using URL. But there are obviously cases where it's useful to do cross-platform path manipulation, but IMHO we should limit the nonsense outputs, like when > path.posix.resolve('./gaga/')
'C:\\bin\\dev\\node/gaga' // bug
> path.posix.resolve('/gaga/')
'/gaga' // Ok |
Throw an error `ERR_UNSUPPORTED_PLATFOMR` when direct use `path.posix.resolve` on Windows or direct use `path.win32.resolve` on *nix. Update docs, list win32 functions do not support direct use on *nix and posix functions do not support direct use on Windows. Update tests, only run current platform type test. Fixes: nodejs#13683 Refs: nodejs#13714
Add a known_issues test for a known Windows issue. Refs: nodejs#13683
Add a known_issues test for a known Windows issue. Refs: nodejs#13683
Add a known_issues test for a known Windows issue. Refs: nodejs#13683
I know this issue is quite old, but if anyone wants to test out #37747 to see if it fixes the issue for them, that would be great. |
Add a known_issues test for a known Windows issue. Refs: nodejs#13683 PR-URL: nodejs#37744 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
This code returns different values depending on whether run on *nix or windows:
The text was updated successfully, but these errors were encountered: