D6555: vfs: require use of .seek() or .write() before .tell() on append-mode files

[durin42 (Augie Fackler)]

durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This prevents the bug in the previous change, but avoids an extraneous
  seek() call in the common case when it's not required. My preference
  was to ban .seek() and .tell() entirely on append-mode files since
  they're potentially misleading, but our codebase doesn't make that
  easy. This is better than nothing.
  
  See the previous change for a detailed explanation of the bug we've
  observed in the wild.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6555

AFFECTED FILES
  mercurial/vfs.py

CHANGE DETAILS

diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -303,6 +303,53 @@
             finally:
                 vfs._backgroundfilecloser = None
 
+class _appendfileproxy(object):
+    """Proxy type to prevent .tell() before .seek() or .write() append files.
+
+    Files opened for append aren't required to have a meaningful fpos
+    after open, so we require a write or explicit seek before allowing
+    .tell() on those files.
+    """
+    def __init__(self, fp):
+        object.__setattr__(self, r'_fp', fp)
+        object.__setattr__(self, r'_tellok', False)
+
+    def tell(self):
+        if not self._tellok:
+            raise error.ProgrammingError(
+                'tell() on append-mode file requires write() or seek() first')
+        return self._fp.tell()
+
+    def seek(self, *args, **kwargs):
+        object.__setattr__(self, r'_tellok', True)
+        return self._fp.seek(*args, **kwargs)
+
+    def write(self, *args, **kwargs):
+        object.__setattr__(self, r'_tellok', True)
+        return self._fp.write(*args, **kwargs)
+
+    def __repr__(self):
+        return r'<_appendfileproxy of %r>' % self._fp
+
+    # __magic__ methods get looked up on the type, not the instance,
+    # so we have to proxy __enter__ and __exit__ by hand.
+    def __enter__(self):
+        self._fp.__enter__()
+        return self
+
+    def __exit__(self, *args, **kwargs):
+        self._fp.__exit__(*args, **kwargs)
+
+    # proxy all other methods
+    def __getattr__(self, attr):
+        return getattr(self._fp, attr)
+
+    def __setattr__(self, attr, value):
+        return setattr(self._fp, attr, value)
+
+    def __delattr__(self, attr):
+        return delattr(self._fp, attr)
+
 class vfs(abstractvfs):
     '''Operate files relative to a base directory
 
@@ -441,7 +488,8 @@
                                   )
 
             fp = delayclosedfile(fp, self._backgroundfilecloser)
-
+        if mode in ('a', 'ab'):
+            return _appendfileproxy(fp)
         return fp
 
     def symlink(self, src, dst):



To: durin42, #hg-reviewers
Cc: mercurial-devel