on Windows, strip the '\\?\' prefix from the canonical path #1471
Conversation
|
@bors r+ |
|
📌 Commit 7d9d74e has been approved by |
|
☀️ Test successful - checks-travis, status-appveyor |
| let path = path.canonicalize().unwrap_or(path); | ||
|
|
||
| // On Windows, this produces a path starting with `\\?\`, which xargo cannot deal with. | ||
| // Strip that prefix; the resulting path should still be valid. |
There was a problem hiding this comment.
If the path is more than 260 characters long, the \\?\ is required See the "Maximum Path Length Limitation" section of https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
There was a problem hiding this comment.
Oh man, Windows is hard to handle.^^
I think this should be a good enough hack to land the Miri fix in rustc now. It anyway only affetcs setups that set XARGO_RUST_SRC which should be rare.
Once Miri is green in rustc, we don't have to rush any more. Then I'll release a new xargo with japaric/xargo#297 and remove the hack here in Miri. @bjorn3 do you have a Windows system you could test this on?
There was a problem hiding this comment.
No I have switched to Linux a long time ago. Wine may help with testing though.
There was a problem hiding this comment.
Drive by comment, but it seems to me that we only use canonicalize here to go from relative path to an absolute one. If that is the case, I suggest using std::env::current_dir().unwrap().join(val) as a better alternative. Note that join auto-magically handles abs path in a convenient way (= is susceptible to the directory traversal attack :0) )
There was a problem hiding this comment.
@matklad that's a great idea, I like it. :)
There was a problem hiding this comment.
Note that std::env::current_dir().unwrap().join(val) is also incorrect on Windows, as drive relative paths are a thing (aka C:foo is relative to the current directory on that drive).
There was a problem hiding this comment.
aka C:foo is relative to the current directory on that drive
I don't even know what that means, and I am not sure I want to.^^
This should help with rust-lang/rust#74146 (comment)