Patchwork [2,of,8] py3: proxy posixfile objects to re-add a useful 'name' attribute on Windows

login
register
mail settings
Submitter Matt Harbison
Date Sept. 22, 2018, 3:28 p.m.
Message ID <0d1b3d5a92e159506c82.1537630118@Envy>
Download mbox | patch
Permalink /patch/34916/
State Accepted
Headers show

Comments

Matt Harbison - Sept. 22, 2018, 3:28 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1537574587 14400
#      Fri Sep 21 20:03:07 2018 -0400
# Node ID 0d1b3d5a92e159506c823be481cf9f57e45ec03d
# Parent  758cf8cdf994942174238c28c8f06ece63dae2b5
py3: proxy posixfile objects to re-add a useful 'name' attribute on Windows

This file object is used in the vfs layer, so there are many errors like this:

    ...
      File "mercurial\localrepo.py", line 2569, in savecommitmessage
        return self.pathto(fp.name[len(self.root) + 1:])
    TypeError: 'int' object is not subscriptable

It looks like the 'name' value is actually the fileno() value, and the
documentation says the name parameter to PyFile_FromFd() is ignored. [1]  I
tried just assigning the attribute after osutil.posixfile() returns, but that
crashes saying that it's read-only.  I don't know nearly enough python to know
if this handles all of the cases that it needs to, or if this is even sane.  The
context manager functions and __iter__() were things that crashed when missing,
for instance.  But it works well enough to fix a whole bunch of stacktraces.

[1] https://docs.python.org/3.6/c-api/file.html
Yuya Nishihara - Sept. 23, 2018, 6:38 a.m.
On Sat, 22 Sep 2018 11:28:38 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1537574587 14400
> #      Fri Sep 21 20:03:07 2018 -0400
> # Node ID 0d1b3d5a92e159506c823be481cf9f57e45ec03d
> # Parent  758cf8cdf994942174238c28c8f06ece63dae2b5
> py3: proxy posixfile objects to re-add a useful 'name' attribute on Windows

> diff --git a/mercurial/windows.py b/mercurial/windows.py
> --- a/mercurial/windows.py
> +++ b/mercurial/windows.py
> @@ -128,6 +128,28 @@ def posixfile(name, mode='r', buffering=
>      try:
>          fp = osutil.posixfile(name, mode, buffering) # may raise WindowsError
>  
> +        # PyFile_FromFd() ignores the name, and seems to report fp.name as the
> +        # underlying file descriptor.
> +        if pycompat.ispy3:
> +            class fdproxy(object):
> +                def __init__(self, name, fp):
> +                    self.name = name
> +                    self._fp = fp
> +
> +                def __enter__(self):
> +                    return self._fp.__enter__()
> +
> +                def __exit__(self, exc_type, exc_value, traceback):
> +                    self._fp.__exit__(exc_type, exc_value, traceback)
> +
> +                def __iter__(self):
> +                    return iter(self._fp)
> +
> +                def __getattr__(self, name):
> +                    return getattr(self._fp, name)

Can you move the class definition to module scope? It isn't nice to create
the proxy type per posixfile() call.

Patch

diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -128,6 +128,28 @@  def posixfile(name, mode='r', buffering=
     try:
         fp = osutil.posixfile(name, mode, buffering) # may raise WindowsError
 
+        # PyFile_FromFd() ignores the name, and seems to report fp.name as the
+        # underlying file descriptor.
+        if pycompat.ispy3:
+            class fdproxy(object):
+                def __init__(self, name, fp):
+                    self.name = name
+                    self._fp = fp
+
+                def __enter__(self):
+                    return self._fp.__enter__()
+
+                def __exit__(self, exc_type, exc_value, traceback):
+                    self._fp.__exit__(exc_type, exc_value, traceback)
+
+                def __iter__(self):
+                    return iter(self._fp)
+
+                def __getattr__(self, name):
+                    return getattr(self._fp, name)
+
+            fp = fdproxy(name, fp)
+
         # 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: