Skip to content

Fix Permission Error [WinError32] during cleanup#253

Merged
jovnc merged 2 commits intogit-mastery:mainfrom
desmondwong1215:fix/clean-up
Feb 4, 2026
Merged

Fix Permission Error [WinError32] during cleanup#253
jovnc merged 2 commits intogit-mastery:mainfrom
desmondwong1215:fix/clean-up

Conversation

@desmondwong1215
Copy link
Contributor

Fix #252

I have tested it using WSL (it also works before these changes), PowerShell, and Git Bash. It works for me.

@github-actions
Copy link

Hi @desmondwong1215, thank you for your contribution! 🎉

This PR comes from your fork desmondwong1215/exercises on branch fix/clean-up.

Before you request for a review, please ensure that you have tested your changes locally!

Important

The previously recommended way of using ./test-download.py is no longer the best way to test your changes locally.

Please read the following instructions for the latest instructions.

Prerequisites

Ensure that you have the gitmastery app installed locally (instructions)

Testing steps

If you already have a local Git-Mastery root to test, you can skip the following step.

Create a Git-Mastery root locally:

gitmastery setup

Navigate into the Git-Mastery root (defaults to gitmastery-exercises/):

cd gitmastery-exercises/

Edit the .gitmastery.json configuration file. You need to set the following values under the exercises_source key.

{
    # other fields...
    "exercises_source": {
        "username": "desmondwong1215",
        "repository": "exercises",
        "branch": "fix/clean-up"
    }
}

Then, you can use the gitmastery app to download and verify your changes locally.

gitmastery download <your new change>
gitmastery verify

Checklist

  • (For exercises and hands-ons) I have verified that the downloading behavior works
  • (For exercises only) I have verified that the verification behavior is accurate

Important

To any reviewers of this pull request, please use the same instructions above to test the changes.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a PermissionError (WinError 32) that occurs on Windows when cleaning up temporary directories containing Git repositories. The issue stems from Windows file locking behavior where open file handles and the current working directory prevent deletion of the temporary directory.

Changes:

  • Added explicit repo.close() calls for Git repository objects before cleanup
  • Changed working directory to parent of temp directory before attempting cleanup
  • Reordered cleanup sequence to close repos → exit contexts → change directory → cleanup temp directories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jovnc
Copy link
Collaborator

jovnc commented Jan 31, 2026

@desmondwong1215 On the app side, we likely also want to give an appropriate error/warning message to the user

@desmondwong1215
Copy link
Contributor Author

Yeah, I will add an error/warning message to the user when fixing this issue on the app side.

@desmondwong1215 desmondwong1215 marked this pull request as ready for review January 31, 2026 16:58

if self.__temp_dir is not None:
self.__temp_dir.cleanup()
if self.__rs and self.__rs.repo:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rs is used for testing only, is this related to the bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we try to clean up the temporary directory on line 224 (self.__temp_dir.cleanup()), Windows can't delete it because the Git repository objects still have open file handles.

Copy link
Collaborator

@jovnc jovnc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the work!

@jovnc jovnc merged commit 416a5cc into git-mastery:main Feb 4, 2026
4 checks passed
@desmondwong1215 desmondwong1215 deleted the fix/clean-up branch February 9, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PermissionError [WinError 32] during TemporaryDirectory cleanup on Windows

2 participants