app: cleanup temporary directories immediately after use

We were relying on re-using a temp directory so that we could
easily cleanup that directory when exiting.  This is problematic
when git-cola is exited via a signal handler, as it does not
have a chance to cleanup.

Instead of relying on a deferred cleanup, make the individual
commands cleanup after themselves.  This removes the need to
cleanup as the application is shutting down.

Closes #566
Reported-by: Jordon Bedwell <jordon@envygeeks.io>
Signed-off-by: David Aguilar <davvid@gmail.com>

cola/app.py

@@ -297,9 +297,6 @@ def application_start(context, view, monitor_refs_only=False):
     fsmonitor.current().stop()
     QtCore.QThreadPool.globalInstance().waitForDone()
 
-    tmpdir = utils.tmpdir()
-    shutil.rmtree(tmpdir, ignore_errors=True)
-
     return result
 
 

cola/cmds.py

@@ -229,16 +229,17 @@ def do(self):
             return
 
         cfg = gitcfg.current()
-        tmp_path = utils.tmp_filename('patch')
+        tmp_dir, tmp_file = utils.tmp_filename('patch')
         try:
-            core.write(tmp_path, patch,
+            core.write(tmp_file, patch,
                        encoding=cfg.file_encoding(self.model.filename))
             if self.apply_to_worktree:
-                status, out, err = self.model.apply_diff_to_worktree(tmp_path)
+                status, out, err = self.model.apply_diff_to_worktree(tmp_file)
             else:
-                status, out, err = self.model.apply_diff(tmp_path)
+                status, out, err = self.model.apply_diff(tmp_file)
         finally:
-            os.unlink(tmp_path)
+            core.rmtree(tmp_dir)
+
         Interaction.log_status(status, out, err)
         self.model.update_file_status(update_index=True)
 
@@ -443,18 +444,18 @@ def do(self):
         # Create the commit message file
         comment_char = prefs.comment_char()
         msg = self.strip_comments(self.msg, comment_char=comment_char)
-        tmpfile = utils.tmp_filename('commit-message')
+        tmp_dir, tmp_file = utils.tmp_filename('commit-message')
         try:
-            core.write(tmpfile, msg)
+            core.write(tmp_file, msg)
 
             # Run 'git commit'
-            status, out, err = self.model.git.commit(F=tmpfile,
+            status, out, err = self.model.git.commit(F=tmp_file,
                                                      v=True,
                                                      gpg_sign=self.sign,
                                                      amend=self.amend,
                                                      no_verify=self.no_verify)
         finally:
-            core.unlink(tmpfile)
+            core.rmtree(tmp_dir)
 
         if status == 0:
             ResetMode.do(self)
@@ -1661,10 +1662,12 @@ def do(self):
         log_msg = (N_('Tagging "%(revision)s" as "%(name)s"') %
                    dict(revision=self._revision, name=self._name))
         opts = {}
+        tmp_dir = None
         try:
             if self._message:
-                opts['F'] = utils.tmp_filename('tag-message')
-                core.write(opts['F'], self._message)
+                tmp_dir, tmp_file = utils.tmp_filename('tag-message')
+                opts['F'] = tmp_file
+                core.write(tmp_file, self._message)
 
             if self._sign:
                 log_msg += ' (%s)' % N_('GPG-signed')
@@ -1674,8 +1677,8 @@ def do(self):
             status, output, err = self.model.git.tag(self._name,
                                                      self._revision, **opts)
         finally:
-            if 'F' in opts:
-                os.unlink(opts['F'])
+            if tmp_dir:
+                core.rmtree(tmp_dir)
 
         if output:
             log_msg += '\n' + (N_('Output: %s') % output)

cola/core.py

@@ -10,6 +10,7 @@
 import sys
 import itertools
 import platform
+import shutil
 import subprocess
 
 from cola.decorators import interruptable
@@ -274,6 +275,10 @@ def xopen(path, mode='r', encoding=None):
     return open(mkpath(path, encoding=encoding), mode)
 
 
+def rmtree(path, ignore_errors=True):
+    shutil.rmtree(mkpath(path), ignore_errors=ignore_errors)
+
+
 def stdout(msg):
     msg = msg + '\n'
     if PY2:

cola/utils.py

@@ -165,14 +165,14 @@ def shell_split(s):
 
 
 def tmp_filename(label):
+    dirpath = tmp_dir()
     label = label.replace('/', '-').replace('\\', '-')
-    fd = tempfile.NamedTemporaryFile(dir=tmpdir(), prefix=label+'-')
+    fd = tempfile.NamedTemporaryFile(dir=dirpath, prefix=label+'-')
     fd.close()
-    return fd.name
+    return (dirpath, fd.name)
 
 
-@memoize
-def tmpdir():
+def tmp_dir():
     return tempfile.mkdtemp(prefix='git-cola-')
 
 

share/doc/git-cola/relnotes/v2.6.rst

@@ -37,6 +37,12 @@ Fixes
 
   https://github.com/git-cola/git-cola/issues/548
 
+* `git cola` now cleans up after itself immediately to avoid leaving behind
+  empty `/tmp/git-cola-XXXXXX` directories when the user uses `Ctrl+C`
+  to quit the app.
+
+  https://github.com/git-cola/git-cola/issues/566
+
 Packaging
 ---------
 

test/utils_test.py

@@ -52,31 +52,26 @@ def test_add_parents(self):
         expect = set(['foo///bar///baz'])
         self.assertEqual(expect, paths)
 
-
-    def test_tmpdir_gives_consistent_results(self):
-        utils.tmpdir.func.cache.clear()
-
-        first = utils.tmpdir()
-        second = utils.tmpdir()
-        third = utils.tmpdir()
-
-        self.assertEqual(first, second)
-        self.assertEqual(first, third)
-
-        shutil.rmtree(first)
+    def test_tmpdir_gives_unique_results(self):
+        tmp_dir1 = utils.tmp_dir()
+        tmp_dir2 = utils.tmp_dir()
+        self.assertNotEqual(tmp_dir1, tmp_dir2)
+        self.assertTrue(os.path.isdir(tmp_dir1))
+        self.assertTrue(os.path.isdir(tmp_dir2))
+        os.rmdir(tmp_dir1)
+        os.rmdir(tmp_dir2)
 
     def test_tmp_filename_gives_good_file(self):
-        utils.tmpdir.func.cache.clear()
-
-        tmpdir = utils.tmpdir()
-        first = utils.tmp_filename('test')
-        second = utils.tmp_filename('test')
+        tmp_dir1, first = utils.tmp_filename('test')
+        tmp_dir2, second = utils.tmp_filename('test')
 
         self.assertNotEqual(first, second)
-        self.assertTrue(first.startswith(os.path.join(tmpdir, 'test')))
-        self.assertTrue(second.startswith(os.path.join(tmpdir, 'test')))
+        self.assertNotEqual(tmp_dir1, tmp_dir2)
+        self.assertTrue(first.startswith(os.path.join(tmp_dir1, 'test')))
+        self.assertTrue(second.startswith(os.path.join(tmp_dir2, 'test')))
 
-        shutil.rmtree(tmpdir)
+        shutil.rmtree(tmp_dir1)
+        shutil.rmtree(tmp_dir2)
 
     def test_strip_one_abspath(self):
         expect = 'bin/git'