Add assumptions for resolveWindows, resolvePosixand realpath
#10988
Conversation
|
This was quite abit surprising behavior on figuring out the problem with workarounds of matu3ba/chepa#1 |
|
Is the assumption for Both
|
|
I'm not sure the realpath description is accurate either, or maybe I'm just misunderstanding it.
realpath("lib") = "/home/topolarity/repos/zig/lib"
realpath("lib/..") = "/home/topolarity/repos/zig"
realpath("build.zig") = "/home/topolarity/repos/zig/build.zig"
realpath("build.zig/..") = error.NotDirIs the comment pointing out that "build.zig/.." fails, or are you hitting a different problem? |
Yes. Mostly its about pointing out that "build.zig/.." fails. |
|
I don't personally find that behavior surprising. However, I do see how that could depend on one's familiarity with POSIX I don't think the comment needs to mention the root path at all - Maybe clearer phrasing would be:
|
matu3ba
force-pushed
the
resolve
branch
2 times, most recently
from
f6a51ec to
a02f03d
Compare
|
@topolarity Do you think this good enough? +/// Note: All /-separated components of the supplied path must resolve to a
+/// directory, or a symlink to a directory, except for the last component
+/// Use `fs.path.resolve` or `fs.path.relative` to get dir of file (`file/..`).
+/// assume: if relative path used, then they are relative to `getcwd()` |
|
Looking better 👍 Last nit is that I think we should either mention that FWIW, the reason file-relative paths work on Windows is that symlinks and directories are resolved after ".." is resolved (roughly equivalent to doing path.resolve, followed by realpath). However, this changes how symlinks interact with WASI is tackling the same behavioral differences in WebAssembly/wasi-filesystem#26 and so far seems to be leaning towards leaving this up to the Host OS |
* doc `realpath`: specify requirements and hint to get dir of file * doc `resolve`: behavior difference on Windows and Linux + consequence
| @@ -468,6 +468,10 @@ fn asciiEqlIgnoreCase(s1: []const u8, s2: []const u8) bool { | |||
| } | |||
|
|
|||
| /// On Windows, this calls `resolveWindows` and on POSIX it calls `resolvePosix`. | |||
| /// Note: Symlinks currently dont work and should not be used, | |||
| /// because Windows and Posix having different behavior. | |||
| /// On Windows, symlinks are resolved after `..` is resolved. | |||
There was a problem hiding this comment.
I think "Note: does not expand symlinks" is sufficient for the note here
The details on symlink resolution between platforms have to do with realpath and the other std.os calls, not this function
There was a problem hiding this comment.
I think "Note: does not expand symlinks" is sufficient for the note here
agreed.
The details on symlink resolution between platforms have to do with realpath and the other std.os calls, not this function
Why does this not belong into the description of the portable method, if it is a portability restriction?
I would expect that users tend to do tests on one platform with the most portable method and extrapolate to the others.
| @@ -5091,6 +5091,10 @@ pub const RealPathError = error{ | |||
| /// extra `/` characters in `pathname`. | |||
| /// The return value is a slice of `out_buffer`, but not necessarily from the beginning. | |||
| /// See also `realpathZ` and `realpathW`. | |||
| /// Note: All /-separated components of the supplied path must resolve to a | |||
| /// directory, or a symlink to a directory, except for the last component | |||
| /// Use `fs.path.resolve` or `fs.path.relative` to get dir of file (`file/..`). | |||
There was a problem hiding this comment.
realpath should fully resolve any path containing relative . and .. components on any POSIX-compliant system as far as I remember. It is only Windows that is the problem where the NT kernel call we issue is unable to deal with those which implies it is up to the user to resolve the relative components first.
There was a problem hiding this comment.
The non-working semantics (pathtofolder/file/..) of fs.path.relative can be seen here matu3ba/chepa@f34084c#diff-d924ca21d81d7d5a59eb9e10d3e2689c4c58a7e28e6ecce6a064701666f5a730L173
It is only Windows that is the problem where the NT kernel call we issue is unable to deal with those which implies it is up to the user to resolve the relative components first.
Could we on Windows work around the problem with calling relative on the subpath and then call resolve for each extending subpath? If you can point me to the things I need for testing, I can play abit to see if that could work.
Kind related is #4812.
|
The changes here are incorrect and unhelpful. I'll scrutinize these doc comments at some point, but at a glance this PR is not an improvement. |
realpathdescriptionresolveWindows,resolvePosixdo not work with relative paths