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.

hgext/mq.py

@@ -1492,7 +1492,7 b' class queue(object):'
                 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]]

mercurial/archival.py

@@ -195,7 +195,7 b' class fileit(object):'
             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)
 

mercurial/bookmarks.py

@@ -90,7 +90,7 b' def write(repo):'
         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 b' def setcurrent(repo, mark):'
     try:
         file = repo.opener('bookmarks.current', 'w', atomictemp=True)
         file.write(encoding.fromlocal(mark))
-        file.rename()
+        file.close()
     finally:
         wlock.release()
     repo._bookmarkcurrent = mark

mercurial/dirstate.py

@@ -453,7 +453,7 b' class dirstate(object):'
             write(e)
             write(f)
         st.write(cs.getvalue())
-        st.rename()
+        st.close()
         self._lastnormaltime = None
         self._dirty = self._dirtypl = False
 

mercurial/hbisect.py

@@ -150,7 +150,7 b' def save_state(repo, state):'
         for kind in state:
             for node in state[kind]:
                 f.write("%s %s\n" % (kind, hex(node)))
-        f.rename()
+        f.close()
     finally:
         wlock.release()
 

mercurial/localrepo.py

@@ -521,7 +521,7 b' class localrepository(repo.repository):'
             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
 

mercurial/revlog.py

@@ -946,9 +946,9 b' class revlog(object):'
             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()

mercurial/simplemerge.py

@@ -445,7 +445,7 b' def simplemerge(ui, local, base, other, '
         out.write(line)
 
     if not opts.get('print'):
-        out.rename()
+        out.close()
 
     if m3.conflicts:
         if not opts.get('quiet'):

mercurial/store.py

@@ -345,7 +345,7 b' class fncache(object):'
         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):

mercurial/tags.py

@@ -287,6 +287,6 b' def _writetagcache(ui, repo, heads, tagf'
         cachefile.write("%s %s\n" % (hex(node), name))
 
     try:
-        cachefile.rename()
+        cachefile.close()
     except (OSError, IOError):
         pass

mercurial/util.py

@@ -745,11 +745,10 b' class atomictempfile(object):'
     '''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 b' class atomictempfile(object):'
         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 b' class atomictempfile(object):'
 
     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"""

tests/test-atomictempfile.py

@@ -12,22 +12,21 b' def test1_simple():'
     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 b' def test3_oops():'
 
 if __name__ == '__main__':
     test1_simple()
-    test2_close()
+    test2_discard()
     test3_oops()

tests/test-filecache.py

@@ -59,7 +59,7 b' def basic(repo):'
     # because of inode change
     f = scmutil.opener('.')('x', 'w', atomictemp=True)
     f.write('b')
-    f.rename()
+    f.close()
 
     repo.invalidate()
     repo.cached