Patchwork [1,of,2] py3: conditionalize the raise statement

login
register
mail settings
Submitter Pulkit Goyal
Date Aug. 8, 2016, 7:05 p.m.
Message ID <9a344ce563ce2221bdfc.1470683137@waste.org>
Download mbox | patch
Permalink /patch/16213/
State Superseded
Headers show

Comments

Pulkit Goyal - Aug. 8, 2016, 7:05 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1470680471 -19800
#      Mon Aug 08 23:51:11 2016 +0530
# Node ID 9a344ce563ce2221bdfc9031b8f249d9da2f08b0
# Parent  37b6f0ec6241a62de90737409458cd622e2fac0d
py3: conditionalize the raise statement

raise E,V,T is not acceptable in Python 3, thats is conditionalized.
Moreover this will result in syntax error so we have to use exec() to
execute this. Related PEP- https://www.python.org/dev/peps/pep-3109/#id14

Moreover this patch also contain an update to test-check-py3-compat.t
Yuya Nishihara - Aug. 9, 2016, 3:49 p.m.
On Mon, 08 Aug 2016 14:05:37 -0500, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1470680471 -19800
> #      Mon Aug 08 23:51:11 2016 +0530
> # Node ID 9a344ce563ce2221bdfc9031b8f249d9da2f08b0
> # Parent  37b6f0ec6241a62de90737409458cd622e2fac0d
> py3: conditionalize the raise statement
> 
> raise E,V,T is not acceptable in Python 3, thats is conditionalized.
> Moreover this will result in syntax error so we have to use exec() to
> execute this. Related PEP- https://www.python.org/dev/peps/pep-3109/#id14
> 
> Moreover this patch also contain an update to test-check-py3-compat.t
> 
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -989,7 +989,10 @@
>              outdebug(ui, 'closing payload chunk')
>              # abort current part payload
>              yield _pack(_fpayloadsize, 0)
> -            raise exc_info[0], exc_info[1], exc_info[2]
> +            if sys.version_info[0] >= 3:
> +                raise exc_info[0](exc_info[1]).with_traceback(exc_info[2])
> +            else:
> +                exec("""raise exc_info[0], exc_info[1], exc_info[2]""")

This would be better fit for import-time hack, but it wouldn't be easy to
process at token level. So I'm okay for this change, though exec() would add
extra frame to traceback.

Any ideas?
Pulkit Goyal - Aug. 9, 2016, 4:34 p.m.
> Any ideas?

Sorry but I don't have one. My implementation is motivated from the
six implementation
https://bitbucket.org/gutworth/six/src/ca4580a5a648fc75abc568907e81abc80b05d58c/six.py?fileviewer=file-view-default#six.py-680
except they are defining a new function exec_() to prevent adding an
extra frame AFAIK :)
Yuya Nishihara - Aug. 10, 2016, 12:52 p.m.
On Tue, 9 Aug 2016 22:04:44 +0530, Pulkit Goyal wrote:
> My implementation is motivated from the
> six implementation
> https://bitbucket.org/gutworth/six/src/ca4580a5a648fc75abc568907e81abc80b05d58c/six.py?fileviewer=file-view-default#six.py-680
> except they are defining a new function exec_() to prevent adding an
> extra frame AFAIK :)

This is informative, I've copied it to the commit message of the V2 patch.
Martijn Pieters - Aug. 12, 2016, 3:58 p.m.
On 8 August 2016 at 20:05, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1470680471 -19800
> #      Mon Aug 08 23:51:11 2016 +0530
> # Node ID 9a344ce563ce2221bdfc9031b8f249d9da2f08b0
> # Parent  37b6f0ec6241a62de90737409458cd622e2fac0d
> py3: conditionalize the raise statement
>
> raise E,V,T is not acceptable in Python 3, thats is conditionalized.
> Moreover this will result in syntax error so we have to use exec() to
> execute this. Related PEP- https://www.python.org/dev/peps/pep-3109/#id14
>
> Moreover this patch also contain an update to test-check-py3-compat.t
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -989,7 +989,10 @@
>              outdebug(ui, 'closing payload chunk')
>              # abort current part payload
>              yield _pack(_fpayloadsize, 0)
> -            raise exc_info[0], exc_info[1], exc_info[2]
> +            if sys.version_info[0] >= 3:
> +                raise exc_info[0](exc_info[1]).with_traceback(exc_info[2])
> +            else:
> +                exec("""raise exc_info[0], exc_info[1], exc_info[2]""")

There is no need for an exec() call here. In Python 2 you can raise a
*tuple*, where the tuple consists of (type, value, traceback).

In other words, in Python 2, this is enough:

    raise exc_info

In Python 3, you want to raise `exc_info[1]`, don't create another
instance here, you are wrapping the exception value inside another
exception. You don't need to re-attach the traceback either; that same
traceback is *already there*

You could produce a helper function that produces the right 'format'
in Python 2 or 3:

    if sys.version_info[0] >= 3:
        import operator
        exc = operator.itemgetter(1)
    else:
        exc = tuple

then use

    raise exc(exc_info)


