Repository: pyodide/pyodide
Pull request: ENH Change to minimal build by default
With this PR `make` only builds the packages needed for the core test suite (previously equivalent to `make minimal`), 
 - `make minimal` is removed as redundant
 - To build all packages `PYODIDE_PACKAGES='*'` can be used.
 - This supports meta packages `core` and `min-scipy-stack` (cf PR docs for a description)

The end goal in a follow up PR is to switch to only building packages in `min-scipy-stack`, instead of all, to make CI faster.


Closes https://github.com/pyodide/pyodide/issues/1295

Problem statement:
Build only micropip with default PYODIDE_PACKAGES
Currently when a user or contributor downloads pyodide and runs make they end up with the full build which can take a while and with packages they might not need.  This situation is going to get worse as more packages are added.

I propose to,
 -  by default set `PYODIDE_PACKAGES='micropip'` so only it is built, forcing users to explicitly specify packages they need in addition.
 - add an option `PYODIDE_PACKAGES='*'` to build everything.


WDYT?

Hints from issue discussion:
@rth thanks for this proposal, I totally agree with you. For most cases I'm using `make minimal` as build target.

Generally, a fellow and I are currently focusing on building a *nano*-build command for Pyodide to end up with the tiniest working Pyodide which is possible (without support for any native-code packages) but with the full JavaScript integration. We're currently at 4,4 MB for a version which is suitable for our needs. This also heavily relates to #646, and I will update there when we have a fine and solid solution.
> We're currently at 4,4 MB for a version which is suitable for our needs. 

Great, looking forward to it :)

Once in a while when I forget to `export PYODIDE_PACKAGES="micropip"` or `make minimal` or `make debug` I come back and it's building all the packages and I have to quit and `make minimal` to get it to output `packages.json`. Would be nice to have `make minimal` by default.

I think instead of just micropip it might be a good idea to by default include the other packages used in the core test suite: pyparsing, pytz, packaging, and kiwisolver.
I agree that the core test suite should pass with the default build. Another way to get there though is to reduce number of packages it needs, by removing the dependency on at least one or two of the current requirements. As far as I remember the only reason we need so many is to load packages that depend on one another. Maybe we could mock or do isolated unit tests for some of those.
>  Maybe we could mock or do isolated unit tests for some of those.

Just looked into it and it doesn't look simple.

So maybe we should go with that full list as the default.
Would it make sense to even split the repo, moving the packages outside of the core project, similar to what projects with plugins/extensions do ?

This could be done as well as separate npm packages, with their only lifecycle, and potentially maintainers.

```
@pyodide/numpy
@pyodide/pandas
@some-other-org/my-super-duper-package
```

The idea here would be "Build as python", distribute as "web packages" (npm).

The flexibility of installing new packages from pypi via micropip is awesome, but in some cases (deployed applications), we just want to have a set of packages baked into the deployable application, without the need or flexibility to "install", during runtime.

I'm not sure how much the above approach would make sense for python packages, though, since this would be duplicating the packages, anyway (I'm speaking mainly from web dev perspective, where the majority (all) of the dependencies are in npm). 



> Would it make sense to even split the repo, moving the packages outside of the core project, similar to what projects with plugins/extensions do ?

Yes, that's something that would make sense in the long term, but I think we would require significant work on our build system and it's better to discuss that in a separate issue.

Commit message: Skip import tests for packages that are not built independently from
PYODIDE_PACKAGES

Patch to review:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 40fff31d6af..d233ee3877d 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -64,8 +64,7 @@ jobs:
           no_output_timeout: 1200
           command: |
             ccache -z
-            # The following packages are currently used in the main pyodide test suite
-            PYODIDE_PACKAGES="micropip,pyparsing,pytz,packaging,Jinja2" make
+            PYODIDE_PACKAGES="core" make
             ccache -s
 
       - run:
@@ -108,7 +107,7 @@ jobs:
           no_output_timeout: 1800
           command: |
             ccache -z
-            make -C packages
+            PYODIDE_PACKAGES='*' make -C packages
             ccache -s
 
       - run:
diff --git a/Dockerfile-prebuilt b/Dockerfile-prebuilt
index 74593abf913..93bbe16c3b3 100644
--- a/Dockerfile-prebuilt
+++ b/Dockerfile-prebuilt
@@ -11,4 +11,4 @@ COPY . pyodide
 # the rm at the end deletes the build results such that the resulting image can still be used for building pyodide
 # from source (including partial and customized builds). Due to the previous run of make, builds
 # executed with this image will be much faster than the ones executed with pyodide-env
