Skip to content
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

Installation of rustc to a RUSTUP_HOME specified with an extended-length path fails on Windows. #1647

Open
ecstatic-morse opened this issue Feb 8, 2019 · 8 comments
Labels
O-windows Windows related

Comments

@ecstatic-morse
Copy link

ecstatic-morse commented Feb 8, 2019

While trying to set up crater to run on Windows, I encountered the following error:

[2019-02-08T16:37:58Z INFO  crater::run] running `"C:\\Users\\mackendy\\AppData\\Local\\Temp\\.tmpBgcNDX\\rustup-init.exe" "-y" "--no-modify-path" "--default-toolchain" "stable"`
[2019-02-08T16:37:59Z INFO  crater::run] [stderr] info: syncing channel updates for 'stable-x86_64-pc-windows-msvc'
[2019-02-08T16:37:59Z INFO  crater::run] [stderr] info: latest update on 2019-01-17, rust version 1.32.0 (9fda7c223 2019-01-16)
[2019-02-08T16:37:59Z INFO  crater::run] [stderr] info: downloading component 'rustc'
[2019-02-08T16:38:02Z INFO  crater::run] [stderr] info: downloading component 'rust-std'
[2019-02-08T16:38:05Z INFO  crater::run] [stderr] info: downloading component 'cargo'
[2019-02-08T16:38:05Z INFO  crater::run] [stderr] info: downloading component 'rust-docs'
[2019-02-08T16:38:06Z INFO  crater::run] [stderr] info: installing component 'rustc'
[2019-02-08T16:38:06Z INFO  crater::run] [stderr] info: rolling back changes
[2019-02-08T16:38:06Z INFO  crater::run] [stderr] error: failed to extract package
[2019-02-08T16:38:06Z INFO  crater::run] [stderr] info: caused by: The filename, directory name, or volume label syntax is incorrect. (os error 123) when creating dir \\?\C:\Users\mackendy\source\crater\work\local\rustup-home\tmp\rbzjn0zcv3mh1cve_dir\rustc/bin
[2019-02-08T16:38:06Z ERROR crater::utils] unable to install rustup

It appears that rustup is trying to create a directory (ending in "/bin") with a forward slash, but perhaps something more subtle is going on.

Version
rustup-init 1.16.0 (beab5ac2b 2018-12-06)

@Eh2406
Copy link

Eh2406 commented Feb 9, 2019

Yes windows is good at switching / to \ unless you start your path with \\?\ in which case you are opting out of it fixing it for you. This is probably from the path.canonicalize method of std. We had an issue with this on Cargo w/ Cargo-web but Github search refuses to find if for me.
Edit: found it! rust-lang/cargo#6198

@ecstatic-morse
Copy link
Author

ecstatic-morse commented Feb 12, 2019

Sorry, I was away for the weekend. Loading this back into cache...

I can minimally reproduce this by running (in Powershell)
$env:RUSTUP_HOME="\\?\C:\Users\myname\.rustup\"; rustup toolchain install stable
on a fresh system. It attempts to create \\?\C:\Users\myname\.rustup\tmp\tgarbage_dir\rustc/bin, which fails for the reason you mention above. I think rustup should support extended-length path syntax in RUSTUP_HOME, since it can be useful e.g. when path names are longer than 255 characters.

Clearly, rustup is joining the string "rustc/bin" to a path directly instead of as separate components. The fix should be easy enough, but a grep for "/bin" doesn't reveal anything that's obviously the culprit. Perhaps you could point me to the part of the source code that deals with downloading artifacts into a temporary folder?

@ecstatic-morse
Copy link
Author

ecstatic-morse commented Feb 12, 2019

The error occurrs during archive extraction. Specifically, tar::Entry::path() returns a Path which uses "/" as the separator even on Windows. This is then appended to the tempdir which is an extended-length path since it is derived from RUSTUP_HOME.

One could make the case that this is a bug in tar, but I'm not sure I agree. The tar specification explicitly uses "/" as a path separator, so maybe they're doing the right thing? Regardless, I think relpath.components().collect() during tar extraction would fix this issue.

@ecstatic-morse
Copy link
Author

Sorry, I misclicked.

@ecstatic-morse ecstatic-morse changed the title Installation of rustc via rustup fails on Windows 10. Installation of rustc to a RUSTUP_HOME specified with an extended-length path fails on Windows. Feb 12, 2019
@retep998
Copy link
Member

What I'd really like is a smarter version of Path::join that doesn't just blindly concatenate, but actually processes each component, popping a directory for .., doing nothing for ., and normalizing path separators while it is at it. Then everyone could just use that and we'd solve the problem of pushing relative paths that use / onto \\?\ paths.

@ecstatic-morse
Copy link
Author

I agree that it's not feasible to support extended-length paths while using . and .. as components. Maybe there's a need for a bespoke, cross-platform PathBuilder library, although it's unfortunate that there's so much overlap with std::path::PathBuf, which behaves in surprising ways (to me at least :) on Windows.

Does rustup actually use . and .. to build paths anywhere? A quick search didn't reveal anything that looks like path.join(".."). If the use of ., .., or / in paths occurs only during tar extraction, the fix I mentioned above (changing "/" to "\" when reading the path of archive entries) would work. I can do a bit of testing to this effect.

@rbtcollins
Copy link
Contributor

The \?\ paths are processed lower in the NT IO stack as I understand it; so the / separate becomes a problem there, and this should indeed be fixed within tar-rs.

@workingjubilee
Copy link
Member

@rustbot label: +O-windows

@rustbot rustbot added the O-windows Windows related label Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Windows related
Projects
None yet
Development

No branches or pull requests

6 participants