-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Panic if invalid path used for -Z persist-doctests #98690
Comments
This panic is also triggered in Linux if e.g. trying to write into a directory that the user has no permission to write. I PR'd #98708 fix to change the unwrap to more orderly panic and relaying the underlying error detail via eprintln! ~/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc --test -Z unstable-options --persist-doctests /tmp/foobar main.rs instead of:
|
The lack of ICE is definitely nicer, but this still won't handle UNC paths on windows will it? Which is the main issue I personally have with the code |
That's a problem for std::fs::create_dir_all(&path) me thinks has to be fixed there |
Makes sense looking at the code, should I raise another issue for the libs team? |
Actually, yes it would be great to raise another issue targeting std::fs::create_dir_all - as it is wider issue. you are right. Thanks! |
We call those verbatim paths and they have to start with |
So the paths are weirding me out (I normally only use linux), because they seem to sometimes print differently given different display methods. It shows up as |
@xd009642 do you mind changing title s/UNC/invalid/ to address the panic on this issue, thanks! :) |
@xd009642 also please remove s/on windows// thanks a lot 👍 |
Done, and I realised what was wrong with the path, it's correct in the code, it's just when it got set into the environment variable it seems it needed the first slash to be escaped and that caused the invalid path (so no issues in std::fs need fixing) |
…llaumeGomez rustdoc: fix 98690 Panic if invalid path for -Z persist-doctests Closes rust-lang#98690 for rustdoc panic I changed this to do eprintln and orderly panic instead of unwrap doing unhandled panic ~/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc --test -Z unstable-options --persist-doctests /tmp/foobar main.rs Couldn't create directory for doctest executables: Permission denied (os error 13)
…llaumeGomez rustdoc: fix 98690 Panic if invalid path for -Z persist-doctests Closes rust-lang#98690 for rustdoc panic I changed this to do eprintln and orderly panic instead of unwrap doing unhandled panic ~/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc --test -Z unstable-options --persist-doctests /tmp/foobar main.rs Couldn't create directory for doctest executables: Permission denied (os error 13)
…llaumeGomez rustdoc: fix 98690 Panic if invalid path for -Z persist-doctests Closes rust-lang#98690 for rustdoc panic I changed this to do eprintln and orderly panic instead of unwrap doing unhandled panic ~/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc --test -Z unstable-options --persist-doctests /tmp/foobar main.rs Couldn't create directory for doctest executables: Permission denied (os error 13)
When setting the directory to persist doctests to if an UNC path is used on windows there's a segfault.
The offending setting of persist-doctests:
--persist-doctests \?\C:\Users\danie\Documents\doc_coverage\target\doctests
. cargo-tarpaulin sets this programatically using a path it gets for the target directory from cargo-metadata. The path from cargo-metadata is an UNC path and this then causes a segfault. I'm going to detect UNC paths in my code and remove the\?
on windows, but I felt this warrants a bug report as someone else will hit it eventually. Also, I'm not sure if other parts of the compiler fall prey to this issue?Compiler version is: the latest nightly (1.64.0-nightly 8308806 2022-06-28)
The stack trace:
The text was updated successfully, but these errors were encountered: