Patchwork [1,of,2] util: forward __bool__()/__nonzero__() on fileobjectproxy

login
register
mail settings
Submitter Matt Harbison
Date March 10, 2018, 6:05 p.m.
Message ID <e21d0c21f81315d012c4.1520705126@Envy>
Download mbox | patch
Permalink /patch/29252/
State Accepted
Headers show

Comments

Matt Harbison - March 10, 2018, 6:05 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1520702528 18000
#      Sat Mar 10 12:22:08 2018 -0500
# Node ID e21d0c21f81315d012c4806359bc76aa40c89ad6
# Parent  963b4223d14fa419df2a82fbe47cd55075707b6a
util: forward __bool__()/__nonzero__() on fileobjectproxy

In trying to debug the Windows process hang in D2720, I changed the stderr pipe
to the peer to be os.devnull instead.  That caused sshpeer._cleanuppipes()[1] to
explode, complaining NoneType has no __iter__ attribute, even though the
previous line checked for None.

[1] https://www.mercurial-scm.org/repo/hg/file/b434965f984e/mercurial/sshpeer.py#l133
Gregory Szorc - March 10, 2018, 6:12 p.m.
On Sat, Mar 10, 2018 at 10:05 AM, Matt Harbison <mharbison72@gmail.com>
wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1520702528 18000
> #      Sat Mar 10 12:22:08 2018 -0500
> # Node ID e21d0c21f81315d012c4806359bc76aa40c89ad6
> # Parent  963b4223d14fa419df2a82fbe47cd55075707b6a
> util: forward __bool__()/__nonzero__() on fileobjectproxy
>

Good catch.

Queued, thanks.

Proxy objects are hard. FWIW, the most robust implementation I've seen is
http://wrapt.readthedocs.io/en/latest/. If you look at the source code and
bug history of that project, there is some serious magic required to
properly implement a proxy object! If we ever use proxies more seriously in
core, we should consider vendoring wrapt.


>
> In trying to debug the Windows process hang in D2720, I changed the stderr
> pipe
> to the peer to be os.devnull instead.  That caused
> sshpeer._cleanuppipes()[1] to
> explode, complaining NoneType has no __iter__ attribute, even though the
> previous line checked for None.
>
> [1] https://www.mercurial-scm.org/repo/hg/file/b434965f984e/
> mercurial/sshpeer.py#l133
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -551,6 +551,11 @@ class fileobjectproxy(object):
>
>          return getattr(object.__getattribute__(self, r'_orig'), name)
>
> +    def __nonzero__(self):
> +        return bool(object.__getattribute__(self, r'_orig'))
> +
> +    __bool__ = __nonzero__
> +
>      def __delattr__(self, name):
>          return delattr(object.__getattribute__(self, r'_orig'), name)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -551,6 +551,11 @@  class fileobjectproxy(object):
 
         return getattr(object.__getattribute__(self, r'_orig'), name)
 
+    def __nonzero__(self):
+        return bool(object.__getattribute__(self, r'_orig'))
+
+    __bool__ = __nonzero__
+
     def __delattr__(self, name):
         return delattr(object.__getattribute__(self, r'_orig'), name)