-RUN cd pyodide && make && rm -r ./build
+RUN cd pyodide && PYODIDE_PACKAGES='*' make && rm -r ./build
diff --git a/Makefile b/Makefile
index 99f86021006..353f07deb51 100644
--- a/Makefile
+++ b/Makefile
@@ -226,11 +226,6 @@ check:
 	./tools/dependency-check.sh
 
 
-minimal :
-	PYODIDE_PACKAGES+=",micropip" make
-
-
 debug :
 	EXTRA_CFLAGS+=" -D DEBUG_F" \
-	PYODIDE_PACKAGES+=", micropip, pyparsing, pytz, packaging, " \
 	make
diff --git a/docs/development/building-from-sources.md b/docs/development/building-from-sources.md
index 6d7242638cb..752766dd432 100644
--- a/docs/development/building-from-sources.md
+++ b/docs/development/building-from-sources.md
@@ -94,10 +94,19 @@ Dependencies of the listed packages will be built automatically as well. The
 package names must match the folder names in `packages/` exactly; in particular
 they are case sensitive.
 
-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). As a shorthand for this, one can
-say `make minimal`.
+If `PYODIDE_PACKAGES` is not set, a minimal set of packages necessairy to run
+the core test suite is installed, including "micropip", "pyparsing", "pytz",
+"packaging", "Jinja2". This is equivalent to setting `PYODIDE_PACKAGES='core'`
+meta-package. Other supported meta-packages are,
+
+- "min-scipy-stack": includes the "core" meta-package as well as some of the
+  core packages from the scientific python stack and their dependencies:
+  "numpy", "scipy", "pandas", "matplotlib", "scikit-learn", "joblib",
+  "pytest". This option is non exaustive and is mainly intended to make build
+  faster while testing a diverse set of scientific packages.
+- "\*" builds all packages
+
+micropip and distutils are always automatically included.
 
 ## Environment variables
 
diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index 8258c6c74a9..aaba659bc2f 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -20,6 +20,12 @@ substitutions:
   console.
   {pr}`1790`
 
+### pyodide-build
+
+- By default only a minimal set of packages is built. To build all packages set `PYODIDE_PACKAGES='*'`
+  In addition, `make minimal` was removed, since it is now equivalent to `make` without extra arguments.
+  {pr}`1801`
+
 ## Version 0.18.0
 
 _August 3rd, 2021_
diff --git a/packages/test_common.py b/packages/test_packages_common.py
similarity index 79%
rename from packages/test_common.py
rename to packages/test_packages_common.py
index 8b3c15519b4..affabd98b89 100644
--- a/packages/test_common.py
+++ b/packages/test_packages_common.py
@@ -1,13 +1,17 @@
 import pytest
 import os
 from pathlib import Path
-from pyodide_build.common import _parse_package_subset
+from typing import List
+import functools
+
 from pyodide_build.io import parse_package_config
 
 PKG_DIR = Path(__file__).parent
+BUILD_DIR = PKG_DIR.parent / "build"
 
 
-def registered_packages():
+@functools.cache
+def registered_packages() -> List[str]:
     """Returns a list of registered package names"""
     packages = []
     for name in os.listdir(PKG_DIR):
@@ -16,6 +20,22 @@ def registered_packages():
     return packages
 
 
