view tests/test-mq-qimport-fail-cleanup.t @ 14007:d764463b433e

atomictempfile: avoid infinite recursion in __del__(). The problem is that a programmer using atomictempfile directly can make an innocent everyday mistake -- not enough args to the constructor -- which escalates badly. You would expect a simple TypeError crash in that case, but you actually get an infinite recursion that is surprisingly difficult to kill: it happens between __del__() and __getattr__(), and Python does not handle infinite recursion from __del__() well. The fix is to not implement __getattr__(), but instead assign instance attributes for the methods we wish to delegate to the builtin file type: write() and fileno(). I've audited mercurial.* and hgext.* and found no users of atomictempfile using methods other than write() and rename(). I audited third-party extensions and found one (snap) passing an atomictempfile to util.fstat(), so I also threw in fileno(). The last time I submitted a similar patch, Matt proposed that we make atomictempfile a subclass of file instead of wrapping it. Rejected on grounds of unnecessary complexity: for one thing, it would make the Windows implementation of posixfile quite a bit more complex. It would have to become a subclass of file rather than a simple function -- but since it's written in C, this is non-obvious and non-trivial. Furthermore, there's nothing wrong with wrapping objects and delegating methods: it's a well-established pattern that works just fine in many cases. Subclassing is not the answer to all of life's problems.
author Greg Ward <greg@gerg.ca>
date Sun, 24 Apr 2011 19:25:10 -0400
parents 8b12768b15e2
children 1e9451476bf8
line wrap: on
line source

Failed qimport of patches from files should cleanup by recording successfully
imported patches in series file.

  $ echo "[extensions]" >> $HGRCPATH
  $ echo "mq=" >> $HGRCPATH
  $ hg init repo
  $ cd repo
  $ echo a > a
  $ hg ci -Am'add a'
  adding a
  $ cat >b.patch<<EOF
  > diff --git a/a b/a
  > --- a/a
  > +++ b/a
  > @@ -1,1 +1,2 @@
  >  a
  > +b
  > EOF
  $ echo
  

empty series

  $ hg qseries
  $ echo
  

qimport valid patch followed by invalid patch

  $ hg qimport b.patch fakepatch
  adding b.patch to series file
  abort: unable to read file fakepatch
  [255]
  $ echo
  

valid patches before fail added to series

  $ hg qseries
  b.patch