Commits

Remy Blank committed 41f1750

0.11.7dev: Fixed two issues with file renaming on Windows:

* Convert both arguments to Unicode and use the wide Windows API. This allows using both string and Unicode paths.
* Try all three variants for file replacement (`MoveFileTransacted`, `MoveFileEx` and `os.rename`) and fall back to the next one in case of failure. This fixes renaming on filesystems where atomic operations are not supported.

Closes #8623 and #8913.

Comments (0)

Files changed (2)

trac/util/__init__.py

 import errno
 import locale
 import os.path
+import random
 import re
 import sys
 import time
 
 can_rename_open_file = False
 if os.name == 'nt':
+    _rename = lambda src, dst: False
+    _rename_atomic = lambda src, dst: False
+    
     try:
         import ctypes
         MOVEFILE_REPLACE_EXISTING = 0x1
         MOVEFILE_WRITE_THROUGH = 0x8
+        MoveFileEx = ctypes.windll.kernel32.MoveFileExW
         
+        def _rename(src, dst):
+            if not isinstance(src, unicode):
+                src = unicode(src, sys.getfilesystemencoding())
+            if not isinstance(dst, unicode):
+                dst = unicode(dst, sys.getfilesystemencoding())
+            if _rename_atomic(src, dst):
+                return True
+            return MoveFileEx(src, dst, MOVEFILE_REPLACE_EXISTING
+                                        | MOVEFILE_WRITE_THROUGH)
+        
+        CreateTransaction = ctypes.windll.ktmw32.CreateTransaction
+        CommitTransaction = ctypes.windll.ktmw32.CommitTransaction
+        MoveFileTransacted = ctypes.windll.kernel32.MoveFileTransactedW
+        CloseHandle = ctypes.windll.kernel32.CloseHandle
+        can_rename_open_file = True
+        
+        def _rename_atomic(src, dst):
+            ta = CreateTransaction(None, 0, 0, 0, 0, 1000, 'Trac rename')
+            if ta == -1:
+                return False
+            try:
+                return (MoveFileTransacted(src, dst, None, None,
+                                           MOVEFILE_REPLACE_EXISTING
+                                           | MOVEFILE_WRITE_THROUGH, ta)
+                        and CommitTransaction(ta))
+            finally:
+                CloseHandle(ta)
+    except Exception:
+        pass
+    
+    def rename(src, dst):
+        # Try atomic or pseudo-atomic rename
+        if _rename(src, dst):
+            return
+        # Fall back to "move away and replace"
         try:
-            MoveFileTransacted = ctypes.windll.kernel32.MoveFileTransactedA
-            CreateTransaction = ctypes.windll.ktmw32.CreateTransaction
-            CommitTransaction = ctypes.windll.ktmw32.CommitTransaction
-            CloseHandle = ctypes.windll.kernel32.CloseHandle
-            can_rename_open_file = True
-            
-            def rename(src, dst):
-                ta = CreateTransaction(None, 0, 0, 0, 0, 1000, 
-                                       'Trac forced rename')
-                if ta == -1:
-                    raise ctypes.WinError()
-                try:
-                    if not (MoveFileTransacted(src, dst, None, None,
-                                               MOVEFILE_REPLACE_EXISTING
-                                               | MOVEFILE_WRITE_THROUGH, ta) \
-                            and CommitTransaction(ta)):
-                        raise ctypes.WinError()
-                finally:
-                    CloseHandle(ta)
-        except AttributeError:
-            MoveFileEx = ctypes.windll.kernel32.MoveFileExA
-            
-            def rename(src, dst):
-                if not MoveFileEx(src, dst, MOVEFILE_REPLACE_EXISTING
-                                            | MOVEFILE_WRITE_THROUGH):
-                    raise ctypes.WinError()
-    except Exception:
-        import random
-        
-        def rename(src, dst):
+            os.rename(src, dst)
+        except OSError, e:
+            if e.errno != errno.EEXIST:
+                raise
+            old = "%s-%08x" % (dst, random.randint(0, sys.maxint))
+            os.rename(dst, old)
+            os.rename(src, dst)
             try:
-                os.rename(src, dst)
-            except OSError, e:
-                if e.errno != errno.EEXIST:
-                    raise
-                old = "%s-%08x" % (dst, random.randint(0, sys.maxint))
-                os.rename(dst, old)
-                os.rename(src, dst)
-                try:
-                    os.unlink(old)
-                except Exception:
-                    pass
+                os.unlink(old)
+            except Exception:
+                pass
 else:
     rename = os.rename
     can_rename_open_file = True

trac/util/tests/__init__.py

             finally:
                 rf.close()
             self.assertEqual('Replaced content', util.read_file(self.path))
-        
+    
+    def test_unicode_path(self):
+        self.path = os.path.join(tempfile.gettempdir(), u'träc-témpfilè')
+        f = util.AtomicFile(self.path)
+        try:
+            f.write('test content')
+        finally:
+            f.close()
+        self.assertEqual('test content', util.read_file(self.path))
+
 
 class ContentDispositionTestCase(unittest.TestCase):