[PATCH] Trivial - transaction destructor fix

[K Thananchayan]

Hi Matt,
--- Matt Mackall <mpm at selenic.com> wrote:

> On Fri, Jul 01, 2005 at 03:15:40AM -0700, K
> Thananchayan wrote:
> > The destructor of tranaction no longer complain
> that attribute 'entries' does
> > not exists when journal already exists. Also, it
> closes the transaction file
> > before unlinking it to play safe for the other OS.
> > 
> > diff -r 877942ea8480 -r 9eea3299bbbf
> mercurial/transaction.py
> > --- a/mercurial/transaction.py	Fri Jul 01 10:10:56
> 2005
> > +++ b/mercurial/transaction.py	Fri Jul 01 10:16:13
> 2005
> > @@ -31,8 +31,11 @@
> >          self.file = open(self.journal, "w")
> >  
> >      def __del__(self):
> > -        if self.entries: self.abort()
> > -        try: os.unlink(self.journal)
> > +        try:
> > +            # if attr 'entries' does not exists,
> no cleanup is needed.
> > +            if self.entries: self.abort()
> > +            self.file.close()
> > +            os.unlink(self.journal)
> >          except: pass
> 
> This isn't quite right. This can leave the journal
> in place if we open
> and close an empty transaction. 

It appears to be correct to me. In case of existance
of 
journal during the construction of transaction,
entries 
attribute is not yet created and so no cleanup will be

performed. Otherwise, attributes entries, journal are 
created. As we are already removing the journal file 
when we close it (in the close method), this appear to
do the right thing in all cases.

> I think we want:
> 
> diff -r b9fee419a1bd mercurial/transaction.py
> --- a/mercurial/transaction.py  Fri Jul  1 16:54:52
> 2005
> +++ b/mercurial/transaction.py  Fri Jul  1 09:59:03
> 2005
> @@ -31,9 +31,11 @@
>          self.file = open(self.journal, "w")
> 
>      def __del__(self):
> -        if self.entries: self.abort()
> -        try: os.unlink(self.journal)
> -        except: pass
> +        if self.journal:
> +            if self.entries: self.abort()
> +            self.file.close()
> +            try: os.unlink(self.journal)
> +            except: pass
> 
>      def add(self, file, offset):
>          if file in self.map: return
> 
> -- 
> Mathematics is the supreme nostalgia of our time.
> 

On the other hand, this version still raise exception
if journal file already exists when tranaction is
opened (object does not have attribute journal yet).
This may also raise exception during destruction if 
transaction is successfully closed already as file is
not open.

Thanks,

-thanan

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com