From a704d5a04be055dc2b606db0cea3eea7df928fda Mon Sep 17 00:00:00 2001 From: Madhur Tandon Date: Tue, 25 Dec 2018 15:14:09 +0530 Subject: [PATCH 1/3] Implement the ABI number functionality. Modified ``Makefile`` and ``packages/Makefile`` to get ``PYODIDE_PACKAGE_ABI`` from ``Makefile.envs`` Made ``checkABI`` as a public API function. Modified ``file_packager.py`` to check for the compatible ABI number. Defined ABI number with which pyodide is built in Makefile.envs Accepted ABI number in ``file_packager.py`` as an argument. Modified ``buildpkg.py`` and ``buildall.py`` to account for the new argument. Added the corresponding ``test_different_ABI`` test. --- Makefile | 6 ++++-- Makefile.envs | 2 ++ packages/Makefile | 2 +- pyodide_build/buildall.py | 3 +++ pyodide_build/buildpkg.py | 4 ++++ src/pyodide.js | 11 ++++++++++- test/test_package_loading.py | 29 +++++++++++++++++++++++++++-- tools/file_packager.py | 8 ++++++++ 8 files changed, 59 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 238b53661..4b56b3dfe 100644 --- a/Makefile +++ b/Makefile @@ -72,7 +72,7 @@ build/pyodide.asm.js: src/main.bc src/jsimport.bc src/jsproxy.bc src/js2python.b build/pyodide.asm.data: root/.built ( \ cd build; \ - python $(FILEPACKAGER) pyodide.asm.data --lz4 --preload ../root/lib@lib --js-output=pyodide.asm.data.js --use-preload-plugins \ + python $(FILEPACKAGER) pyodide.asm.data --abi=$(PYODIDE_PACKAGE_ABI) --lz4 --preload ../root/lib@lib --js-output=pyodide.asm.data.js --use-preload-plugins \ ) uglifyjs build/pyodide.asm.data.js -o build/pyodide.asm.data.js @@ -80,11 +80,13 @@ build/pyodide.asm.data: root/.built build/pyodide_dev.js: src/pyodide.js cp $< $@ sed -i -e "s#{{DEPLOY}}##g" $@ + sed -i -e "s#{{ABI}}#$(PYODIDE_PACKAGE_ABI)#g" $@ build/pyodide.js: src/pyodide.js cp $< $@ sed -i -e 's#{{DEPLOY}}#https://iodide.io/pyodide-demo/#g' $@ + sed -i -e "s#{{ABI}}#$(PYODIDE_PACKAGE_ABI)#g" $@ build/python.html: src/python.html @@ -145,7 +147,7 @@ build/test.data: $(CPYTHONLIB) ) ( \ cd build; \ - python $(FILEPACKAGER) test.data --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python3.7/test --js-output=test.js --export-name=pyodide._module --exclude __pycache__ \ + python $(FILEPACKAGER) test.data --abi=$(PYODIDE_PACKAGE_ABI) --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python3.7/test --js-output=test.js --export-name=pyodide._module --exclude __pycache__ \ ) uglifyjs build/test.js -o build/test.js diff --git a/Makefile.envs b/Makefile.envs index eee1b79ba..3581259b9 100644 --- a/Makefile.envs +++ b/Makefile.envs @@ -15,6 +15,8 @@ export HOSTPYTHON=$(HOSTPYTHONROOT)/bin/python3 export TARGETPYTHONROOT=$(PYODIDE_ROOT)/cpython/installs/python-$(PYVERSION) export PYTHONINCLUDE=$(PYODIDE_ROOT)/cpython/installs/python-$(PYVERSION)/include/python$(PYMINOR) +export PYODIDE_PACKAGE_ABI=1 + export SIDE_LDFLAGS=\ -O3 \ -s "BINARYEN_METHOD='native-wasm'" \ diff --git a/packages/Makefile b/packages/Makefile index 0188c14fb..d7fd22a43 100644 --- a/packages/Makefile +++ b/packages/Makefile @@ -3,7 +3,7 @@ include ../Makefile.envs all: deps ../bin/pyodide buildall . ../build \ - --ldflags="$(SIDE_LDFLAGS)" --host=$(HOSTPYTHONROOT) --target=$(TARGETPYTHONROOT) + --package_abi=$(PYODIDE_PACKAGE_ABI) --ldflags="$(SIDE_LDFLAGS)" --host=$(HOSTPYTHONROOT) --target=$(TARGETPYTHONROOT) deps: # Install build dependencies $(HOSTPYTHON) -m pip install Cython Tempita diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index e237e4973..913bb9625 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -69,6 +69,9 @@ def make_parser(parser): parser.add_argument( 'output', type=str, nargs=1, help='Output directory in which to put all built packages') + parser.add_argument( + '--package_abi', type=int, required=True, + help='The ABI number for the packages to be built') parser.add_argument( '--cflags', type=str, nargs='?', default=common.DEFAULTCFLAGS, help='Extra compiling flags') diff --git a/pyodide_build/buildpkg.py b/pyodide_build/buildpkg.py index ad230d6a3..63421a1ba 100755 --- a/pyodide_build/buildpkg.py +++ b/pyodide_build/buildpkg.py @@ -127,6 +127,7 @@ def package_files(buildpath, srcpath, pkg, args): 'python', common.ROOTDIR / 'file_packager.py', name + '.data', + '--abi={0}'.format(args.package_abi), '--lz4', '--preload', '{}@/'.format(install_prefix), @@ -169,6 +170,9 @@ def make_parser(parser): parser.add_argument( 'package', type=str, nargs=1, help="Path to meta.yaml package description") + parser.add_argument( + '--package_abi', type=int, required=True, + help='The ABI number for the package to be built') parser.add_argument( '--cflags', type=str, nargs='?', default=common.DEFAULTCFLAGS, help='Extra compiling flags') diff --git a/src/pyodide.js b/src/pyodide.js index 3a763ab81..b0a90ae9e 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -179,7 +179,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { // If the package_uri fails to load, call monitorRunDependencies twice // (so packageCounter will still hit 0 and finish loading), and remove // the package from toLoad so we don't mark it as loaded. - console.log(`Couldn't load package from URL ${script.src}`) + console.error(`Couldn't load package from URL ${script.src}`) let index = toLoad.indexOf(package); if (index !== -1) { toLoad.splice(index, 1); @@ -247,6 +247,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { 'repr', 'runPython', 'runPythonAsync', + 'checkABI', 'version', ]; @@ -277,6 +278,14 @@ var languagePluginLoader = new Promise((resolve, reject) => { return {}; }; + Module.checkABI = function(ABI_number) { + if (ABI_number !== parseInt('{{ABI}}')) { + console.error(`ABI numbers differ. Expected {{ABI}}, got ${ABI_number}`); + return false; + } + return true; + }; + Module.locateFile = (path) => baseURL + path; var postRunPromise = new Promise((resolve, reject) => { Module.postRun = () => { diff --git a/test/test_package_loading.py b/test/test_package_loading.py index 86088f8ad..790e77fef 100644 --- a/test/test_package_loading.py +++ b/test/test_package_loading.py @@ -1,7 +1,7 @@ import pytest - -from pathlib import Path import shutil +import re +from pathlib import Path @pytest.mark.parametrize('active_server', ['main', 'secondary']) @@ -108,6 +108,31 @@ def test_load_packages_sequential(selenium_standalone, packages): assert selenium.logs.count(f'Loading {packages[1]}') == 1 +def test_different_ABI(selenium_standalone): + url = selenium_standalone.server_hostname + port = selenium_standalone.server_port + + build_dir = Path(__file__).parent.parent / 'build' + + original_file = open('build/numpy.js', 'r+') + original_contents = original_file.read() + original_file.close() + + modified_contents = re.sub(r'checkABI\(\d+\)', 'checkABI(-1)', + original_contents) + modified_file = open('build/numpy-broken.js', 'w+') + modified_file.write(modified_contents) + modified_file.close() + + try: + selenium_standalone.load_package( + f'http://{url}:{port}/numpy-broken.js' + ) + assert 'ABI numbers differ.' in selenium_standalone.logs + finally: + (build_dir / 'numpy-broken.js').unlink() + + def test_load_handle_failure(selenium_standalone): selenium = selenium_standalone selenium.load_package('pytz') diff --git a/tools/file_packager.py b/tools/file_packager.py index 0af4edab1..afe933d19 100644 --- a/tools/file_packager.py +++ b/tools/file_packager.py @@ -144,6 +144,10 @@ for arg in sys.argv[2:]: leading = 'preload' elif arg == '--embed': leading = 'embed' + elif arg.startswith('--abi'): + if '=' in arg: + package_abi = arg.split('=', 1)[1] + leading = '' elif arg == '--exclude': leading = 'exclude' elif arg == '--no-force': @@ -227,6 +231,10 @@ if not from_emcc: var Module = typeof %(EXPORT_NAME)s !== 'undefined' ? %(EXPORT_NAME)s : {}; ''' % {"EXPORT_NAME": export_name} +ret += ''' +Module.checkABI({0}); +'''.format(package_abi) + ret += ''' if (!Module.expectedDataFileDownloads) { Module.expectedDataFileDownloads = 0; From 37b0913563934c9e9251d002618444a05f17613b Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 22 Jan 2019 11:03:28 -0500 Subject: [PATCH 2/3] Fix error handling --- src/pyodide.js | 15 +++++++++++++-- test/test_package_loading.py | 8 ++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index b0a90ae9e..64612e8d3 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -159,6 +159,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { window.pyodide.loadedPackages[package] = toLoad[package]; } delete window.pyodide._module.monitorRunDependencies; + window.removeEventListener('error', windowErrorHandler); if (!isFirefox) { preloadWasm().then(() => {resolve(`Loaded ${packageList}`)}); } else { @@ -167,6 +168,17 @@ var languagePluginLoader = new Promise((resolve, reject) => { } }; + // Add a handler for any exceptions that are thrown in the process of + // loading a package + var windowErrorHandler = (err) => { + delete window.pyodide._module.monitorRunDependencies; + window.removeEventListener('error', windowErrorHandler); + // Set up a new Promise chain, since this one failed + loadPackagePromise = new Promise((resolve) => resolve()); + reject(err.message); + }; + window.addEventListener('error', windowErrorHandler); + for (let package in toLoad) { let script = document.createElement('script'); let package_uri = toLoad[package]; @@ -280,8 +292,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { Module.checkABI = function(ABI_number) { if (ABI_number !== parseInt('{{ABI}}')) { - console.error(`ABI numbers differ. Expected {{ABI}}, got ${ABI_number}`); - return false; + throw `ABI numbers differ. Expected {{ABI}}, got ${ABI_number}`; } return true; }; diff --git a/test/test_package_loading.py b/test/test_package_loading.py index 790e77fef..89520a340 100644 --- a/test/test_package_loading.py +++ b/test/test_package_loading.py @@ -132,6 +132,14 @@ def test_different_ABI(selenium_standalone): finally: (build_dir / 'numpy-broken.js').unlink() + selenium_standalone.load_package('kiwisolver') + selenium_standalone.run('import kiwisolver') + assert ( + selenium_standalone.run('repr(kiwisolver)') == + "" + ) + def test_load_handle_failure(selenium_standalone): selenium = selenium_standalone From cf7072d37b6700558e45eb8ff6b93a6e02588a61 Mon Sep 17 00:00:00 2001 From: Madhur Tandon Date: Tue, 22 Jan 2019 22:43:02 +0530 Subject: [PATCH 3/3] Used console.error to make the error of ABI mismatch appear in the logs --- src/pyodide.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pyodide.js b/src/pyodide.js index 64612e8d3..73a27ce71 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -292,7 +292,10 @@ var languagePluginLoader = new Promise((resolve, reject) => { Module.checkABI = function(ABI_number) { if (ABI_number !== parseInt('{{ABI}}')) { - throw `ABI numbers differ. Expected {{ABI}}, got ${ABI_number}`; + var ABI_mismatch_exception = + `ABI numbers differ. Expected {{ABI}}, got ${ABI_number}`; + console.error(ABI_mismatch_exception); + throw ABI_mismatch_exception; } return true; };