Use upstream file packager instead of fork (pyodide#991)

bus3297 · Jan 1, 2021 · 27e2800 · 27e2800
1 parent ba2d394
commit 27e2800
Show file tree

Hide file tree

Showing 17 changed files with 85 additions and 1,089 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,4 +1,4 @@
-exclude: '^.*patches|file_packager.*|.*\.cgi$'
+exclude: '^.*patches|.*\.cgi$'
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v2.3.0

diff --git a/Makefile b/Makefile
@@ -2,7 +2,7 @@ PYODIDE_ROOT=$(abspath .)
 include Makefile.envs
 .PHONY=check
 
-FILEPACKAGER=$(PYODIDE_ROOT)/tools/file_packager.py
+FILEPACKAGER=$(PYODIDE_ROOT)/emsdk/emsdk/fastcomp/emscripten/tools/file_packager.py
 
 CPYTHONROOT=cpython
 CPYTHONLIB=$(CPYTHONROOT)/installs/python-$(PYVERSION)/lib/python$(PYMINOR)
@@ -17,7 +17,6 @@ OPTFLAGS=-O2
 CFLAGS=$(OPTFLAGS) -g -I$(PYTHONINCLUDE) -Wno-warn-absolute-paths -Werror=int-conversion -Werror=incompatible-pointer-types
 CXXFLAGS=$(CFLAGS) -std=c++14
 
-
 LDFLAGS=\
 	-O2 \
 	-s MODULARIZE=1 \
@@ -53,7 +52,6 @@ SITEPACKAGES=root/lib/python$(PYMINOR)/site-packages
 
 all: check \
 	build/pyodide.asm.js \
-	build/pyodide.asm.data \
 	build/pyodide.js \
 	build/console.html \
 	build/renderedhtml.css \
@@ -70,31 +68,22 @@ build/pyodide.asm.js: src/main.bc src/type_conversion/jsimport.bc \
 		src/type_conversion/pyimport.bc src/type_conversion/pyproxy.bc \
 		src/type_conversion/python2js.bc \
 		src/type_conversion/python2js_buffer.bc \
-		src/type_conversion/runpython.bc src/type_conversion/hiwire.bc
+		src/type_conversion/runpython.bc src/type_conversion/hiwire.bc \
+		root/.built
 	date +"[%F %T] Building pyodide.asm.js..."
 	[ -d build ] || mkdir build
-	$(CXX) -s EXPORT_NAME="'pyodide'" -o build/pyodide.asm.html $(filter %.bc,$^) \
-		$(LDFLAGS) -s FORCE_FILESYSTEM=1
-	rm build/pyodide.asm.html
+	$(CXX) -s EXPORT_NAME="'pyodide'" -o build/pyodide.asm.js $(filter %.bc,$^) \
+		$(LDFLAGS) -s FORCE_FILESYSTEM=1 --preload-file root/lib@lib
 	date +"[%F %T] done building pyodide.asm.js."
 
 
 env:
 	env
 
 
-build/pyodide.asm.data: root/.built
-	( \
-		cd build; \
-		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
-
-
 build/pyodide.js: src/pyodide.js
 	cp $< $@
 	sed -i -e 's#{{ PYODIDE_BASE_URL }}#$(PYODIDE_BASE_URL)#g' $@
-	sed -i -e "s#{{ PYODIDE_PACKAGE_ABI }}#$(PYODIDE_PACKAGE_ABI)#g" $@
 
 
 build/test.html: src/templates/test.html
@@ -125,7 +114,7 @@ lint:
 	# check for unused imports, the rest is done by black
 	flake8 --select=F401 src tools pyodide_build benchmark
 	clang-format-6.0 -output-replacements-xml `find src -type f -regex ".*\.\(c\|h\|js\)"` | (! grep '<replacement ')
-	black --check --exclude tools/file_packager.py .
+	black --check .
 	mypy --ignore-missing-imports pyodide_build/ src/ packages/micropip/micropip/ packages/*/test*
 
 
@@ -167,7 +156,7 @@ build/test.data: $(CPYTHONLIB)
 	)
 	( \
 		cd build; \
-		python $(FILEPACKAGER) test.data --abi=$(PYODIDE_PACKAGE_ABI) --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python3.8/test --js-output=test.js --export-name=pyodide._module --exclude __pycache__ \
+		python $(FILEPACKAGER) test.data --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python3.8/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
@@ -17,6 +17,9 @@ export PYTHONINCLUDE=$(PYODIDE_ROOT)/cpython/installs/python-$(PYVERSION)/includ
 
 # Use env variable if defined, otherwise fallback to './'
 export PYODIDE_BASE_URL?=./
-# This env variable is used to detect pyodide at build time,
-# do not rename it.
+
+# This environment variable is used for packages to detect if they are built
+# for pyodide during build time
+export PYODIDE=1
+# This is the legacy environment variable used for the aforementioned purpose
 export PYODIDE_PACKAGE_ABI=1
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -8,11 +8,18 @@
   iodide.
   [#878](https://github.com/iodide-project/pyodide/pull/878),
   [#981](https://github.com/iodide-project/pyodide/pull/981)
+- Use upstream `file_packager.py`, and stop checking package abi versions.
+  The `PYODIDE_PACKAGE_ABI` environment variable is no longer used, but is
+  still set as some packages use it to detect whether it is being built for
+  pyodide. This usage is deprecated, and a new environment variable `PYODIDE`
+  is introduced for this purpose.
+
+  As part of the change, Module.checkABI is no longer present.
+  [#991](https://github.com/iodide-project/pyodide/pull/991)
 
 ### Added
 - `micropip` now supports installing wheels from relative urls. [#872](https://github.com/iodide-project/pyodide/pull/872)
 
-
 ## Version 0.16.1
 *December 25, 2020*
 

diff --git a/docs/faq.md b/docs/faq.md
@@ -63,6 +63,8 @@ To detect pyodide, **at build time** use,
 ```python
 import os
 
-if "PYODIDE_PACKAGE_ABI" in os.environ:
+if "PYODIDE" in os.environ:
     # building for Pyodide
 ```
+We used to use the environment variable `PYODIDE_BASE_URL` for this purpose,
+but this usage is deprecated.
diff --git a/docs/new_packages.md b/docs/new_packages.md
@@ -179,17 +179,11 @@ also `scipy` and `CLAPACK`).
 built manually. They can be used by adding e.g. `-s USE_ZLIB` in the `cflags`
 of the python package. See e.g. `matplotlib` for an example.
 
-## Manual creation of a Pyodide package (advanced)
-The previous sections describes how to add a python package to the pyodide
-build.
+## Structure of a Pyodide package
+This section describes the structure of a pure python package, and how our
+build system creates it (In general, it is not recommended, to construct these
+by hand; instead create a Python wheel and install it with micropip)
 
-There are cases where you want to ship additional python libraries without
-adding it to pyodide itself. For pure python packages, this can be achieved
-reasonably easily. The most straightforward way is to create a Python wheel and
-load it with micropip. Alternatively, we can construct a python package
-manually.
-
-It is helpful to have some understanding of the structure of a Pyodide package.
 Pyodide is obtained by compiling CPython into web assembly. As such, it loads
 packages the same way as CPython --- it looks for relevant files `.py` files in
 `/lib/python3.x/`. When creating and loading a package, our job is to put our
@@ -209,23 +203,22 @@ contents to emscripten's virtual filesystem. Afterwards, since the files are
 now in `/lib/python3.8/`, running `import PACKAGE_NAME` in python will
 successfully import the module as usual.
 
-To produce these files, download the `file_packager.py` script from
-[https://github.com/iodide-project/pyodide/blob/master/tools/file_packager.py](https://github.com/iodide-project/pyodide/blob/master/tools/file_packager.py). You then run the command
+To construct this bundle, we use the `file_packager.py` script from emscripten.
+We invoke it as follows:
 ```sh
-$ ./file_packager.py PACKAGE_NAME.data --js-output=PACKAGE_NAME.js --abi=1 --export-name=pyodide._module --use-preload-plugins --preload /PATH/TO/LIB/@/lib/python3.8/site-packages/PACKAGE_NAME/ --exclude "*__pycache__*"
+$ ./file_packager.py PACKAGE_NAME.data \
+     --js-output=PACKAGE_NAME.js \
+     --export-name=pyodide._module \
+     --use-preload-plugins \
+     --preload /PATH/TO/LIB/@/lib/python3.8/site-packages/PACKAGE_NAME/ \
+     --exclude "*__pycache__*" \
+     --lz4
 ```
-The `--preload` argument instructs the package to look for the file/directory
-before the separator `@` (namely `/PATH/TO/LIB/`) and place it at the path
-after the `@` in the virtual filesystem (namely
-`/lib/python3.8/site-packages/PACKAGE_NAME/`). Remember to use the correct python version in the target path. At the time of writing, the latest release of Pyodide uses python 3.7 while git master uses python 3.8.
-
-The `--exclude` argument
-specifies files to omit from the package. This argument can be repeated, e.g.
-you can append `--exclude README.md` to the command.
-
-**Remark.** The bundled Pyodide packages uses lz4 compression when producing
-`PACKAGE_NAME.data`. These instructions skip this step as it requires
-additional dependencies, which complicates the process. In general, lz4
-compression decreases memory usage and can increase performance. On the other
-hand, if your webserver serves the files with gzip compression, pre-compressing
-with lz4 could in fact increase the number of bytes transferred.
+
+The arguments can be explained as follows:
+ - The `--preload` argument instructs the package to look for the
+   file/directory before the separator `@` (namely `/PATH/TO/LIB/`) and place
+   it at the path after the `@` in the virtual filesystem (namely
+   `/lib/python3.8/site-packages/PACKAGE_NAME/`).
+ - The `--exclude` argument specifies files to omit from the package.
+ - The `--lz4` argument says to use LZ4 to compress the files
diff --git a/emsdk/patches/packager.patch b/emsdk/patches/packager.patch
@@ -0,0 +1,16 @@
+This fixes file_packager.py when used out of emcc, since LZ4 is no
+longer in the namespace
+
+--- a/emsdk/fastcomp/emscripten/tools/file_packager.py
++++ b/emsdk/fastcomp/emscripten/tools/file_packager.py
+@@ -530,8 +530,8 @@
+     use_data = '''
+           var compressedData = %s;
+           compressedData.data = byteArray;
+-          assert(typeof LZ4 === 'object', 'LZ4 not present - was your app build with  -s LZ4=1  ?');
+-          LZ4.loadPackage({ 'metadata': metadata, 'compressedData': compressedData });
++          assert(typeof Module.LZ4 === 'object', 'LZ4 not present - was your app build with  -s LZ4=1  ?');
++          Module.LZ4.loadPackage({ 'metadata': metadata, 'compressedData': compressedData });
+           Module['removeRunDependency']('datafile_%s');
+     ''' % (meta, shared.JS.escape_for_js_string(data_target))
+
diff --git a/packages/Makefile b/packages/Makefile
@@ -5,7 +5,7 @@ include ../Makefile.envs
 all:
 	mkdir -p $(PYODIDE_LIBRARIES)
 	PYTHONPATH=$(PYODIDE_LIBRARIES)/lib/python ../bin/pyodide buildall . ../build \
-		--package_abi=$(PYODIDE_PACKAGE_ABI) --target=$(TARGETPYTHONROOT) --only $(PYODIDE_PACKAGES) --install-dir $(PYODIDE_LIBRARIES)
+		--target=$(TARGETPYTHONROOT) --only $(PYODIDE_PACKAGES) --install-dir $(PYODIDE_LIBRARIES)
 
 update-all:
 	for pkg in $$(find . -maxdepth 1 -type d -exec basename {} \; | tail -n +2); do \

diff --git a/packages/matplotlib/patches/fix-freetype.patch b/packages/matplotlib/patches/fix-freetype.patch
@@ -7,7 +7,7 @@ diff --git a/setupext.py b/setupext.py
      name = "freetype"
 
      def add_flags(self, ext):
-+        if "PYODIDE_PACKAGE_ABI" in os.environ:
++        if "PYODIDE" in os.environ:
 +            ext.libraries.append('freetype')
 +            return
          ext.sources.insert(0, 'src/checkdep_freetype2.c')

diff --git a/packages/pillow/patches/setitup.patch b/packages/pillow/patches/setitup.patch
@@ -5,7 +5,7 @@
          if feature.want("zlib"):
              _dbg("Looking for zlib")
 -            if _find_include_file(self, "zlib.h"):
-+            if "PYODIDE_PACKAGE_ABI" in os.environ:
++            if "PYODIDE" in os.environ:
 +                feature.zlib = "z"
 +            elif _find_include_file(self, "zlib.h"):
                  if _find_library_file(self, "z"):
@@ -16,7 +16,7 @@
          if feature.want("jpeg"):
              _dbg("Looking for jpeg")
 -            if _find_include_file(self, "jpeglib.h"):
-+            if "PYODIDE_PACKAGE_ABI" in os.environ:
++            if "PYODIDE" in os.environ:
 +                feature.jpeg = "jpeg"
 +            elif _find_include_file(self, "jpeglib.h"):
                  if _find_library_file(self, "jpeg"):
@@ -27,7 +27,7 @@
          if feature.want("freetype"):
              _dbg("Looking for freetype")
 -            if _find_library_file(self, "freetype"):
-+            if "PYODIDE_PACKAGE_ABI" in os.environ:
++            if "PYODIDE" in os.environ:
 +                feature.freetype = "freetype"
 +            elif _find_library_file(self, "freetype"):
                  # look for freetype2 include files
@@ -38,7 +38,7 @@
          defs = []
          if feature.jpeg:
 -            libs.append(feature.jpeg)
-+            if "PYODIDE_PACKAGE_ABI" not in os.environ:
++            if "PYODIDE" not in os.environ:
 +                libs.append(feature.jpeg)
              defs.append(("HAVE_LIBJPEG", None))
          if feature.jpeg2000:

diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py
@@ -56,8 +56,6 @@ def build(self, outputdir: Path, args) -> None:
                         "pyodide_build",
                         "buildpkg",
                         str(self.pkgdir / "meta.yaml"),
-                        "--package_abi",
-                        str(args.package_abi),
                         "--cflags",
                         args.cflags,
                         "--ldflags",
@@ -258,12 +256,6 @@ def make_parser(parser):
         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,

diff --git a/pyodide_build/buildpkg.py b/pyodide_build/buildpkg.py
@@ -183,9 +183,8 @@ def package_files(buildpath: Path, srcpath: Path, pkg: Dict[str, Any], args):
     subprocess.run(
         [
             "python",
-            common.ROOTDIR / "file_packager.py",
+            common.PACKAGERDIR / "file_packager.py",
             name + ".data",
-            "--abi={0}".format(args.package_abi),
             "--lz4",
             "--preload",
             "{}@/".format(install_prefix),
@@ -268,12 +267,6 @@ def make_parser(parser: argparse.ArgumentParser):
     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,

diff --git a/pyodide_build/common.py b/pyodide_build/common.py
@@ -1,8 +1,10 @@
 from pathlib import Path
 from typing import Optional, Set
 
-ROOTDIR = Path(__file__).parents[1].resolve() / "tools"
-TARGETPYTHON = ROOTDIR / ".." / "cpython" / "installs" / "python-3.8.2"
+ROOTDIR = Path(__file__).parents[1].resolve()
+TOOLSDIR = ROOTDIR / "tools"
+PACKAGERDIR = ROOTDIR / "emsdk" / "emsdk" / "fastcomp" / "emscripten" / "tools"
+TARGETPYTHON = ROOTDIR / "cpython" / "installs" / "python-3.8.2"
 DEFAULTCFLAGS = ""
 # fmt: off
 DEFAULTLDFLAGS = " ".join(

diff --git a/pyodide_build/pywasmcross.py b/pyodide_build/pywasmcross.py
@@ -40,7 +40,7 @@
 from pyodide_build import common
 
 
-ROOTDIR = common.ROOTDIR
+TOOLSDIR = common.TOOLSDIR
 symlinks = set(["cc", "c++", "ld", "ar", "gcc", "gfortran"])
 
 
@@ -56,8 +56,8 @@ def collect_args(basename):
     # native compiler
     env = dict(os.environ)
     path = env["PATH"]
-    while str(ROOTDIR) + ":" in path:
-        path = path.replace(str(ROOTDIR) + ":", "")
+    while str(TOOLSDIR) + ":" in path:
+        path = path.replace(str(TOOLSDIR) + ":", "")
     env["PATH"] = path
 
     skip_host = "SKIP_HOST" in os.environ
@@ -107,7 +107,7 @@ def make_symlinks(env):
     """
     exec_path = Path(__file__).resolve()
     for symlink in symlinks:
-        symlink_path = ROOTDIR / symlink
+        symlink_path = TOOLSDIR / symlink
         if os.path.lexists(symlink_path) and not symlink_path.exists():
             # remove broken symlink so it can be re-created
             symlink_path.unlink()
@@ -123,7 +123,7 @@ def make_symlinks(env):
 def capture_compile(args):
     env = dict(os.environ)
     make_symlinks(env)
-    env["PATH"] = str(ROOTDIR) + ":" + os.environ["PATH"]
+    env["PATH"] = str(TOOLSDIR) + ":" + os.environ["PATH"]
 
     cmd = [sys.executable, "setup.py", "install"]
     if args.install_dir == "skip":