GeezerVison is showing github.com — GitPython `repo.index.commit()` spawns persistent git.exe instance, holds handles to repo · Issue #553 · gitpython-developers/GitPython
GitPython repo.index.commit() spawns persistent git.exe instance, holds handles to repo #553DescriptionI am trying to use GitPython for some repo manipulation, but ran into issues with my app, with handles open where i wouldn't expect.
Bug-jarring the issue, it seems that calling repo.index.commit() results in a several (4, consistently) git.exe processes being spawned, each holding a handle to the repo's root directory newRepo. When the test below goes to delete this TempDir, the processes are still up, causing a failure on context-manager __exit__(). My app occasionally needs to do a similar cleanup, so hits the same issue.
On the one hand, it looks like there is no contect-manager capable repo-wrapper, meaning it makes sense if some resources is open, it will not typically be cleaned/GC'd before the tempdir __exit__(). On the other hand - if Repo is going to behave like that, it really should not persist resources that have such side-effects.
Here is a working unittest:
import unittest
import git
import tempfile
import os.path
class Test(unittest.TestCase):
def testCreateRepo(self):
with tempfile.TemporaryDirectory(prefix=(__loader__.name) + "_") as mydir:
# MAKE NEW REPO
repo = git.Repo.init(path=os.path.join(mydir, "newRepo"), mkdir=True)
self.assertTrue(os.path.isdir(os.path.join(repo.working_dir, ".git")), "Failed to make new repo?")
# MAKE FILE, COMMIT REPO
testFileName = "testFile.txt"
open(os.path.join(repo.working_dir, testFileName) , "w").close()
repo.index.add([testFileName])
self.assertTrue(repo.is_dirty())
####
# COMMENTING THIS OUT --> TEST PASSES
repo.index.commit("added initial test file")
self.assertFalse(repo.is_dirty())
####
# adding this does not affect the handle
git.cmd.Git.clear_cache()
print("done") # exception thrown right after this, on __exit__
# i can also os.walk(topdown=False) and delete all files and dirs (including .git/)
# it is just the newRepo/ folder itself who's handle is held open
if __name__ == '__main__':
unittest.main()
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\Users\%USER%\AppData\Local\Temp\EXAMPLE_gitpython_v3kbrly_\newRepo'
digging a little deeper, it seems that gitPython spawns multiple instances of git.exe processes, and each of them holds a handle to the root folder of the repo newRepo.
- set a breakpoint immediately before the error, use sysinternals/handle to see open handles to
newRepo ... git.exe (4 separate PID's of git.exe to be precise)
- using sysinternals/procexp i can see that that they are all spawned from the python instance
-- I'm typically running this from PyDev, but i verified the issue reproduces under vanila command line invocation of python.exe as well
- the exception indicates it's a handle to
newRepo being held. Adding a little extra code to the above I think that is the only handle held. I am able to successfully os.remove/os.rmdir() every dir and file, including all of .git/; and i finally manually recreate the issue seen on exit() in my example when i os.rmdir(newRepo)
stepping through, it's the call to repo.index.commit() that actually leads to the the git.exe(s) being spawned. Activityankostis commented on Dec 2, 2016 That's a quagmire of bugs!
-
Each git-repo instance indeed holds persistent git commands, that are supposed to be cleared by repo.clear_cache(). Otherwise, on Windows only, these commands prevent the repo-dir from being deleted.
So you have to invoke repo.clear_cache().
But that alone won't work unless you have garbage-collect first!
-
Additionally, the temp-file still may not be deleted because it contains read-only files (the blobs in .git/objects), so you need extra code for that. This code exists in git.util.rmtree() but currently git.util modules is being masked by git.index.util module, due to a bug, so you cannot use it - you have to copy it :-(
-
Finally, your code also had an error, you have to invoke clear_cache() on the repo instance, not directly on Git class.
So now the code becomes:
import unittest
import gc
import git
import tempfile
import os.path
import shutil
import stat
def rmtree(path):
"""Remove the given recursively.
:note: we use shutil rmtree but adjust its behaviour to see whether files that
couldn't be deleted are read-only. Windows will not remove them in that case"""
def onerror(func, path, exc_info):
# Is the error an access error ?
os.chmod(path, stat.S_IWUSR)
print('dfsffdss')
try:
func(path) # Will scream if still not possible to delete.
except Exception as ex:
raise
return shutil.rmtree(path, False, onerror)
class Test(unittest.TestCase):
def testCreateRepo(self):
with tempfile.TemporaryDirectory(prefix=(__loader__.name) + "_") as mydir:
# MAKE NEW REPO
repo = git.Repo.init(path=os.path.join(mydir, "newRepo"), mkdir=True)
try:
self.assertTrue(os.path.isdir(os.path.join(repo.working_dir, ".git")), "Failed to make new repo?")
# MAKE FILE, COMMIT REPO
testFileName = "testFile.txt"
open(os.path.join(repo.working_dir, testFileName) , "w").close()
repo.index.add([testFileName])
self.assertTrue(repo.is_dirty())
repo.index.commit("added initial test file")
self.assertFalse(repo.is_dirty())
print("done")
finally:
gc.collect()
repo.git.clear_cache()
rmtree(repo.git_dir)
if __name__ == '__main__':
unittest.main()
Please report if everything has been solved. Reacted by Pooja Bhat and allinone-1 ankostis commented on Dec 2, 2016 We would have fixed it yesterday, if we could :-)
Just search for tag.leaks to get an idea of the invested effort to reach to the point where git-python runs decently in Windows on PY34+.
The 1st and easiest fix is to retrofit git.Repo as a context-manager, so instead of try..finally you would use it as with Repo() as repo: .... ankostis commented on Dec 8, 2016 Assuming #555 gets merged in the next release, in your case the code would become simply like that:
class Test(unittest.TestCase):
def testCreateRepo(self):
with tempfile.TemporaryDirectory(prefix=(__loader__.name) + "_") as mydir:
with git.Repo.init(path=os.path.join(mydir, "newRepo"), mkdir=True) as repo:
self.assertTrue(os.path.isdir(os.path.join(repo.working_dir, ".git")), "Failed to make new repo?")
# MAKE FILE, COMMIT REPO
testFileName = "testFile.txt"
open(os.path.join(repo.working_dir, testFileName) , "w").close()
repo.index.add([testFileName])
self.assertTrue(repo.is_dirty())
repo.index.commit("added initial test file")
self.assertFalse(repo.is_dirty())
print("done")Byron commented on Dec 8, 2016 @mboard182 It looks like the context-manager addition is in-flight and might make it in the release planned for today. added a commit that references this issue on Oct 9, 2017This issue will close once commit 454feda is merged into the 'master' branch.454feda added a commit that references this issue on Dec 7, 2023This issue will close once commit a98c653 is merged into the 'master' branch.a98c653
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
