From f64bee40d7db5835c78976d090569502eacde9cf Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 10 Feb 2021 15:39:01 -0800 Subject: [PATCH] Simplify setup code (#1195) --- Makefile | 1 - docs/development/core.md | 6 +- src/core/jsproxy.c | 4 +- src/core/main.c | 35 ++++++-- src/core/runpython.c | 129 ------------------------------ src/core/runpython.h | 12 --- src/pyodide.js | 55 ++++++++----- src/tests/test_console.py | 1 + src/tests/test_python.py | 7 -- src/tests/test_typeconversions.py | 6 ++ 10 files changed, 77 insertions(+), 179 deletions(-) delete mode 100644 src/core/runpython.c delete mode 100644 src/core/runpython.h diff --git a/Makefile b/Makefile index 719dac5d3..e9c4ef46c 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,6 @@ build/pyodide.asm.js: \ src/core/pyproxy.o \ src/core/python2js_buffer.o \ src/core/python2js.o \ - src/core/runpython.o \ src/pystone.py \ src/_testcapi.py \ src/webbrowser.py \ diff --git a/docs/development/core.md b/docs/development/core.md index 7b1d5a88d..8a2c92095 100644 --- a/docs/development/core.md +++ b/docs/development/core.md @@ -5,10 +5,8 @@ This file is intended as guidelines to help contributors trying to modify the C ## What the files do The primary purpose of `core` is to implement {ref}`type conversions ` between Python and Javascript. Here is a breakdown of the purposes of the files. -* `main.c` -- responsible for configuring and initializing the python interpreter, initializing the other source files, and creating the `_pyodide_core` module which is used to expose Python objects to `pyodide_py`. `main.c` also tries to generate fatal initialization error messages to help with debugging when there is a mistake in the initialization code. - -* `runpython` -- Defines the `_runPythonDebug` entrypoint to help in case there is a bug in `PyProxy.apply`. - +* `main` -- responsible for configuring and initializing the python interpreter, initializing the other source files, and creating the `_pyodide_core` module which is used to expose Python objects to `pyodide_py`. `main.c` also tries to generate fatal initialization error messages to help with debugging when there is a mistake in the initialization code. +* `keyboard_interrupt` -- This sets up the keyboard interrupts system for using Pyodide with a webworker. ### Backend utilities * `hiwire` -- A helper framework. It is impossible for wasm to directly hold owning references to javascript objects. The primary purpose of hiwire is to act as a surrogate owner for javascript references by holding the references in a javascript `Map`. `hiwire` also defines a wide variety of `EM_JS` helper functions to do javascript operations on the held objects. The primary type that hiwire exports is `JsRef`. References are created with `Module.hiwire.new_value` (only can be done from javascript) and must be destroyed from C with `hiwire_decref` or `hiwire_CLEAR`, or from javascript with `Module.hiwire.decref`. diff --git a/src/core/jsproxy.c b/src/core/jsproxy.c index 6ab0eb6e6..195c1865d 100644 --- a/src/core/jsproxy.c +++ b/src/core/jsproxy.c @@ -887,7 +887,6 @@ JsProxy_init(PyObject* core_module) bool success = false; PyObject* asyncio_module = NULL; - PyObject* pyodide_module = NULL; asyncio_module = PyImport_ImportModule("asyncio"); FAIL_IF_NULL(asyncio_module); @@ -901,7 +900,7 @@ JsProxy_init(PyObject* core_module) JsMethodType.tp_base = &JsProxyType; JsBufferType.tp_base = &JsProxyType; - // Add JsException to the pyodide module so people can catch it if they want. + // Add JsException to core_module so people can catch it if they want. FAIL_IF_MINUS_ONE(PyModule_AddType(core_module, &JsProxyType)); FAIL_IF_MINUS_ONE(PyModule_AddType(core_module, &JsBufferType)); FAIL_IF_MINUS_ONE(PyModule_AddType(core_module, &JsMethodType)); @@ -910,6 +909,5 @@ JsProxy_init(PyObject* core_module) success = true; finally: Py_CLEAR(asyncio_module); - Py_CLEAR(pyodide_module); return success ? 0 : -1; } diff --git a/src/core/main.c b/src/core/main.c index 7e338d876..9e1b0ebc4 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -11,7 +11,6 @@ #include "keyboard_interrupt.h" #include "pyproxy.h" #include "python2js.h" -#include "runpython.h" #define FATAL_ERROR(args...) \ do { \ @@ -73,6 +72,19 @@ static struct PyModuleDef core_module_def = { .m_size = -1, }; +PyObject* init_dict; + +void +run_python_simple_inner(char* code) +{ + PyObject* result = PyRun_String(code, Py_file_input, init_dict, init_dict); + if (result == NULL) { + pythonexc2js(); + } else { + Py_DECREF(result); + } +} + int main(int argc, char** argv) { @@ -96,7 +108,7 @@ main(int argc, char** argv) TRY_INIT(hiwire); TRY_INIT(error_handling); TRY_INIT(js2python); - TRY_INIT_WITH_CORE_MODULE(JsProxy); // JsProxy needs to be before JsImport + TRY_INIT_WITH_CORE_MODULE(JsProxy); TRY_INIT(pyproxy); TRY_INIT(python2js); TRY_INIT(keyboard_interrupt); @@ -106,9 +118,22 @@ main(int argc, char** argv) FATAL_ERROR("Failed to add '_pyodide_core' module to modules dict."); } - // pyodide.py imported for these two. - // They should appear last so that core_module is ready. - TRY_INIT(runpython); + init_dict = PyDict_New(); + JsRef init_dict_proxy = python2js(init_dict); + EM_ASM( + { + Module.init_dict = Module.hiwire.pop_value($0); + Module.runPythonSimple = function(code) + { + let code_c_string = stringToNewUTF8(code); + try { + _run_python_simple_inner(code_c_string); + } finally { + _free(code_c_string); + } + }; + }, + init_dict_proxy); Py_CLEAR(core_module); printf("Python initialization complete\n"); diff --git a/src/core/runpython.c b/src/core/runpython.c deleted file mode 100644 index 5d0f8bab6..000000000 --- a/src/core/runpython.c +++ /dev/null @@ -1,129 +0,0 @@ -#define PY_SSIZE_T_CLEAN -#include "Python.h" - -#include "error_handling.h" -#include "hiwire.h" -#include "pyproxy.h" -#include "python2js.h" -#include "runpython.h" - -#include - -static PyObject* pyodide_py; -static PyObject* globals; -_Py_IDENTIFIER(eval_code); - -JsRef -_runPythonDebug(char* code) -{ - PyObject* py_code; - py_code = PyUnicode_FromString(code); - if (py_code == NULL) { - fprintf(stderr, "runPythonDebug -- error occurred converting argument:\n"); - PyErr_Print(); - return Js_undefined; - } - - PyObject* result = _PyObject_CallMethodIdObjArgs( - pyodide_py, &PyId_eval_code, py_code, globals, NULL); - - if (result == NULL) { - fprintf(stderr, "runPythonDebug -- error occurred\n"); - PyErr_Print(); - return Js_undefined; - } - - printf("runPythonDebug -- eval_code succeeded, it returned:\n"); - PyObject_Print(result, stdout, 0); - - printf("runPythonDebug -- doing python2js(result):\n"); - JsRef id = python2js(result); - Py_DECREF(result); - return id; -} - -EM_JS_NUM(int, - runpython_init_js, - (JsRef pyodide_py_proxy, JsRef globals_proxy), - { - Module.pyodide_py = Module.hiwire.get_value(pyodide_py_proxy); - Module.globals = Module.hiwire.get_value(globals_proxy); - - // Use this to test python code separate from pyproxy.apply. - Module.runPythonDebug = function(code) - { - let pycode = stringToNewUTF8(code); - let idresult = Module.__runPythonDebug(pycode); - let jsresult = Module.hiwire.get_value(idresult); - Module.hiwire.decref(idresult); - _free(pycode); - return jsresult; - }; - return 0; - }); - -int -runpython_init() -{ - bool success = false; - JsRef pyodide_py_proxy = NULL; - JsRef globals_proxy = NULL; - - // I'm a bit confused about this deal with globals and builtins: - // 1. Why are we using __main__.__dict__ as globals? Shouldn't we make a fresh - // dictionary? - // 2. Why do we dump "builtins" directly into the dict? Normally we would - // leave the builtins where they belong as globals().__builtins__. - - // borrowed - PyObject* builtins = PyImport_AddModule("builtins"); - FAIL_IF_NULL(builtins); - - // borrowed - PyObject* builtins_dict = PyModule_GetDict(builtins); - FAIL_IF_NULL(builtins_dict); - - // borrowed - PyObject* __main__ = PyImport_AddModule("__main__"); - FAIL_IF_NULL(__main__); - - // globals is static variable, borrowed - globals = PyModule_GetDict(__main__); - FAIL_IF_NULL(globals); - Py_INCREF(globals); // to owned - - FAIL_IF_MINUS_ONE(PyDict_Update(globals, builtins_dict)); - - // pyodide_py is static variable, new - pyodide_py = PyImport_ImportModule("pyodide"); - FAIL_IF_NULL(pyodide_py); - - pyodide_py_proxy = python2js(pyodide_py); - FAIL_IF_NULL(pyodide_py_proxy); - // Currently by default, python2js copies dicts into objects. - // We want to feed Module.globals back to `eval_code` in `pyodide.runPython` - // (see definition in pyodide.js) but because the round trip conversion - // py => js => py for a dict object is a JsProxy, that causes trouble. - // Instead we explicitly call pyproxy_new. - // We also had to add ad-hoc modifications to _pyproxy_get, etc to support - // this. I (HC) will fix this with the rest of the type conversions - // modifications. - Py_INCREF(globals); // pyproxy_new steals argument - globals_proxy = pyproxy_new(globals); - FAIL_IF_NULL(globals_proxy); - FAIL_IF_MINUS_ONE(runpython_init_js(pyodide_py_proxy, globals_proxy)); - - success = true; -finally: - hiwire_decref(pyodide_py_proxy); - hiwire_decref(globals_proxy); - if (success) { - return 0; - } - if (PyErr_Occurred()) { - PyErr_Print(); - } - Py_CLEAR(pyodide_py); - Py_CLEAR(globals); - return -1; -} diff --git a/src/core/runpython.h b/src/core/runpython.h deleted file mode 100644 index 1c458953c..000000000 --- a/src/core/runpython.h +++ /dev/null @@ -1,12 +0,0 @@ -#ifndef RUNPYTHON_H -#define RUNPYTHON_H -#define PY_SSIZE_T_CLEAN -#include "Python.h" - -/** The primary entry point function that runs Python code. - */ - -int -runpython_init(); - -#endif /* RUNPYTHON_H */ diff --git a/src/pyodide.js b/src/pyodide.js index 32ae206f3..1b3ff3985 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -89,7 +89,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { } function recursiveDependencies(names, _messageCallback, errorCallback) { - const packages = self.pyodide._module.packages.dependencies; + const packages = Module.packages.dependencies; const loadedPackages = self.pyodide.loadedPackages; const toLoad = new Map(); @@ -132,7 +132,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { // locateFile is the function used by the .js file to locate the .data // file given the filename - self.pyodide._module.locateFile = (path) => { + Module.locateFile = (path) => { // handle packages loaded from custom URLs let pkg = path.replace(/\.data$/, ""); if (toLoad.has(pkg)) { @@ -208,7 +208,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { // no pending runDependencies. function waitRunDependency() { const promise = new Promise(r => { - self.pyodide._module.monitorRunDependencies = (n) => { + Module.monitorRunDependencies = (n) => { if (n === 0) { r(); } @@ -217,8 +217,8 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { // If there are no pending dependencies left, monitorRunDependencies will // never be called. Since we can't check the number of dependencies, // manually trigger a call. - self.pyodide._module.addRunDependency("dummy"); - self.pyodide._module.removeRunDependency("dummy"); + Module.addRunDependency("dummy"); + Module.removeRunDependency("dummy"); return promise; } @@ -229,7 +229,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { try { await Promise.race([ successPromise, windowErrorPromise ]); } finally { - delete self.pyodide._module.monitorRunDependencies; + delete Module.monitorRunDependencies; if (windowErrorHandler) { self.removeEventListener('error', windowErrorHandler); } @@ -251,14 +251,14 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { if (!isFirefox) { await preloadWasm(); - self.pyodide._module.reportUndefinedSymbols(); + Module.reportUndefinedSymbols(); } messageCallback(resolveMsg); // We have to invalidate Python's import caches, or it won't // see the new files. - self.pyodide.runPython('import importlib as _importlib\n' + - '_importlib.invalidate_caches()\n'); + Module.runPythonSimple('import importlib\n' + + 'importlib.invalidate_caches()\n'); }; // This is a promise that is resolved iff there are no pending package loads. @@ -322,7 +322,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { if (recursionLimit > 1000) { recursionLimit = 1000; } - pyodide.runPython( + pyodide.runPythonSimple( `import sys; sys.setrecursionlimit(int(${recursionLimit}))`); }; @@ -442,8 +442,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { if (imports.length === 0) { return; } - let packageNames = - self.pyodide._module.packages.import_name_to_package_name; + let packageNames = Module.packages.import_name_to_package_name; let packages = new Set(); for (let name of imports) { if (name in packageNames) { @@ -497,6 +496,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { return Module.runPython(code); }; + // clang-format off /** * Registers the Js object ``module`` as a Js module with ``name``. * This module can then be imported from Python using the standard Python @@ -509,8 +509,9 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { * @param {string} name Name of js module to add * @param {object} module Javascript object backing the module */ - Module.registerJsModule = function( - name, module) { Module.pyodide_py.register_js_module(name, module); }; + Module.registerJsModule = function(name, module) { + Module.pyodide_py.register_js_module(name, module); + }; /** * Unregisters a Js module with given name that has been previously registered @@ -523,8 +524,10 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { * * @param {string} name Name of js module to remove */ - Module.unregisterJsModule = function( - name) { Module.pyodide_py.unregister_js_module(name); }; + Module.unregisterJsModule = function(name) { + Module.pyodide_py.unregister_js_module(name); + }; + // clang-format on Module.function_supports_kwargs = function(funcstr) { // This is basically a finite state machine (except for paren counting) @@ -617,15 +620,31 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { Module.locateFile = (path) => baseURL + path; Module.postRun = async () => { - Module.version = Module.pyodide_py.__version__; + // Unfortunately the indentation here matters. + Module.runPythonSimple(` +def temp(Module): + import pyodide + import __main__ + import builtins + + globals = __main__.__dict__ + globals.update(builtins.__dict__) + + Module.version = pyodide.__version__ + Module.globals = globals + Module.pyodide_py = pyodide + `); + Module.init_dict["temp"](Module); + delete self.Module; let response = await fetch(`${baseURL}packages.json`); let json = await response.json(); + fixRecursionLimit(self.pyodide); self.pyodide = makePublicAPI(self.pyodide, PUBLIC_API); self.pyodide.registerJsModule("js", globalThis); self.pyodide.registerJsModule("pyodide_js", self.pyodide); - self.pyodide._module.packages = json; + Module.packages = json; resolve(); }; diff --git a/src/tests/test_console.py b/src/tests/test_console.py index 974144105..20fe04714 100644 --- a/src/tests/test_console.py +++ b/src/tests/test_console.py @@ -128,6 +128,7 @@ def test_repr(safe_sys_redirections): @pytest.fixture def safe_selenium_sys_redirections(selenium): + selenium.run("import sys") selenium.run("_redirected = sys.stdout, sys.stderr, sys.displayhook") yield selenium.run("sys.stdout, sys.stderr, sys.displayhook = _redirected") diff --git a/src/tests/test_python.py b/src/tests/test_python.py index c19e59f83..6caa98d58 100644 --- a/src/tests/test_python.py +++ b/src/tests/test_python.py @@ -167,10 +167,3 @@ def test_unknown_attribute(selenium): assert "asdf" in str(e) """ ) - - -def test_run_python_debug(selenium): - assert selenium.run_js("return pyodide._module.runPythonDebug('1+1');") == 2 - assert selenium.run_js( - "return pyodide._module.runPythonDebug('[x*x + 1 for x in range(4)]').deepCopyToJavascript();" - ) == [1, 2, 5, 10] diff --git a/src/tests/test_typeconversions.py b/src/tests/test_typeconversions.py index cfeb94532..4855b172c 100644 --- a/src/tests/test_typeconversions.py +++ b/src/tests/test_typeconversions.py @@ -56,6 +56,12 @@ def test_pythonexc2js(selenium): selenium.run_js('return pyodide.runPython("5 / 0")') +def test_run_python_simple_error(selenium): + msg = "ZeroDivisionError" + with pytest.raises(selenium.JavascriptException, match=msg): + selenium.run_js("return pyodide._module.runPythonSimple('5 / 0');") + + def test_js2python(selenium): selenium.run_js( """