Skip to content

run-make: fix run_make_support::fs::create_symlink API and audit symlink usages in rmake.rs tests #129389

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
jieyouxu opened this issue Aug 22, 2024 · 2 comments
Assignees
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

I need to revisit run_make_support::fs::create_symlink because it's a wrong abstraction: there's a good reason why symlink_file/symlink_dir are different operations under std::os::windows::fs.

Furthermore, I need to double-check our symlink handling in rmake.rs tests. It's ok if symlinks are removed via fs::remove_dir_all because that has special handling for symlinks on Windows, but need to be extra careful if symlinks are attempted to be removed with fs::{remove_file, remove_dir} if the test can be run on Windows.

@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-run-make Area: port run-make Makefiles to rmake.rs labels Aug 22, 2024
@jieyouxu jieyouxu self-assigned this Aug 22, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 22, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 22, 2024
@ChrisDenton
Copy link
Member

ChrisDenton commented Aug 22, 2024

For convenience, there could be run_make_support::fs::remove_symlink that, on Windows, checks whether the symlink is a file or dir and uses the right remove function automagically.

Or the run-make remove_file function could lean in to people's expectations and test if the file is a is_symlink_dir and use the std remove_dir function.

@jieyouxu
Copy link
Member Author

jieyouxu commented Oct 9, 2024

Fixed in #130427

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants