[RFC/MERGE][#243391] _run_and_cleanup helper (alternative to try/finally)

[Andrew Bennetts]

Hi all,

This patch is an extension of my fix for bug 230902 at
<http://bundlebuggy.aaronbentley.com/request/%3C20081008054310.GC19754@steerpike.home.puzzling.org%3E>.

I'm looking for opinions on the API defined by the attached patch.  I'm
especially interested in ideas for a more convenient alternative.  I'm
also interested ideas for a better home for this function than
bzrlib/branch.py :)

The patch defines a function, _run_and_cleanup:

  def _run_and_cleanup(cleanup, callable, *args, **kwargs):
      """Like ``callable(*args, **kwargs); cleanup()``, but errors from callable.
  
      e.g.::
  
          branch.lock_write()
          _run_and_cleanup(branch.unlock, branch.push, ...)
  
      If an exception is raised by callable, then that exception *will* be
      propagated, even if the cleanup function raises its own error.
      """
      # This is very similar to bzrlib.decorators.needs_write_lock.  Perhaps they
      # should share code?
      try:
          result = callable(*args, **kwargs)
      except:
          exc_info = sys.exc_info()
          try:
              cleanup()
          finally:
              raise exc_info[0], exc_info[1], exc_info[2]
      else:
          cleanup()
          return result

This is a little more flexible than the _run_with_write_locked_target
helper in my previous patch, because it doesn't assume that you want to
call foo.lock_write before the callable or foo.unlock after.

So for instance, _run_with_write_locked_target is now implemented as:

  def _run_with_write_locked_target(target, callable, *args, **kwargs):
      # [docstring snipped]
      target.lock_write()
      return _run_and_cleanup(target.unlock, callable, *args, **kwargs)

The idea is that anywhere where we have a finally block where we
*don't* want an exception from the corresponding try block to be
squashed, we should use _run_and_cleanup instead of try/finally. 

A quick grep of finally blocks in the codebase (try “grep -A1 -Irn
finally: bzrlib/ | grep -B1 unlock | less” for example) suggests that we
have lots of places in the code that may need changing one way or other.

This patch also updates bzrlib/commit.py to use this to run its cleanup
function, to demonstrate how it looks to use in real code.  This should
fix <https://bugs.launchpad.net/bzr/+bug/243391>!  But this demonstrates
the main drawback of this approach: it typically requires defining a new
function for no purpose other than to have code run from within
_run_and_cleanup.  This tends to make code be littered with “_foo_inner”
methods with fairly arbitrary arguments.  It may be that this is the
price we have to pay — it certainly seems better than embedding the
eleven lines of _run_and_cleanup directly into the function.

So, feedback sought.

-Andrew.