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.
--- 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()
--- a/tests/test-filecache.py Wed Aug 24 05:42:41 2011 -0400
+++ b/tests/test-filecache.py Thu Aug 25 20:21:04 2011 -0400
@@ -59,7 +59,7 @@
# because of inode change
f = scmutil.opener('.')('x', 'w', atomictemp=True)
f.write('b')
- f.rename()
+ f.close()
repo.invalidate()
repo.cached