Rename std::path to std::old_path; introduce new std::path#21759
Rename std::path to std::old_path; introduce new std::path#21759bors merged 2 commits intorust-lang:masterfrom
std::path to std::old_path; introduce new std::path#21759Conversation
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
If we have a path C:foo.txt this is a relative path, not an absolute path, so it doesn't make sense to replace self outright.
There was a problem hiding this comment.
Consider pushing C:foo.txt onto D:\temp. What would you want to happen?
The existing path API appends the path only for drive prefixes where the volumes match. I wrote some code to do the same initially, but ultimately felt that it was inconsistent behavior, rather than always treating pushing C:foo.txt as a drive-CWD-relative path. But I could be convinced otherwise! The tradeoffs aren't completely clear to me.
There was a problem hiding this comment.
Update: based on an IRC chat with @retep998, it's unclear that drive-CWD is available in winapi (though it is within cmd). Also, PathCchCombineEx (from https://msdn.microsoft.com/en-us/library/windows/desktop/hh707086%28v=vs.85%29.aspx) behaves identically to the API proposed here.
I'm somewhat uncomfortable with pushing C:foo.txt onto D:\temp resulting in C:\temp\foo.txt, but I really don't know in what contexts this situation is likely to come up (and what you'd want it to do for those).
There was a problem hiding this comment.
As it turns out, Windows does have drive specific current directories. Unfortunately I cannot find a way to directly get the CWD of a specific drive. Also the CWD is a very racy thing that is shared across threads and may change values on you between calls.
Since methods in winapi treat C:foo.txt as an absolute path when joining, we should replicate that behavior.
Any method that actually interacts with the filesystem and is given a C:foo.txt will treat it as a path relative to the CWD on that drive. Therefore when normalizing the path based on the filesystem we should probably use GetFullPathNameW to get the absolute path. The only other method I can think of involves changing the CWD to the drive of that path, and then getting the CWD, which is undesirable because it changes the CWD for other threads.
src/libstd/path.rs
Outdated
There was a problem hiding this comment.
Not sure if I'll see something down below, but are paths like C:\foo and c:\foo equivalent? If so, does this derived implementation handle that?
There was a problem hiding this comment.
Ah it looks like PartialEq is on Component below, but also not case-insensitive, just confirming that is desired.
There was a problem hiding this comment.
Unfortunately case sensitivity is very hard for us to do right, because the case sensitivity tables are stored in the partition itself. This means that we cannot determine whether two paths are equal if they differ in case unless we ask the filesystem to resolve the two paths for us.
However, we can at least assume that the drive letter is always case insensitive so C:\foo and c:\foo should always be considered equal.
There was a problem hiding this comment.
Ah yeah I would only consider the drive letter to be considered for case insensitivity, not the actual components of the path itself. Thanks for clarifying!
This PR implements path reform, and motivation and details for the change can be found there.
For convenience, the old path API is being kept as
old_pathfor the time being. Updating after this PR is just a matter of changing imports toold_path(which is likely not needed, since the prelude entries still export the old path API).This initial PR does not include additional normalization or platform-specific path extensions. These will be done in follow up commits or PRs.
[breaking-change]
Closes #20034
Closes #12056
Closes #11594
Closes #14028
Closes #14049
Closes #10035