+@functools.cache
+def built_packages() -> List[str]:
+    """Returns a list of built package names"""
+    if not BUILD_DIR.exists():
+        return []
+    registered_packages_ = registered_packages()
+    packages = []
+    for fpath in os.listdir(BUILD_DIR):
+        if not fpath.endswith(".data"):
+            continue
+        name = fpath.split(".")[0]
+        if name in registered_packages_:
+            packages.append(name)
+    return packages
+
+
 def registered_packages_meta():
     """Returns a dictionary with the contents of `meta.yaml`
     for each registered package
@@ -59,9 +79,7 @@ def test_import(name, selenium_standalone):
             )
         )
 
-    built_packages = _parse_package_subset(os.environ.get("PYODIDE_PACKAGES"))
-    # only a subset of packages were built
-    if built_packages is not None and name not in built_packages:
+    if name not in built_packages():
         pytest.skip(f"{name} was skipped due to PYODIDE_PACKAGES")
 
     selenium_standalone.run("import glob, os")
diff --git a/pyodide-build/pyodide_build/buildall.py b/pyodide-build/pyodide_build/buildall.py
index 1c1c18bc47b..5aae9a33b86 100755
--- a/pyodide-build/pyodide_build/buildall.py
+++ b/pyodide-build/pyodide_build/buildall.py
@@ -148,10 +148,10 @@ def build(self, outputdir: Path, args) -> None:
 
 
 def generate_dependency_graph(
-    packages_dir: Path, package_list: Optional[str]
+    packages_dir: Path, packages: Optional[Set[str]]
 ) -> Dict[str, BasePackage]:
-    """
-    This generates a dependency graph for the packages listed in package_list.
+    """This generates a dependency graph for listed packages.
+
     A node in the graph is a BasePackage object defined above, which maintains
     a list of dependencies and also dependents. That is, each node stores both
     incoming and outgoing edges.
@@ -163,7 +163,7 @@ def generate_dependency_graph(
 
     Parameters:
      - packages_dir: directory that contains packages
-     - package_list: set of packages to build. If None, then all packages in
+     - packages: set of packages to build. If None, then all packages in
        packages_dir are compiled.
 
     Returns:
@@ -172,7 +172,6 @@ def generate_dependency_graph(
 
     pkg_map: Dict[str, BasePackage] = {}
 
-    packages: Optional[Set[str]] = common._parse_package_subset(package_list)
     if packages is None:
         packages = set(
             str(x) for x in packages_dir.iterdir() if (x / "meta.yaml").is_file()
@@ -308,7 +307,9 @@ def generate_packages_json(pkg_map: Dict[str, BasePackage]) -> Dict:
 
 
 def build_packages(packages_dir: Path, outputdir: Path, args) -> None:
-    pkg_map = generate_dependency_graph(packages_dir, args.only)
+    packages = common._parse_package_subset(args.only)
+
+    pkg_map = generate_dependency_graph(packages_dir, packages)
 
     build_from_graph(pkg_map, outputdir, args)
 
diff --git a/pyodide-build/pyodide_build/common.py b/pyodide-build/pyodide_build/common.py
index b7253b19221..f60a6232545 100644
--- a/pyodide-build/pyodide_build/common.py
+++ b/pyodide-build/pyodide_build/common.py
@@ -12,13 +12,46 @@ def _parse_package_subset(query: Optional[str]) -> Optional[Set[str]]:
     Also add the list of mandatory packages: ["pyparsing", "packaging",
     "micropip"]
 
+    Supports folowing meta-packages,
+     - 'core': corresponds to packages needed to run the core test suite
+       {"micropip", "pyparsing", "pytz", "packaging", "Jinja2"}. This is the default option
+       if query is None.
+     - 'min-scipy-stack': includes the "core" meta-package as well as some of the
+       core packages from the scientific python stack and their dependencies:
+       {"numpy", "scipy", "pandas", "matplotlib", "scikit-learn", "joblib", "pytest"}.
+       This option is non exaustive and is mainly intended to make build faster
+       while testing a diverse set of scientific packages.
+     - '*': corresponds to all packages (returns None)
+
     Returns:
       a set of package names to build or None.
     """
     if query is None:
-        return None
+        query = "core"
+
+    core_packages = {"micropip", "pyparsing", "pytz", "packaging", "Jinja2"}
+    core_scipy_packages = {
+        "numpy",
+        "scipy",
+        "pandas",
+        "matplotlib",
+        "scikit-learn",
+        "joblib",
+        "pytest",
+    }
     packages = {el.strip() for el in query.split(",")}
     packages.update(["pyparsing", "packaging", "micropip"])
+    # handle meta-packages
+    if "*" in packages:
+        # build all packages
+        return None
+    elif "core" in packages:
+        packages |= core_packages
+        packages.discard("core")
+    elif "min-scipy-stack" in packages:
+        packages |= core_packages | core_scipy_packages
+        packages.discard("min-scipy-stack")
+
     # Hack to deal with the circular dependence between soupsieve and
     # beautifulsoup4
     if "beautifulsoup4" in packages:
diff --git a/pyodide-build/pyodide_build/tests/test_buildall.py b/pyodide-build/pyodide_build/tests/test_buildall.py
index 8034b955f58..f982a695a9b 100644
--- a/pyodide-build/pyodide_build/tests/test_buildall.py
+++ b/pyodide-build/pyodide_build/tests/test_buildall.py
@@ -10,7 +10,7 @@
 
 
 def test_generate_dependency_graph():
-    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, "beautifulsoup4")
+    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, {"beautifulsoup4"})
 
     assert set(pkg_map.keys()) == {
         "packaging",
@@ -27,7 +27,7 @@ def test_generate_dependency_graph():
 
 
 def test_generate_packages_json():
-    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, "beautifulsoup4")
+    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, {"beautifulsoup4"})
 
     package_data = buildall.generate_packages_json(pkg_map)
     assert set(package_data.keys()) == {"info", "packages"}
@@ -59,7 +59,7 @@ def build(self, outputdir: Path, args) -> None:
 
     monkeypatch.setattr(buildall, "Package", MockPackage)
 
