fix(utils): resolve input path in delete_dir_contents() if it's a link
#3754
Conversation
rami3l
force-pushed
the
fix/remove-dir-all-or-unlink
branch
2 times, most recently
from
b553a5f
to
92e4da4
Compare
rami3l
force-pushed
the
fix/remove-dir-all-or-unlink
branch
from
92e4da4
to
a267f6f
Compare
| // issue #3344 | ||
| /// `~/.rustup/tmp` and `~/.rustup/downloads` are permitted to be symlinks. | ||
| #[test] | ||
| #[cfg(any(unix, windows))] |
There was a problem hiding this comment.
isn't this equivalent to all platforms?
There was a problem hiding this comment.
There are indeed other platforms such as wasm, although I don't know if Rustup is meaningful there or not. Similar to other tests above/below it, this one relies on the fact that we compile for Unix or Windows. I just copied the code over as I mentioned above.
There was a problem hiding this comment.
Given that platforms like WASM exist, this seems okay.
There was a problem hiding this comment.
I think this is a side effect of code evolution, it isn't the pattern in general, and rustup does depend on hardlinks to operate at all. Are there platforms that have hardlinks, but not symlinks?
There was a problem hiding this comment.
@rbtcollins Looks like symlinks are also available on WASM/WASI: https://doc.rust-lang.org/std/os/wasi/fs/fn.symlink.html, so I'm also okay with removing this line. Anyway Rustup in its current form will fail to build on WASM and not all dependencies of this test work on WASM anyway.
| // issue #3344 | ||
| /// `~/.rustup/tmp` and `~/.rustup/downloads` are permitted to be symlinks. | ||
| #[test] | ||
| #[cfg(any(unix, windows))] |
There was a problem hiding this comment.
Given that platforms like WASM exist, this seems okay.
…nts_following_links()`
Closes #3344, superseding #3452: