Patchwork [1,of,4] py3: ensure the proxied Windows fd doesn't escape by entering context manager

login
register
mail settings
Submitter Matt Harbison
Date Dec. 15, 2018, 8:04 p.m.
Message ID <068910232e124a50a13f.1544904264@Envy>
Download mbox | patch
Permalink /patch/37190/
State Accepted
Headers show

Comments

Matt Harbison - Dec. 15, 2018, 8:04 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1544855178 18000
#      Sat Dec 15 01:26:18 2018 -0500
# Node ID 068910232e124a50a13fd7444844d9151db48d6b
# Parent  5817c3b186a7799ecc3130493ba134b55cd4ba07
py3: ensure the proxied Windows fd doesn't escape by entering context manager

The purpose of the proxy class is to provide the `name` attribute which contains
the file path.  But in tests that used a context manager, it still blew up
complaining that 'int' doesn't have a 'startswith' function.
Yuya Nishihara - Dec. 16, 2018, 1:02 a.m.
On Sat, 15 Dec 2018 15:04:24 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1544855178 18000
> #      Sat Dec 15 01:26:18 2018 -0500
> # Node ID 068910232e124a50a13fd7444844d9151db48d6b
> # Parent  5817c3b186a7799ecc3130493ba134b55cd4ba07
> py3: ensure the proxied Windows fd doesn't escape by entering context manager

Good catch. Queued these for stable because some of them will break things
if "with" statement is involved, and I can't be sure we have no such invocation.
Matt Harbison - Dec. 16, 2018, 2:06 a.m.
On Sat, 15 Dec 2018 20:02:08 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sat, 15 Dec 2018 15:04:24 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1544855178 18000
>> #      Sat Dec 15 01:26:18 2018 -0500
>> # Node ID 068910232e124a50a13fd7444844d9151db48d6b
>> # Parent  5817c3b186a7799ecc3130493ba134b55cd4ba07
>> py3: ensure the proxied Windows fd doesn't escape by entering context  
>> manager
>
> Good catch. Queued these for stable because some of them will break  
> things
> if "with" statement is involved, and I can't be sure we have no such  
> invocation.

There are a couple of other odd ones.  fsmonitor.state_update[1],  
_AcquireFutures[2], and extensions.wrappedfunction[3] don't return  
anything from __enter__(), and bundle2.partiterator returns an iterator  
[4].  I thought not explicitly returning something means None is returned,  
and I *assume* something would be broke if partiterator is wrong.


[1]  
https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/hgext/fsmonitor/__init__.py#l661
[2]  
https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/thirdparty/concurrent/futures/_base.py#l149
[3]  
https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/extensions.py#l530
[4]  
https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/bundle2.py#l369
Yuya Nishihara - Dec. 16, 2018, 5:38 a.m.
On Sat, 15 Dec 2018 21:06:37 -0500, Matt Harbison wrote:
> On Sat, 15 Dec 2018 20:02:08 -0500, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sat, 15 Dec 2018 15:04:24 -0500, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1544855178 18000
> >> #      Sat Dec 15 01:26:18 2018 -0500
> >> # Node ID 068910232e124a50a13fd7444844d9151db48d6b
> >> # Parent  5817c3b186a7799ecc3130493ba134b55cd4ba07
> >> py3: ensure the proxied Windows fd doesn't escape by entering context  
> >> manager
> >
> > Good catch. Queued these for stable because some of them will break  
> > things
> > if "with" statement is involved, and I can't be sure we have no such  
> > invocation.
> 
> There are a couple of other odd ones.  fsmonitor.state_update[1],  
> _AcquireFutures[2], and extensions.wrappedfunction[3] don't return  
> anything from __enter__(), and bundle2.partiterator returns an iterator  
> [4].  I thought not explicitly returning something means None is returned,  
> and I *assume* something would be broke if partiterator is wrong.
> 
> 
> [1]  
> https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/hgext/fsmonitor/__init__.py#l661
> [2]  
> https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/thirdparty/concurrent/futures/_base.py#l149
> [3]  
> https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/extensions.py#l530
> [4]  
> https://www.mercurial-scm.org/repo/hg/file/e06719b7544d/mercurial/bundle2.py#l369

They are correct. What was wrong with the file proxies is that the caller
expects a wrapped file object to be returned.

Patch

diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -132,7 +132,10 @@  class fdproxy(object):
         self._fp = fp
 
     def __enter__(self):
-        return self._fp.__enter__()
+        self._fp.__enter__()
+        # Return this wrapper for the context manager so that the name is
+        # still available.
+        return self
 
     def __exit__(self, exc_type, exc_value, traceback):
         self._fp.__exit__(exc_type, exc_value, traceback)