Simplify setup code (#1195)

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 \

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 <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`.

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;
 }

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");

src/core/runpython.c

@@ -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 <emscripten.h>
-
-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;
-}

src/core/runpython.h

@@ -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 */

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();
   };
 

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")

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]

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(
         """