Re: [StGit PATCH] Convert "sink" to the new infrastructure

On 2008-09-12 23:01:27 +0100, Catalin Marinas wrote:

"if they are applied"?

goto does it too. The docstring of the StackTransaction class explains how it works (if it doesn't, we need to improve it):

    """A stack transaction, used for making complex updates to an
    StGit stack in one single operation that will either succeed or
    fail cleanly.

    The basic theory of operation is the following:

      1. Create a transaction object.

      2. Inside a::

         try
           ...
         except TransactionHalted:
           pass

      block, update the transaction with e.g. methods like
      L{pop_patches} and L{push_patch}. This may create new git
      objects such as commits, but will not write any refs; this means
      that in case of a fatal error we can just walk away, no clean-up
      required.

      (Some operations may need to touch your index and working tree,
      though. But they are cleaned up when needed.)

      3. After the C{try} block -- wheher or not the setup ran to
      completion or halted part-way through by raising a
      L{TransactionHalted} exception -- call the transaction's L{run}
      method. This will either succeed in writing the updated state to
      your refs and index+worktree, or fail without having done
      anything."""

Not all transaction modifications need to be protected by the try block, only those that may actually raise TransactionHalted (i.e. those that may conflict). Specifically, in the code below, you need to put push_patch() in a try block. Otherwise that exception will propagate all the way up to the top level, and you will never reach the transaction's run() call which is where refs are updated and the new tree checked out.

The current set of functions made sense from an implementation perspective. But you are right that other variants would be helpful for some callers.