[MERGE] Make cmd_branch check the destination before doing any I/O on the source.

[Martin Pool]

On 26 Jul 2006, Michael Ellerman <michael at ellerman.id.au> wrote:
> On Wed, 2006-07-26 at 10:48 +1000, Andrew Bennetts wrote:
> > On Tue, Jul 25, 2006 at 01:48:31PM -0500, John Arbash Meinel wrote:
> > [...]
> > > The only thing it really means is that if we get an exception, and then
> > > delete_tree() also raises, it will hide the original exception.
> > > The only way I know around it is:
> > > 
> > > except Exception, e:
> > >   try:
> > >     to_transport.delete_tree('.')
> > >   except:
> > >     raise e
> > >   raise
> > > 
> > > That will raise the original exception delete_tree() fails, though you
> > > lose the traceback. And if delete_tree() succeeds, it raises the
> > > original exception with the original traceback.
> > 
> > You can do slightly better with sys.exc_info():
> > 
> > except Exception, e:
> >   exc_value, exc_type, exc_tb = sys.exc_info()
> >   try:
> >     to_transport.delete_tree('.')
> >   except:
> >     raise exc_value, exc_type, exc_tb
> >   raise
> > 
> > This preserves the original exception, although the code is a bit uglier.

I'm not really convinced this is a sufficient improvement to justify the
complexity.  If the original exception was say a KeyboardInterrupt, and
we then got an exception indicating a real bug in bzr then it would be
much worse to raise the first.

In the case of exceptions during cleanup I think we should probably
follow what python does for del methods - show a warning and continue.
So

  except:
    try:
      to_transport.delete_tree('.')
    except TransportError, e:
      warning("error %s while removing destination", e)
    raise

(perhaps display of the exception should be lifted to a
warn_about_exception).

Arguably that should catch and warn about everything rather than just
TransportError, but I think making it specific is fine.

-- 
Martin