Skip to content

Fix rollback bug in SymbolicReference.set_reference #1675

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

Merged
merged 3 commits into from
Sep 21, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Tweak rollback logic in log.to_file
This modifies the exception handling in log.to_file so it catches
BaseException rather than Exception and rolls back. Ordinarily we
do not want to catch BaseException, since this means catching
things like SystemExit, KeyboardInterupt, etc., but the other cases
of rolling back with LockedFD do it that strongly (both before when
try-finally was used with a flag, and now with try-except catching
BaseException to roll back the temporary-file write and reraise).
Having this behave subtly different does not appear intentional.

(This is also closer to what will happen if LockedFD becomes a
context manager and these pieces of code use it in a
with-statement: even exceptions not inheriting from Exception will
cause __exit__ to be called.)
  • Loading branch information
EliahKagan committed Sep 21, 2023
commit e480985aa4d358d0cc167d4552910e85944b8966
3 changes: 1 addition & 2 deletions git/refs/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@ def to_file(self, filepath: PathLike) -> None:
try:
self._serialize(fp)
lfd.commit()
except Exception:
# on failure it rolls back automatically, but we make it clear
except BaseException:
lfd.rollback()
raise
# END handle change
Expand Down