-    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, "lxml")
+    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, {"lxml"})
 
     Args = namedtuple("args", ["n_jobs"])
     buildall.build_from_graph(pkg_map, Path("."), Args(n_jobs=n_jobs))
@@ -100,7 +100,7 @@ def build(self, outputdir: Path, args) -> None:
 
     monkeypatch.setattr(buildall, "Package", MockPackage)
 
-    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, package_list=None)
+    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, packages=None)
 
     Args = namedtuple("args", ["n_jobs"])
     buildall.build_from_graph(pkg_map, Path("."), Args(n_jobs=n_jobs))
@@ -116,7 +116,7 @@ def build(self, outputdir: Path, args) -> None:
 
     monkeypatch.setattr(buildall, "Package", MockPackage)
 
-    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, "lxml")
+    pkg_map = buildall.generate_dependency_graph(PACKAGES_DIR, {"lxml"})
 
     with pytest.raises(ValueError, match="Failed build"):
         Args = namedtuple("args", ["n_jobs"])
diff --git a/pyodide-build/pyodide_build/tests/test_common.py b/pyodide-build/pyodide_build/tests/test_common.py
index 663ecda8ecd..1a23c86481e 100644
--- a/pyodide-build/pyodide_build/tests/test_common.py
+++ b/pyodide-build/pyodide_build/tests/test_common.py
@@ -11,7 +11,6 @@
 
 
 def test_parse_package_subset():
-    assert _parse_package_subset(None) is None
     # micropip is always included
     assert _parse_package_subset("numpy,pandas") == {
         "pyparsing",
@@ -40,6 +39,37 @@ def test_parse_package_subset():
         "c",
         "d",
     }
+    # "*" means select all packages
+    assert _parse_package_subset("*") == None
+
+    assert _parse_package_subset("core") == {
+        "pyparsing",
+        "packaging",
+        "pytz",
+        "Jinja2",
+        "micropip",
+    }
+    # by default core packages are built
+    assert _parse_package_subset(None) == _parse_package_subset("core")
+
+    assert _parse_package_subset("min-scipy-stack") == {
+        "pyparsing",
+        "packaging",
+        "pytz",
+        "Jinja2",
+        "micropip",
+        "numpy",
+        "scipy",
+        "pandas",
+        "matplotlib",
+        "scikit-learn",
+        "joblib",
+        "pytest",
+    }
+    # reserved key words can be combined with other packages
+    assert _parse_package_subset("core, unknown") == _parse_package_subset("core") | {
+        "unknown"
+    }
 
 
 def test_get_make_flag():
diff --git a/src/js/package-lock.json b/src/js/package-lock.json
index 0e136cc6d2e..ea6897e31fb 100644
--- a/src/js/package-lock.json
+++ b/src/js/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "pyodide",
-  "version": "0.18.0-dev.0",
+  "version": "0.19.0-dev.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {


Code review comments:
diff_hunk: @@ -12,13 +12,46 @@ def _parse_package_subset(query: Optional[str]) -> Optional[Set[str]]:
     Also add the list of mandatory packages: ["pyparsing", "packaging",
     "micropip"]
 
+    Supports folowing meta-packages,
+     - 'core': corresponds to packages needed to run the core test suite
+       {"micropip", "pyparsing", "pytz", "packaging", "Jinja2"}. This is the default option
+       if query is None.
+     - 'min-scipy-stack': includes the "core" meta-package as well as some of the
+       core packages from the scientific python stack and their dependencies:
+       {"numpy", "scipy", "pandas", "matplotlib", "scikit-learn", "joblib", "pytest"}.
+       This option is non exaustive and is mainly intended to make build faster
+       while testing a diverse set of scientific packages.
+     - '*': corresponds to all packages (returns None)
+
     Returns:
       a set of package names to build or None.
path: pyodide-build/pyodide_build/common.py
text: @user1:
Would be good to say separately that `None` as an argument results from `PYODIDE_PACKAGES` being unset and leads to only `core` packages being built, and that `None` as a return value means all packages.

It might be better in the * case to return the set of all packages, which would eliminate this special case.

@author:
> It might be better in the * case to return the set of all packages, which would eliminate this special case.

This function is only responsible for parsing CLI string input, not  loading the list of available packages from disk. So we don't have that list at this point. In the case of  all packages (`*`) we need some special marker. 

FWIW before this PR the output of None also meant to build all packages, but I agree that the fact that,
 - before: "None" input -> "None": output
 - now: "None: input -> 'core' meta package, "*" input -> "None" output

might be a bit confusing. I'll improve the docs.