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

gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup #99930

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kwi-dk
Copy link

@kwi-dk kwi-dk commented Dec 1, 2022

This PR fixes issue gh-91133, in which TemporaryDirectory.cleanup could try to fix permissions when deleting a symlink, but accidentally fix permissions of the target of the symlink instead.

(It also changes the test_flags test case so it doesn't leave behind NOUNLINK files when the test fails, which is just annoying when working on any change to the TemporaryDirectory code.)

Compatibility: There could conceivably be code out there relying on TemporaryDirectory.cleanup modifying permissions on files outside the tempdir being cleaned up, which the fixed code will no longer do.

Regarding the PR checks: "Ubuntu SSL tests with OpenSSL (3.0.7)" is failing due to a network error (instability, I assume), and the CLA Signing bot updated its comment when I signed the CLA, but not its check status. Not sure how to proceed.

kwi-dk added 2 commits Dec 1, 2022
During development, it becomes tiresome to have to manually clean up
these files in case of unrelated TemporaryDirectory breakage.
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Dec 1, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants