Fix(lib/fs/tests): Avoid permission denials when cleaning up TempDirs in set_get_permissions_nofollows*

[PaulDance]

At least under Windows 7, the set_get_permissions_nofollows and set_get_permissions_nofollows_symlink FS tests currently fail on:

---- fs::tests::set_get_permissions_nofollows stdout ----
thread 'fs::tests::set_get_permissions_nofollows' (2308) panicked at library/std/src/test_helpers.rs:53:20:
called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: PermissionDenied, message: "Access is denied." }
---- fs::tests::set_get_permissions_nofollows stdout end ----
---- fs::tests::set_get_permissions_nofollows_symlink stdout ----
thread 'fs::tests::set_get_permissions_nofollows_symlink' (1108) panicked at library/std/src/test_helpers.rs:53:20:
called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: PermissionDenied, message: "Access is denied." }
---- fs::tests::set_get_permissions_nofollows_symlink stdout end ----

The panic clearly occurs in TempDir::drop that calls fs::remove_dir_all. This is consistent with the fact that FILE_ATTRIBUTE_READONLY is set on the file:

Applications can read the file, but cannot write to it or delete it.

from the attribute's documentation.

This therefore fixes these tests by resetting the attribute before letting the drop guard run.

cc #141607 @roblabla

@rustbot label T-libs A-io O-windows-7

[rustbot]

r? @clarfonthey

rustbot has assigned @clarfonthey.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 13 candidates
• Random selection from 7 candidates

[clarfonthey]

Hmm, I honestly would be fine updating the test to do this on all platforms just as a safety measure, since nothing about this change is necessarily windows-specific.

Looking at our target policy, only Windows 10+ is supported, but I do think it's fine to ensure the tests pass in this one case just to extend the support.

@rustbot ping windows
In case anyone wants to comment on Windows 7-specific hacks, or support before Windows 10 in general.

[rustbot]

Hey Windows Group! This issue has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
issues. Maybe take a look?
Thanks! <3

cc @albertlarsan68 @arlosi @ChrisDenton @danielframpton @dpaoliello @gdr-at-ms @kennykerr @luqmana @nico-abram @retep998 @sivadeilra @wesleywiser

[clarfonthey]

Would add a comment that currently, this has only been noticed for Windows 7 (which is EOL), but that it's fine to add this out of an abundance of caution.

(Ditto for the other comment.)

Would be particularly interested to ensure that this passes all the tests on the other platforms that were relevant for this change, since I can't imagine that happening, but it's worth trying anyway.

You'd want to move the block specifically into the cfg_select! block, though, to only do this on platforms where we explicitly trust that this operation is supported.

View changes since the review

[clarfonthey]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[ChrisDenton]

@clarfonthey Windows 7 is a tier 3 target. We don't test it in CI but people supporting the platform should fix any problems (as is done here).

As to the issue itself, we do actually document this:

In Windows 7 and earlier this attribute prevents deleting empty directories. It does not prevent modifying the directory contents. On later versions of Windows this attribute is ignored for directories.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[PaulDance]

I moved the block to the cfg_select!. I also made it Win7-specific as this is more correct. If other platforms fail, then it would be surprising and there would be something to investigate I'd say.

@rustbot ready

[clarfonthey]

Probably should have tried harder to check that Windows 7 was a separate tier; I just assumed that they would all fall under Windows but… yeah.

Anyway, thank you for the change, and assuming that it works:

@bors r+ rollup

[rust-bors]

📌 Commit bcc5501 has been approved by clarfonthey

It is now in the queue for this repository.

[PaulDance]

Thanks!