Patchwork [1,of,3,V2] windows: seek to the end of posixfile when opening in append mode

login
register
mail settings
Submitter Matt Harbison
Date Feb. 6, 2015, 3:19 a.m.
Message ID <05678c22e04da407ab61.1423192754@Envy>
Download mbox | patch
Permalink /patch/7701/
State Accepted
Commit 7956d17431bce17300c36c74021dbbb1edf299a3
Headers show

Comments

Matt Harbison - Feb. 6, 2015, 3:19 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1422725984 18000
#      Sat Jan 31 12:39:44 2015 -0500
# Node ID 05678c22e04da407ab610765248636a125e1348a
# Parent  a9b61dbdb827165a9fd9d44dd42b892dbd9fa07c
windows: seek to the end of posixfile when opening in append mode

The position is implementation defined when opening in append mode, and it seems
like Linux sets it to EOF while Windows keeps it at zero.  This has caused
problems in the past when a file is opened and tell() is immediately called,
such as 48c232873a54 and 6bf93440a717.

Since the only caller of osutil.posixfile is this windows module, this seems
like a better place to fix the issue than in osutil.c and pure.osutil.
Adrian Buehlmann - Feb. 6, 2015, 8:24 a.m.
On 2015-02-06 04:19, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1422725984 18000
> #      Sat Jan 31 12:39:44 2015 -0500
> # Node ID 05678c22e04da407ab610765248636a125e1348a
> # Parent  a9b61dbdb827165a9fd9d44dd42b892dbd9fa07c
> windows: seek to the end of posixfile when opening in append mode
> 
> The position is implementation defined when opening in append mode, and it seems
> like Linux sets it to EOF while Windows keeps it at zero.  This has caused
> problems in the past when a file is opened and tell() is immediately called,
> such as 48c232873a54 and 6bf93440a717.

..

> Since the only caller of osutil.posixfile is this windows module, this seems
> like a better place to fix the issue than in osutil.c and pure.osutil.

Yep. Probably not worth the extra work. And it could be moved to
osutil.c as a followup later if really needed.

> diff --git a/mercurial/windows.py b/mercurial/windows.py
> --- a/mercurial/windows.py
> +++ b/mercurial/windows.py
> @@ -26,11 +26,19 @@
>  unlink = win32.unlink
>  
>  umask = 0022
> +_SEEK_END = 2 # os.SEEK_END was introduced in Python 2.5
>  
>  # wrap osutil.posixfile to provide friendlier exceptions
>  def posixfile(name, mode='r', buffering=-1):
>      try:
> -        return osutil.posixfile(name, mode, buffering)
> +        fp = osutil.posixfile(name, mode, buffering)
> +
> +        # The position when opening in append mode is implementation defined, so
> +        # make it consistent with other platforms, which position at EOF.
> +        if 'a' in mode:
> +            fp.seek(0, _SEEK_END)
> +
> +        return fp
>      except WindowsError, err:
>          raise IOError(err.errno, '%s: %s' % (name, err.strerror))
>  posixfile.__doc__ = osutil.posixfile.__doc__

I would probably have placed the seek call outside of the try-block, but
this looks good enough to me either way.

Patch

diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -26,11 +26,19 @@ 
 unlink = win32.unlink
 
 umask = 0022
+_SEEK_END = 2 # os.SEEK_END was introduced in Python 2.5
 
 # wrap osutil.posixfile to provide friendlier exceptions
 def posixfile(name, mode='r', buffering=-1):
     try:
-        return osutil.posixfile(name, mode, buffering)
+        fp = osutil.posixfile(name, mode, buffering)
+
+        # The position when opening in append mode is implementation defined, so
+        # make it consistent with other platforms, which position at EOF.
+        if 'a' in mode:
+            fp.seek(0, _SEEK_END)
+
+        return fp
     except WindowsError, err:
         raise IOError(err.errno, '%s: %s' % (name, err.strerror))
 posixfile.__doc__ = osutil.posixfile.__doc__