Code review comment for lp://staging/~spiv/bzr/command-cleanup

Revision history for this message
Andrew Bennetts (spiv) wrote :

This branch is mostly mechanical: it replaces lots of try/finally blocks in bzrlib/builtins.py with calls to a new self.add_cleanup method, implemented using the robust logic in the bzrlib.cleanup module. This fixes issues of the sort reported in bug 496590 (ObjectNotLocked annotating file from other branch): try/finally tends to unconditionally attempt cleanups that might fail when there's a prior error, thus failing a second time and masking the original error.

There are some small infrastructural improvements to support this: Command classes now have add_cleanup and cleanup_now methods (the latter because sometimes commands have multiple stages, e.g. 'access locked repository' followed by 'format and print output'). The OperationWithCleanups class now has cleanup_now and run_simple.

The diff is large but as already mentioned mostly mechanical. It overall shrinks the line count of builtins.py *and* reduces the indentation level of most of the touched lines... so the code is hopefully clearer as well as more robust.

« Back to merge proposal