Patchwork [STABLE] revlog: seek to end of file before writing (issue4943)

login
register
mail settings
Submitter Adrian Buehlmann
Date Dec. 18, 2015, 7:37 p.m.
Message ID <56746083.5000901@cadifra.com>
Download mbox | patch
Permalink /patch/12167/
State Accepted
Commit e240e914d2261788c3ba700401659c11ed820fe3
Headers show

Comments

Adrian Buehlmann - Dec. 18, 2015, 7:37 p.m.
On 2015-12-18 02:17, Gregory Szorc wrote:
[..]

> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -12,8 +12,9 @@ and O(changes) merge between branches.
>  """
>  
>  # import stuff from node for others to import from revlog
>  import collections
> +import os
>  from node import bin, hex, nullid, nullrev
>  from i18n import _
>  import ancestor, mdiff, parsers, error, util, templatefilters
>  import struct, zlib, errno
> @@ -1425,10 +1426,23 @@ class revlog(object):
>          self._basecache = (curr, chainbase)
>          return node
>  
>      def _writeentry(self, transaction, ifh, dfh, entry, data, link, offset):
> +        # Files opened in a+ mode have inconsistent behavior on various
> +        # platforms. Windows requires that a file positioning call be made
> +        # when the file handle transitions between reads and writes. See
> +        # 3686fa2b8eee and the mixedfilemodewrapper in windows.py. On other
> +        # platforms, Python or the platform itself can be buggy. Some versions
> +        # of Solaris have been observed to not append at the end of the file
> +        # if the file was seeked to before the end. See issue4943 for more.
> +        #
> +        # We work around this issue by inserting a seek() before writing.
> +        # Note: This is likely not necessary on Python 3.
> +        ifh.seek(0, os.SEEK_END)
> +
>          curr = len(self) - 1
>          if not self._inline:
> +            dfh.seek(0, os.SEEK_END)
>              transaction.add(self.datafile, offset)
>              transaction.add(self.indexfile, curr * len(entry))
>              if data[0]:
>                  dfh.write(data[0])

Nitpick:

When reading this, I was thinking that this might be a bit easier to
read, if it were written this way:

(dfh is None if the revlog is inlined)

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -13,6 +13,7 @@ 
 
 # import stuff from node for others to import from revlog
 import collections
+import os
 from node import bin, hex, nullid, nullrev
 from i18n import _
 import ancestor, mdiff, parsers, error, util, templatefilters
@@ -1426,6 +1427,20 @@ 
         return node
 
     def _writeentry(self, transaction, ifh, dfh, entry, data, link, offset):
+        # Files opened in a+ mode have inconsistent behavior on various
+        # platforms. Windows requires that a file positioning call be made
+        # when the file handle transitions between reads and writes. See
+        # 3686fa2b8eee and the mixedfilemodewrapper in windows.py. On other
+        # platforms, Python or the platform itself can be buggy. Some versions
+        # of Solaris have been observed to not append at the end of the file
+        # if the file was seeked to before the end. See issue4943 for more.
+        #
+        # We work around this issue by inserting a seek() before writing.
+        # Note: This is likely not necessary on Python 3.
+        ifh.seek(0, os.SEEK_END)
+        if dfh:
+            dfh.seek(0, os.SEEK_END)
+
         curr = len(self) - 1
         if not self._inline:
             transaction.add(self.datafile, offset)