Mercurial > hg
changeset 15057:774da7121fc9
atomictempfile: make close() consistent with other file-like objects.
The usual contract is that close() makes your writes permanent, so
atomictempfile's use of close() to *discard* writes (and rename() to
keep them) is rather unexpected. Thus, change it so close() makes
things permanent and add a new discard() method to throw them away.
discard() is only used internally, in __del__(), to ensure that writes
are discarded when an atomictempfile object goes out of scope.
I audited mercurial.*, hgext.*, and ~80 third-party extensions, and
found no one using the existing semantics of close() to discard
writes, so this should be safe.
| author | Greg Ward <greg@gerg.ca> |
|---|---|
| date | Thu, 25 Aug 2011 20:21:04 -0400 |
| parents | 8413916df816 |
| children | 81f33be0ea79 |
| files | hgext/mq.py mercurial/archival.py mercurial/bookmarks.py mercurial/dirstate.py mercurial/hbisect.py mercurial/localrepo.py mercurial/revlog.py mercurial/simplemerge.py mercurial/store.py mercurial/tags.py mercurial/util.py tests/test-atomictempfile.py tests/test-filecache.py |
| diffstat | 13 files changed, 25 insertions(+), 27 deletions(-) [+] |
line wrap: on
line diff
--- a/hgext/mq.py Wed Aug 24 05:42:41 2011 -0400 +++ b/hgext/mq.py Thu Aug 25 20:21:04 2011 -0400 @@ -1492,7 +1492,7 @@ n = repo.commit(message, user, ph.date, match=match, force=True) # only write patch after a successful commit - patchf.rename() + patchf.close() self.applied.append(statusentry(n, patchfn)) except: ctx = repo[cparents[0]]
--- a/mercurial/archival.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/archival.py Thu Aug 25 20:21:04 2011 -0400 @@ -195,7 +195,7 @@ return f = self.opener(name, "w", atomictemp=True) f.write(data) - f.rename() + f.close() destfile = os.path.join(self.basedir, name) os.chmod(destfile, mode)
--- a/mercurial/bookmarks.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/bookmarks.py Thu Aug 25 20:21:04 2011 -0400 @@ -90,7 +90,7 @@ file = repo.opener('bookmarks', 'w', atomictemp=True) for refspec, node in refs.iteritems(): file.write("%s %s\n" % (hex(node), encoding.fromlocal(refspec))) - file.rename() + file.close() # touch 00changelog.i so hgweb reloads bookmarks (no lock needed) try: @@ -121,7 +121,7 @@ try: file = repo.opener('bookmarks.current', 'w', atomictemp=True) file.write(encoding.fromlocal(mark)) - file.rename() + file.close() finally: wlock.release() repo._bookmarkcurrent = mark
--- a/mercurial/dirstate.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/dirstate.py Thu Aug 25 20:21:04 2011 -0400 @@ -453,7 +453,7 @@ write(e) write(f) st.write(cs.getvalue()) - st.rename() + st.close() self._lastnormaltime = None self._dirty = self._dirtypl = False
--- a/mercurial/hbisect.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/hbisect.py Thu Aug 25 20:21:04 2011 -0400 @@ -150,7 +150,7 @@ for kind in state: for node in state[kind]: f.write("%s %s\n" % (kind, hex(node))) - f.rename() + f.close() finally: wlock.release()
--- a/mercurial/localrepo.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/localrepo.py Thu Aug 25 20:21:04 2011 -0400 @@ -521,7 +521,7 @@ for label, nodes in branches.iteritems(): for node in nodes: f.write("%s %s\n" % (hex(node), encoding.fromlocal(label))) - f.rename() + f.close() except (IOError, OSError): pass
--- a/mercurial/revlog.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/revlog.py Thu Aug 25 20:21:04 2011 -0400 @@ -946,9 +946,9 @@ e = self._io.packentry(self.index[i], self.node, self.version, i) fp.write(e) - # if we don't call rename, the temp file will never replace the + # if we don't call close, the temp file will never replace the # real index - fp.rename() + fp.close() tr.replace(self.indexfile, trindex * self._io.size) self._chunkclear()
--- a/mercurial/simplemerge.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/simplemerge.py Thu Aug 25 20:21:04 2011 -0400 @@ -445,7 +445,7 @@ out.write(line) if not opts.get('print'): - out.rename() + out.close() if m3.conflicts: if not opts.get('quiet'):
--- a/mercurial/store.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/store.py Thu Aug 25 20:21:04 2011 -0400 @@ -345,7 +345,7 @@ fp = self.opener('fncache', mode='wb', atomictemp=True) for p in self.entries: fp.write(encodedir(p) + '\n') - fp.rename() + fp.close() self._dirty = False def add(self, fn):
--- a/mercurial/tags.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/tags.py Thu Aug 25 20:21:04 2011 -0400 @@ -287,6 +287,6 @@ cachefile.write("%s %s\n" % (hex(node), name)) try: - cachefile.rename() + cachefile.close() except (OSError, IOError): pass
--- a/mercurial/util.py Wed Aug 24 05:42:41 2011 -0400 +++ b/mercurial/util.py Thu Aug 25 20:21:04 2011 -0400 @@ -745,11 +745,10 @@ '''writeable file object that atomically updates a file All writes will go to a temporary copy of the original file. Call - rename() when you are done writing, and atomictempfile will rename - the temporary copy to the original name, making the changes visible. - - Unlike other file-like objects, close() discards your writes by - simply deleting the temporary file. + close() when you are done writing, and atomictempfile will rename + the temporary copy to the original name, making the changes + visible. If the object is destroyed without being closed, all your + writes are discarded. ''' def __init__(self, name, mode='w+b', createmode=None): self.__name = name # permanent name @@ -761,12 +760,12 @@ self.write = self._fp.write self.fileno = self._fp.fileno - def rename(self): + def close(self): if not self._fp.closed: self._fp.close() rename(self._tempname, localpath(self.__name)) - def close(self): + def discard(self): if not self._fp.closed: try: os.unlink(self._tempname) @@ -776,7 +775,7 @@ def __del__(self): if safehasattr(self, '_fp'): # constructor actually did something - self.close() + self.discard() def makedirs(name, mode=None): """recursive directory creation with parent mode inheritance"""
--- a/tests/test-atomictempfile.py Wed Aug 24 05:42:41 2011 -0400 +++ b/tests/test-atomictempfile.py Thu Aug 25 20:21:04 2011 -0400 @@ -12,22 +12,21 @@ assert basename in glob.glob('.foo-*') file.write('argh\n') - file.rename() + file.close() assert os.path.isfile('foo') assert basename not in glob.glob('.foo-*') print 'OK' -# close() removes the temp file but does not make the write -# permanent -- essentially discards your work (WTF?!) -def test2_close(): +# discard() removes the temp file without making the write permanent +def test2_discard(): if os.path.exists('foo'): os.remove('foo') file = atomictempfile('foo') (dir, basename) = os.path.split(file._tempname) file.write('yo\n') - file.close() + file.discard() assert not os.path.isfile('foo') assert basename not in os.listdir('.') @@ -45,5 +44,5 @@ if __name__ == '__main__': test1_simple() - test2_close() + test2_discard() test3_oops()