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

run-make: add a //@ needs-symlink directive over //@ ignore-windows #127797

Closed
jieyouxu opened this issue Jul 16, 2024 · 1 comment
Closed

run-make: add a //@ needs-symlink directive over //@ ignore-windows #127797

jieyouxu opened this issue Jul 16, 2024 · 1 comment
Assignees
Labels
A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

symlinks are a priviledged operation on Windows, but we should not straight up //@ ignore-windows as the CI runners have symlinks enabled, and so might the contributor on their local Windows machines. Instead, we should try to do what stdlib does here:

// Several test fail on windows if the user does not have permission to
// create symlinks (the `SeCreateSymbolicLinkPrivilege`). Instead of
// disabling these test on Windows, use this function to test whether we
// have permission, and return otherwise. This way, we still don't run these
// tests most of the time, but at least we do if the user has the right
// permissions.
pub fn got_symlink_permission(tmpdir: &TempDir) -> bool {
if cfg!(not(windows)) || env::var_os("CI").is_some() {
return true;
}
let link = tmpdir.join("some_hopefully_unique_link_name");
match symlink_file(r"nonexisting_target", link) {
// ERROR_PRIVILEGE_NOT_HELD = 1314
Err(ref err) if err.raw_os_error() == Some(1314) => false,
Ok(_) | Err(_) => true,
}
}

Thanks to @ChrisDenton for pointing this out to me!

@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs labels Jul 16, 2024
@jieyouxu jieyouxu self-assigned this Jul 16, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 16, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 16, 2024
@jieyouxu jieyouxu moved this to In progress in Oxidizing run-make tests Jul 16, 2024
@jieyouxu jieyouxu moved this from Backlog to In progress in compiletest maintenance and improvements Jul 16, 2024
@jieyouxu
Copy link
Member Author

Wait Chris already added this in #126862 and I was the one to review that PR 💀 Nevermind lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

No branches or pull requests

2 participants