Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/lib/pythongen.ml: Fix InternalError(str(exception)) for Py3 #175

Merged

Conversation

bernhardkaindl
Copy link
Contributor

@bernhardkaindl bernhardkaindl commented Jun 26, 2023

Cc: @edwintorok, @MarkSymsCtx, @lindig

In pythongen.ml, fix return InternalError(str(exn)).failure():

For Python3, compat_block defines str = bytes:

  ; Line "if sys.version_info[0] > 2:"
  ; Block [ Line "long = int"; Line "unicode = str"; Line "str = bytes" ]

The easy fix is to get a copy of str and use it to convert exn to str:

get_str = str
if PY3:
    str = bytes
...
    # In the generated code:
    return InternalError(get_str(exn).failure())

(Instead, we could also prepend def get_str(a): return str(a), this is just shorter. It would be fine either way.)

Without this fix, str(Exception()), compat_block causes it to be bytes(Exception()) on Py3. This would raise a TypeError because Exception can't be converted to bytes:

$ python3 -c 'str = bytes;  str(Exception("hi"))'

Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: cannot convert 'Exception' object to bytes

Setting get_str() to be str() for Py2 and Py3 fixes it and gets a str for InternalError.__init__(arg) (which expects a str value as argument).

In `src/lib/pythongen.ml`, fix `return InternalError(str(exn)).failure()`:

For Python3, `compat_block` defines `str = bytes`:
```ml
  ; Line "if sys.version_info[0] > 2:"
  ; Block [ Line "long = int"; Line "unicode = str"; Line "str = bytes" ]
```
The easy fix is to get a copy of `str` and use it to convert `exn` to `str`:
```py
get_str = str
if PY3:
    str = bytes
...
    # In the generated code:
    return InternalError(get_str(exn).failure())
```
(Instead, we could also prepend `def get_str(a): return str(a)`,
this is just shorter. It would be fine either way.)

Without this fix, `str(Exception())`, `compat_block` causes it to be
`bytes(Exception())` on Py3.  This would raise a `TypeError` because
`Exception` can't be converted to `bytes`:

```py
$ python3 -c 'str = bytes;  str(Exception("hi"))'

Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: cannot convert 'Exception' object to bytes
```

Setting `get_str()` to be `str()` for Py2 and Py3 fixes it and gets a `str`
for `InternalError.__init__(arg)` (which expects a `str` value as argument)

Co-authored-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
@bernhardkaindl bernhardkaindl force-pushed the Python3-Fix-InternalError-Creation branch from 8081913 to e7054a8 Compare June 26, 2023 13:20
Copy link
Collaborator

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this should allow us to finally move to Python3, at least after this change and a small tweak in XAPI the unit tests pass (and after a short transition period drop Python2 support completely, see #148 at which point we can probably make the 'str' to 'bytes' substitution in the OCaml code itself instead of overriding the names like this)

@mseri mseri merged commit cbcdec7 into mirage:master Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants