Open
Description
uff -- that is a nasty one ;) Our piece of code is
repo.config_writer().set_value("annex", "backends", backend)
where backend is a str (python3). And it seems to set the value properly and actual wipe out of the file doesn't happen until this writer (I guess) gets GCed -- I force that in our test now with gc.collect() and it results then in .git/config of that repo being wiped (0 byte size).
I have upgraded even to current master in that tox env, and the same result. Let me know if you need more details. python is 3.4.3, Debian amd64 sid/testing
Activity
PY3+ENH: apparently a bug in GitPython leads to wipe out of .git/config
Byron commentedon Jul 31, 2015
Unfortunately, this issue cannot be fixed on the side of GitPython. Back in the days, I wrote the code as if there were deterministic destructors, which seemed to be the case in py2.7. However, since py3, this clearly is not the case at all anymore, and one simply shouldn't use
__del__
at all.Code involving the writer now has to be written like so:
If that is not done, bad things can happen, even though I wouldn't know why it can also lead to a wiped configuration file as you describe it.
As noted, you will have to change all code involving a
config_writer()
to release it after use - you will find this kind of code throughout the GitPython code-base as well.Please let me know if this makes sense to you.
yarikoptic commentedon Jul 31, 2015
Great -- thanks for the quick get back!
__del__
on python3 -- now will be even more careful.config_writer(batch=True)
, it would require explicit.release()
. Even better. config_writer should exhibit itself as a context manager, so I could do more pythonicwhich would also release automatically whenever I leave the context
Byron commentedon Aug 3, 2015
I love the idea stated at 2), and believe this should have been the solution from the get-go. Probably I decided against it back in the days due to the performance implications, as I most certainly didn't think of introducing something like
config_writer(batch=True)
.Currently I am totally unsure when I will find the state of mind to do this - you might be faster with a respective PR in case it's critical to you.
yarikoptic commentedon Aug 3, 2015
;-) in my case it was a single use-case and I could easily fix it up
with the explicit .release, so most probably I will not be useful here
with a PR :-/
Cheers!
35 remaining items