Build C libraries via buildall.py (pyodide#927)

This addresses part of pyodide#713, by listing and building C libraries as dependencies. This means we automatically build CLAPACK when needed, and `lxml`'s C dependencies are not built if lxml is not. In particular, building "core" should now be faster. The building itself is still performed by Makefile.
jmsmdy · Dec 23, 2020 · 8de2ed1 · 8de2ed1
1 parent 12a8681
commit 8de2ed1
Show file tree

Hide file tree

Showing 14 changed files with 119 additions and 82 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -84,10 +84,6 @@ jobs:
           no_output_timeout: 1200
           command: |
             ccache -z
-            # Mark CLAPACK as not built as it's currently incorrectly marked as built
-            # when running make with PYODIDE_PACKAGES
-            # TODO: this should be resolved with #713
-            make -C packages/CLAPACK clean
             make
             ccache -s
 

diff --git a/Makefile b/Makefile
@@ -7,12 +7,6 @@ FILEPACKAGER=$(PYODIDE_ROOT)/tools/file_packager.py
 CPYTHONROOT=cpython
 CPYTHONLIB=$(CPYTHONROOT)/installs/python-$(PYVERSION)/lib/python$(PYMINOR)
 
-LIBXML=packages/libxml/libxml2-2.9.10/.libs/libxml2.a
-LIBXSLT=packages/libxslt/libxslt-1.1.33/libxslt/.libs/libxslt.a
-LIBICONV=packages/libiconv/libiconv-1.16/lib/.libs/libiconv.a
-ZLIB=packages/zlib/zlib-1.2.11/lib/libz.a
-CLAPACK=packages/CLAPACK/CLAPACK-WA/lapack_WA.bc
-
 PYODIDE_EMCC=$(PYODIDE_ROOT)/ccache/emcc
 PYODIDE_CXX=$(PYODIDE_ROOT)/ccache/em++
 
@@ -241,28 +235,6 @@ $(CPYTHONLIB): emsdk/emsdk/.complete $(PYODIDE_EMCC) $(PYODIDE_CXX)
 	date +"[%F %T] done building cpython..."
 
 
-$(LIBXML): $(CPYTHONLIB) $(ZLIB)
-	date +"[%F %T] Building libxml..."
-	make -C packages/libxml
-	date +"[%F %T] done building libxml..."
-
-
-$(LIBXSLT): $(CPYTHONLIB) $(LIBXML)
-	date +"[%F %T] Building libxslt..."
-	make -C packages/libxslt
-	date +"[%F %T] done building libxslt..."
-
-$(LIBICONV):
-	date +"[%F %T] Building libiconv..."
-	make -C packages/libiconv
-	date +"[%F %T] done building libiconv..."
-
-$(ZLIB):
-	date +"[%F %T] Building zlib..."
-	make -C packages/zlib
-	date +"[%F %T] done building zlib..."
-
-
 $(SIX_LIBS): $(CPYTHONLIB)
 	date +"[%F %T] Building six..."
 	make -C packages/six
@@ -281,21 +253,7 @@ $(PARSO_LIBS): $(CPYTHONLIB)
 	date +"[%F %T] done building parso."
 
 
-$(CLAPACK): $(CPYTHONLIB)
-ifdef PYODIDE_PACKAGES
-	echo "Skipping BLAS/LAPACK build due to PYODIDE_PACKAGES being defined."
-	echo "Build it manually with make -C packages/CLAPACK if needed."
-	mkdir -p packages/CLAPACK/CLAPACK-WA/
-	touch $(CLAPACK)
-else
-	date +"[%F %T] Building CLAPACK..."
-	make -C packages/CLAPACK
-	date +"[%F %T] done building CLAPACK."
-endif
-
-
-
-build/packages.json: $(CLAPACK) $(LIBXML) $(LIBXSLT) FORCE
+build/packages.json: FORCE
 	date +"[%F %T] Building packages..."
 	make -C packages
 	date +"[%F %T] done building packages..."

diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md
@@ -88,9 +88,6 @@ To build a minimal version of pyodide, set `PYODIDE_PACKAGES="micropip"`. The
 packages micropip and distutils are always automatically included (but an empty
 `PYODIDE_PACKAGES` is interpreted as unset).
 
-If scipy is included in `PYODIDE_PACKAGES`, BLAS/LAPACK must be manually built
-first with `make -C packages/CLAPACK`.
-
 ## Environment variables
 
 Following environment variables additionally impact the build,

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -38,7 +38,8 @@
   ```
   before loading it.
   [#855](https://github.com/iodide-project/pyodide/pull/855)
-
+- Build runtime C libraries (e.g. libxml) via package build system with correct
+  dependency resolution
 
 ## Version 0.15.0
 *May 19, 2020*

diff --git a/docs/new_packages.md b/docs/new_packages.md
@@ -149,6 +149,36 @@ A list of required packages.
 
 (Unlike conda, this only supports package names, not versions).
 
+## C library dependencies
+Some python packages depend on certain C libraries, e.g. `lxml` depends on
+`libxml`.
+
+To package a C library, create a directory in `packages/` for the C library.
+This directory should contain (at least) two files:
+
+- `Makefile` that specifies how the library should be be built. Note that the
+  build system will call `make`, not `emmake make`. The convention is that the
+  source for the library is downloaded by the Makefile, as opposed to being
+  included in the `pyodide` repository.
+
+- `meta.yaml` that specifies metadata about the package. For C libraries, only
+  three options are supported:
+
+  - `package/name`: The name of the library, which must equal the directory
+    name.
+  - `requirements/run`: The dependencies of the library, which can include both
+    C libraries and python packages.
+  - `build/library`: This must be set to `true` to indicate that this is a
+    library and not an ordinary package.
+
+After packaging a C library, it can be added as a dependency of a python
+package like a normal dependency. See `lxml` and `libxml` for an example (and
+also `scipy` and `CLAPACK`).
+
+*Remark:* Certain C libraries come as emscripten ports, and do not have to be
+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.

diff --git a/packages/CLAPACK/meta.yaml b/packages/CLAPACK/meta.yaml
@@ -0,0 +1,5 @@
+package:
+  name: CLAPACK
+
+build:
+  library: true
diff --git a/packages/libiconv/meta.yaml b/packages/libiconv/meta.yaml
@@ -0,0 +1,5 @@
+package:
+  name: libiconv
+
+build:
+  library: true
diff --git a/packages/libxml/meta.yaml b/packages/libxml/meta.yaml
@@ -0,0 +1,9 @@
+package:
+  name: libxml
+
+requirements:
+  run:
+    - zlib
+
+build:
+  library: true
diff --git a/packages/libxslt/meta.yaml b/packages/libxslt/meta.yaml
@@ -0,0 +1,9 @@
+package:
+  name: libxslt
+
+requirements:
+  run:
+    - libxml
+
+build:
+  library: true
diff --git a/packages/lxml/meta.yaml b/packages/lxml/meta.yaml
@@ -22,6 +22,10 @@ requirements:
     - beautifulsoup4
     - cssselect
     - html5lib
+    - libxml
+    - libxslt
+    - zlib
+    - libiconv
 test:
   imports:
   - lxml

diff --git a/packages/scipy/meta.yaml b/packages/scipy/meta.yaml
@@ -29,6 +29,7 @@ build:
 requirements:
   run:
     - numpy
+    - CLAPACK
 
 test:
   imports:

diff --git a/packages/zlib/meta.yaml b/packages/zlib/meta.yaml
@@ -0,0 +1,5 @@
+package:
+  name: zlib
+
+build:
+  library: true
diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py
@@ -30,6 +30,7 @@ def __init__(self, pkgdir: Path):
 
         self.meta: dict = common.parse_package(pkgpath)
         self.name: str = self.meta["package"]["name"]
+        self.library: bool = self.meta.get("build", {}).get("library", False)
 
         assert self.name == pkgdir.stem
 
@@ -39,42 +40,52 @@ def __init__(self, pkgdir: Path):
 
     def build(self, outputdir: Path, args) -> None:
         with open(self.pkgdir / "build.log", "w") as f:
-            p = subprocess.run(
-                [
-                    sys.executable,
-                    "-m",
-                    "pyodide_build",
-                    "buildpkg",
-                    str(self.pkgdir / "meta.yaml"),
-                    "--package_abi",
-                    str(args.package_abi),
-                    "--cflags",
-                    args.cflags,
-                    "--ldflags",
-                    args.ldflags,
-                    "--target",
-                    args.target,
-                    "--install-dir",
-                    args.install_dir,
-                ],
-                check=False,
-                stdout=f,
-                stderr=subprocess.STDOUT,
-            )
+            if self.library:
+                p = subprocess.run(
+                    ["make"],
+                    cwd=self.pkgdir,
+                    check=False,
+                    stdout=f,
+                    stderr=subprocess.STDOUT,
+                )
+            else:
+                p = subprocess.run(
+                    [
+                        sys.executable,
+                        "-m",
+                        "pyodide_build",
+                        "buildpkg",
+                        str(self.pkgdir / "meta.yaml"),
+                        "--package_abi",
+                        str(args.package_abi),
+                        "--cflags",
+                        args.cflags,
+                        "--ldflags",
+                        args.ldflags,
+                        "--target",
+                        args.target,
+                        "--install-dir",
+                        args.install_dir,
+                    ],
+                    check=False,
+                    stdout=f,
+                    stderr=subprocess.STDOUT,
+                )
 
         with open(self.pkgdir / "build.log", "r") as f:
             shutil.copyfileobj(f, sys.stdout)
 
         p.check_returncode()
 
-        shutil.copyfile(
-            self.pkgdir / "build" / (self.name + ".data"),
-            outputdir / (self.name + ".data"),
-        )
-        shutil.copyfile(
-            self.pkgdir / "build" / (self.name + ".js"),
-            outputdir / (self.name + ".js"),
-        )
+        if not self.library:
+            shutil.copyfile(
+                self.pkgdir / "build" / (self.name + ".data"),
+                outputdir / (self.name + ".data"),
+            )
+            shutil.copyfile(
+                self.pkgdir / "build" / (self.name + ".js"),
+                outputdir / (self.name + ".js"),
+            )
 
     # We use this in the priority queue, which pops off the smallest element.
     # So we want the smallest element to have the largest number of dependents
@@ -210,6 +221,9 @@ def build_packages(packages_dir: Path, outputdir: Path, args) -> None:
     }
 
     for name, pkg in pkg_map.items():
+        if pkg.library:
+            continue
+
         package_data["dependencies"][name] = pkg.dependencies
         for imp in pkg.meta.get("test", {}).get("imports", [name]):
             package_data["import_name_to_package_name"][imp] = name

diff --git a/pyodide_build/tests/test_buildall.py b/pyodide_build/tests/test_buildall.py
@@ -43,10 +43,13 @@ def build(self, outputdir: Path, args) -> None:
         "html5lib",
         "cssselect",
         "lxml",
+        "libxslt",
+        "libxml",
+        "zlib",
+        "libiconv",
     }
     assert build_list.index("distlib") < build_list.index("micropip")
     assert build_list.index("soupsieve") < build_list.index("beautifulsoup4")
-    assert build_list.index("webencodings") < build_list.index("beautifulsoup4")
 
 
 @pytest.mark.parametrize("n_jobs", [1, 4])
-Original file line number
+Diff line change
@@ Expand Up / @@ -29,6 +29,7 @@ build: @@
     requirements:
       run:
         - numpy
+        - CLAPACK
     test:
       imports:
@@ Expand Down @@