Fix Windows bootstrap panic on invalid symlink removal (issue #143045)

[hasip-timurtas]

Fix Windows bootstrap panic on invalid symlink removal (issue #143045)

On Windows, use fs::remove_file for symlinks instead of fs::remove_dir to avoid error 267 when the symlink target is invalid. fs::remove_file can handle both file and directory symlinks/junctions, even when their targets don't exist.

r? @ghost

[rustbot]

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

@hasip-timurtas Please make your PR description concise so your reviewer does not waste time reading it. It should only need to be about two sentences for a change of this size.

[workingjubilee]

Oh, your original commit message contains r? @ghost

That's something for guiding rustbot. You need to put it in your PR description for it to work. I can help, though, by unassigning your reviewer, since r? @ghost is for a PR that you do not intend to have reviewed.

[rustbot]

Failed to set assignee to ghost: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[hasip-timurtas]

@workingjubilee please check if it's ok now 🙂

[workingjubilee]

@hasip-timurtas Yes, that looks good to me. You may want to remove the r? @ghost from your commit message but a final decision there is up to the reviewer. Feel free to use r? again for your previous reviewer and to @rustbot ready.

[ChrisDenton]

The comment is incorrect and I also don't think the code is the correct fix. Both remove_file and remove_dir can remove symlinks. However, remove_dir can only remove directory symlinks.

[hasip-timurtas]

@ChrisDenton Thank you for the feedback

After digging into the Windows implementation, I found that fs::remove_file actually has special handling for this case. When it fails with ERROR_ACCESS_DENIED, it retries with FILE_FLAG_OPEN_REPARSE_POINT to handle the symlink directly (see library/std/src/sys/fs/windows.rs:1228-1235).

This is why it works on invalid symlinks - it doesn't try to follow them. fs::remove_dir doesn't have this fallback, which causes error 267.

Since fs::remove_file handles both file and directory symlinks on Windows, it's the right choice here. Happy to add a comment explaining this if you think it'd be helpful.

[hasip-timurtas]

@rustbot ready

[ChrisDenton]

Since fs::remove_file handles both file and directory symlinks on Windows, it's the right choice here.

Sorry, I think you've misunderstood what that code is doing. Both DeleteFile and the fallback will open the symlink itself not the target. There is no issue if the target does not exist. The fallback is only there for readonly files that DeleteFile can't handle. The problem here is different.

On Windows there are two types of symlink: directory and non-directory. remove_dir can delete directory symlinks and remove_file can delete non-directory symlinks. Mixing them up will not work. Whether or not a symlink is a directory or non-directory symlink is a property of the symlink itself and nothing to do with the target.

So I would prefer this be implemented something like: if remove_dir fails with ErrorKind::NotADirectory then retry with remove_file.

[hasip-timurtas]

@ChrisDenton Thank you for the clarification. I've implemented your suggested approach, but I'd like to confirm a few details to ensure we're handling all scenarios correctly:

1. Will fs::remove_dir on a directory symlink with an invalid target still succeed, or could it also fail with error 267?

2. Are there any other error codes besides NotADirectory that should trigger the fallback to remove_file?

3. Should we handle the case where both remove_dir and remove_file fail differently, or is panicking the appropriate behavior?

The current implementation:

if let Err(e) = fs::remove_dir(&host) {
    if e.kind() == io::ErrorKind::NotADirectory {
        t!(fs::remove_file(&host));
    } else {
        panic!("failed to remove symlink: {}", e);
    }
}

I want to make sure we're covering all the edge cases for this Windows-specific issue.