Skip to content

Update to Python 3.8.2#712

Merged
phorward merged 15 commits intopyodide:masterfrom
rth:py3.8
Jul 7, 2020
Merged

Update to Python 3.8.2#712
phorward merged 15 commits intopyodide:masterfrom
rth:py3.8

Conversation

@rth
Copy link
Member

@rth rth commented Jul 5, 2020

An attempt to update to Python 3.8.2

Supersedes and closes #594
Partially addresses #635

Currently the build of CPython works and a number of tests pass. Required patches can be found as commits in https://github.com/rth/cpython/commits/pyodide-py38

TODO

  • apply workarounds for failing CPython tests (e.g. that use threading)
  • check that the rest of the packages build sucessfully
  • possibly add a patch to emsdk as currently it produces the following warning for each emcc call,
    emsdk/emscripten/tag-1.38.31/emcc.py:819: SyntaxWarning: "is not" with a literal. Did you mean "!="?
    
    It's fixed in later emscripten versions but there isn't much progress updating emscripten version so far (e.g. Upgrade to emscripten 1.38.34 with fastcomp #480)
  • some of the packages included in pyodide might also need updating to be compatible with python 3.8 Can be done in follow up PRs.
  • this would also require using a Docker image with Python 3.8. A possible minor issue is that latest circleci/python:3.8.2-buster image include chromium 83, while tests fail for some reason with that version (see MNT Update to firefox 70.0 / geckodriver 0.26 in CI #622). So for the time being we might need to force installation of chromium 80 (which is currently used in CI).

cc @phorward

@phorward
Copy link
Contributor

phorward commented Jul 6, 2020

Hi @rth, this is great news, thanks for the initial work on this!

I hope you'll get the emscripten update fightened during the sprint. I'll take a look at the minor problems in a few days. What do you think about making a separate branch for python38 to merge further pull requests, especially for package contributions? We then might first concentrate on Pyodides core to run with Python 3.8 and then partially extend to the packages.

@rth
Copy link
Member Author

rth commented Jul 6, 2020

What do you think about making a separate branch for python38 to merge further pull requests, especially for package contributions?

I think this PR is not that far from being mergable. There are just 8 failing test modules in the CPython test suite (though I also still need to update the list of CPython tests). The rest of the packages appear to build fine (aside from cytoolz which I updated). Generally packages that worked with Python 3.7 will likely also work with 3.8 aside for some warnings. We can update packages and likely emscripten warning in a follow up PR.

Don't hesitate to either push fixes directly to this branch (you should have access being a maintainer) or open PRs to this branch on my fork. I'll also try to look into failing tests this week.

@rth
Copy link
Member Author

rth commented Jul 6, 2020

I think CPython tests should now pass. I have skipped a few using the async module or subprocess, and patched others.

The only blocker IMO is the test_open_url failure (and of other tests that use pyodide.open_url) both in Chrome and Firefox with,

selenium.common.exceptions.WebDriverException: Message: xpath lookup error: Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://127.0.0.1:39271/test/data.txt'.
E         (Session info: headless chrome=83.0.4103.116)
E         (Driver info: chromedriver=2.41.578700 (2f1ed5f9343c13f73144538f15c00b370eda6706),platform=Linux 5.4.0-39-generic x86_64)

/usr/local/lib/python3.8/site-packages/selenium/webdriver/remote/errorhandler.py:242: WebDriverException
------------------------------------------------------ Captured stdout teardown -------------------------------------------------------
Error: Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://127.0.0.1:39271/test/data.txt'.
    at _hiwire_call_member (http://127.0.0.1:39271/pyodide.asm.js:8:31734)
    at _JsBoundMethod_Call (http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[339]:0xfeb44)
    at http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[15117]:0x9ade0a
    at __PyObject_MakeTpCall (http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[828]:0x134900)
    at _call_function (http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[2832]:0x342b3d)
    at __PyEval_EvalFrameDefault (http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[2825]:0x33b99a)
    at http://127.0.0.1:39271/pyodide.asm.wasm:wasm-function[20875]:0x9c1c2e

which seems to indicate that something in hiwire is not working as it should and might need updating for Python 3.8

Edit: probably need to read through https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-the-c-api in mode detail.

Edit2: actually it was an issue with the test server.

@rth rth marked this pull request as draft July 6, 2020 20:49
@rth rth marked this pull request as ready for review July 7, 2020 09:00
make -C zlib clean
echo "The Emsdk, CPython and CLAPACK are not cleaned. cd into those directories to do so."

clean-all: clean
Copy link
Contributor

Choose a reason for hiding this comment

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

Great! 👍

@rth
Copy link
Member Author

rth commented Jul 7, 2020

@phorward This should now be ready for a review. The one lxml failure on chrome which happens from time to time is a known unrelated issue (#544 (comment)) everything else passes.

If you merge, don't forget to use the "Squash and Merge" UI button :)

Copy link
Contributor

@phorward phorward left a comment

Choose a reason for hiding this comment

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

Hello @rth, thank your very much for your effort in making this happen so quickly!
For me, this all looks good and works very nice, I checked it out locally.
image

@phorward phorward merged commit fc5495f into pyodide:master Jul 7, 2020
@rth rth deleted the py3.8 branch July 7, 2020 14:35
@rth rth added this to the 0.16.0 milestone Aug 16, 2020
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.

2 participants