>          # end of payload
>          outdebug(ui, 'closing payload chunk')
>          yield _pack(_fpayloadsize, 0)
> diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
> --- a/tests/test-check-py3-compat.t
> +++ b/tests/test-check-py3-compat.t
> @@ -81,7 +81,7 @@
>    mercurial/archival.py: invalid syntax: invalid syntax (<unknown>, line *) (glob)
>    mercurial/bookmarks.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
>    mercurial/branchmap.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
> -  mercurial/bundle2.py: invalid syntax: invalid syntax (<unknown>, line *) (glob)
> +  mercurial/bundle2.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
>    mercurial/bundlerepo.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
>    mercurial/byterange.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
>    mercurial/changegroup.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
> @@ -136,13 +136,13 @@
>    mercurial/patch.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
>    mercurial/pathutil.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
>    mercurial/peer.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
> -  mercurial/pure/mpatch.py: error importing module: <ImportError> cannot import name 'pycompat' (line *) (glob)
> +  mercurial/pure/mpatch.py: error importing module: <ImportError> cannot import name 'policy' (line *) (glob)
>    mercurial/pure/osutil.py: error importing module: <ImportError> cannot import name 'policy' (line *) (glob)
>    mercurial/pure/parsers.py: error importing module: <ImportError> No module named 'mercurial.pure.node' (line *) (glob)
>    mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
>    mercurial/pvec.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
>    mercurial/registrar.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
> -  mercurial/repair.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
> +  mercurial/repair.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
>    mercurial/repoview.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
>    mercurial/revlog.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
>    mercurial/revset.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
> @@ -169,6 +169,6 @@
>    mercurial/verify.py: error importing: <TypeError> attribute name must be string, not 'bytes' (error at mdiff.py:*) (glob)
>    mercurial/win32.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
>    mercurial/windows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)
> -  mercurial/wireproto.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
> +  mercurial/wireproto.py: error importing module: <TypeError> a bytes-like object is required, not 'str' (line *) (glob)
>
>  #endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pulkit Goyal - Aug. 12, 2016, 6:16 p.m.
> In other words, in Python 2, this is enough:
>
>     raise exc_info

I tried this but unfortunately test-bundle2-format.t fails.
Martijn Pieters - Aug. 15, 2016, 10:08 a.m.
On 12 August 2016 at 19:16, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> I tried this but unfortunately test-bundle2-format.t fails.


Ah, yes, I was wrong. I should have known this; raising a tuple only
takes the first element (recursively, until an Exception class is
found). So so, there is no way around this other than using `exec`.
Bah, humbug. The Python 3 line can still be simplified however:

    raise exc_info[1]

is enough for that specific case; this time I actually ran the tests
and test-bundle2-format.t passes on Python 3.5.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -989,7 +989,10 @@ 
             outdebug(ui, 'closing payload chunk')
             # abort current part payload
             yield _pack(_fpayloadsize, 0)
-            raise exc_info[0], exc_info[1], exc_info[2]
+            if sys.version_info[0] >= 3:
+                raise exc_info[0](exc_info[1]).with_traceback(exc_info[2])
+            else:
+                exec("""raise exc_info[0], exc_info[1], exc_info[2]""")
         # end of payload
         outdebug(ui, 'closing payload chunk')
         yield _pack(_fpayloadsize, 0)
diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t
+++ b/tests/test-check-py3-compat.t
@@ -81,7 +81,7 @@ 
   mercurial/archival.py: invalid syntax: invalid syntax (<unknown>, line *) (glob)
   mercurial/bookmarks.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
   mercurial/branchmap.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
-  mercurial/bundle2.py: invalid syntax: invalid syntax (<unknown>, line *) (glob)
+  mercurial/bundle2.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
   mercurial/bundlerepo.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
   mercurial/byterange.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
   mercurial/changegroup.py: error importing: <TypeError> str expected, not bytes (error at encoding.py:*) (glob)
@@ -136,13 +136,13 @@ 
   mercurial/patch.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
   mercurial/pathutil.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
   mercurial/peer.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
-  mercurial/pure/mpatch.py: error importing module: <ImportError> cannot import name 'pycompat' (line *) (glob)
+  mercurial/pure/mpatch.py: error importing module: <ImportError> cannot import name 'policy' (line *) (glob)
   mercurial/pure/osutil.py: error importing module: <ImportError> cannot import name 'policy' (line *) (glob)
   mercurial/pure/parsers.py: error importing module: <ImportError> No module named 'mercurial.pure.node' (line *) (glob)
   mercurial/pushkey.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
   mercurial/pvec.py: error importing: <TypeError> getattr(): attribute name must be string (error at pycompat.py:*) (glob)
   mercurial/registrar.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
-  mercurial/repair.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
+  mercurial/repair.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/repoview.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/revlog.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/revset.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
@@ -169,6 +169,6 @@ 
   mercurial/verify.py: error importing: <TypeError> attribute name must be string, not 'bytes' (error at mdiff.py:*) (glob)
   mercurial/win32.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
   mercurial/windows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)
-  mercurial/wireproto.py: error importing module: <SyntaxError> invalid syntax (bundle2.py, line *) (line *) (glob)
+  mercurial/wireproto.py: error importing module: <TypeError> a bytes-like object is required, not 'str' (line *) (glob)
 
 #endif