Fix rollback bug in SymbolicReference.set_reference by EliahKagan · Pull Request #1675 · gitpython-developers/GitPython (original) (raw)

Fixes #1669

This fixes the specific bug in SymbolicReference.set_reference where the local call to ldf.rollback is never made. It also refactors, both here and elsewhere, to make try-finally logic clearer and simpler, and in some cases to replace it. This is to prevent such bugs from recurring, or at least make them easier to spot. This cleanup corresponds to try-finally cleanup done in the test suite as part of the larger PR #1673, but none of these changes were necessary to fix flake8 warnings, nor directly related to changes that were necessary for that, and it seemed like including this in that PR would make it broader than would be ideal.

As noted in #1669 (comment), the best solution to this, along with much of the rest of what may fall under the current resource leakage limitation, would be to make LockedFD and other classes with __del__ methods into context managers and use them as such (typically with with-statements) throughout GitPython. This does not make that change, as it is only a narrow-bugfix and cleanup PR.

From #1669 (comment) I am uncertain if this pull request would be considered to be in good shape without additions or other changes. However, I figured that, by opening it, these extra changes I did in git/ while working on #1673, that I considered useful but not topically suitable for inclusion in #1673, can be reviewed and, if considered valuable, merged with or without refinements.