From 5ba82b81f98cbf3fabc896087d6676696bb3eac0 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 01:28:44 +0200 Subject: [PATCH 1/9] Add mechanism to build a subset of packages --- docs/building_from_sources.md | 19 +++++++++++++++++++ docs/index.rst | 1 + packages/Makefile | 2 +- pyodide_build/buildall.py | 30 ++++++++++++++++++++++++++++-- pyodide_build/common.py | 19 +++++++++++++++++++ test/pyodide_build/test_common.py | 14 ++++++++++++++ 6 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 docs/building_from_sources.md create mode 100644 test/pyodide_build/test_common.py diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md new file mode 100644 index 000000000..425c3aca9 --- /dev/null +++ b/docs/building_from_sources.md @@ -0,0 +1,19 @@ +# Building from sources + +To install Pyodide from sources follow the steps in the +[readme](./rootdir.html#building-from-source). + + +## Partial builds + +To build a subset of available packages in pyodide, set the environement +variable `PYODIDE_PACKAGES` to a comma separated list of packages. For +instance, + +``` +PYODIDE_PACKAGES="toolz,attrs" make +``` + +Note that this environement variables must contain both the packages and their +dependencies. The package names must much the folder names in `packages/` +exactly; in particular they are case sensitive. diff --git a/docs/index.rst b/docs/index.rst index 7eb677fab..de0615405 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -57,6 +57,7 @@ information about the project's organization. :maxdepth: 1 :caption: Development + building_from_sources.md new_packages.md type_conversions.md diff --git a/packages/Makefile b/packages/Makefile index d7fd22a43..1432ea82f 100644 --- a/packages/Makefile +++ b/packages/Makefile @@ -3,7 +3,7 @@ include ../Makefile.envs all: deps ../bin/pyodide buildall . ../build \ - --package_abi=$(PYODIDE_PACKAGE_ABI) --ldflags="$(SIDE_LDFLAGS)" --host=$(HOSTPYTHONROOT) --target=$(TARGETPYTHONROOT) + --package_abi=$(PYODIDE_PACKAGE_ABI) --ldflags="$(SIDE_LDFLAGS)" --host=$(HOSTPYTHONROOT) --target=$(TARGETPYTHONROOT) --only $(PYODIDE_PACKAGES) deps: # Install build dependencies $(HOSTPYTHON) -m pip install Cython Tempita diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index b64a1b161..d20d88fb2 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -9,7 +9,6 @@ import json from pathlib import Path import shutil - from . import common from . import buildpkg @@ -28,12 +27,33 @@ def build_package(pkgname, dependencies, packagesdir, outputdir, args): outputdir / (pkgname + '.js')) + def build_packages(packagesdir, outputdir, args): # We have to build the packages in the correct order (dependencies first), # so first load in all of the package metadata and build a dependency map. dependencies = {} import_name_to_package_name = {} + included_packages = common._parse_package_subset(args.only) + if included_packages is not None: + # check that the specified packages exist + for name in included_packages: + if not (packagesdir / name).exists(): + raise ValueError( + f'package name {name} does not exist. ' + f'The value of PYODIDE_PACKAGES is likely incorrect.' + ) + for pkgdir in packagesdir.iterdir(): + if ( + included_packages is not None + and pkgdir.name not in included_packages + ): + print( + f"Warning: skiping build of {pkgdir.name} due " + f"to specified PYODIDE_PACKAGES" + ) + continue + pkgpath = pkgdir / 'meta.yaml' if pkgdir.is_dir() and pkgpath.is_file(): pkg = common.parse_package(pkgpath) @@ -60,7 +80,10 @@ def build_packages(packagesdir, outputdir, args): def make_parser(parser): - parser.description = "Build all of the packages in a given directory" + parser.description = ( + "Build all of the packages in a given directory\n\n" + "Unless the --only option is provided" + ) parser.add_argument( 'dir', type=str, nargs=1, help='Input directory containing a tree of package definitions') @@ -82,6 +105,9 @@ def make_parser(parser): parser.add_argument( '--target', type=str, nargs='?', default=common.TARGETPYTHON, help='The path to the target Python installation') + parser.add_argument( + '--only', type=str, default="", + help='Only build the specified packages') return parser diff --git a/pyodide_build/common.py b/pyodide_build/common.py index f681b1206..eef9722f7 100644 --- a/pyodide_build/common.py +++ b/pyodide_build/common.py @@ -1,4 +1,5 @@ from pathlib import Path +from typing import Optional, List ROOTDIR = Path(__file__).parents[1].resolve() / 'tools' @@ -24,3 +25,21 @@ def parse_package(package): # TODO: Validate against a schema with open(package) as fd: return yaml.load(fd) + + +def _parse_package_subset(query: str) -> Optional[List[str]]: + """Parse the list of packages specified with PYODIDE_PACKAGES env var. + + Also add the list of mandatory packages: ['micropip'] + + Returns: + a list of package names to build, or None if all packages should be + built. + """ + if not query: + # build all packages + return None + packages = query.split(',') + packages = [el.strip() for el in packages] + return ['micropip'] + packages + diff --git a/test/pyodide_build/test_common.py b/test/pyodide_build/test_common.py new file mode 100644 index 000000000..5b0e36578 --- /dev/null +++ b/test/pyodide_build/test_common.py @@ -0,0 +1,14 @@ +import sys +from pathlib import Path + +sys.path.append(str(Path(__file__).parents[2])) + +from pyodide_build.common import _parse_package_subset # noqa: E402 + + +def test_parse_package_subset(): + assert _parse_package_subset("") is None + # micropip is always included + assert _parse_package_subset("numpy,pandas") == [ + 'micropip', 'numpy', 'pandas' + ] From ab6d0d9703cf6ed0bb3275e9f6694150586a6ecf Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 01:46:59 +0200 Subject: [PATCH 2/9] Add build-test-minimal CI job --- .circleci/config.yml | 38 +++++++++++++++++++++++++++++++++++ docs/building_from_sources.md | 8 ++++++-- pyodide_build/common.py | 4 ++-- test/test_common.py | 7 ++++++- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b54a6ec09..2f7ad8e79 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -50,6 +50,43 @@ jobs: - store_artifacts: path: /home/circleci/repo/build/ + build-test-minimal: + <<: *defaults + steps: + - checkout + + - run: + name: lint + command: | + make lint + + # this cache is generated by the main build job, we never store it here + - restore_cache: + keys: + - v1-emsdk-{{ checksum "emsdk/Makefile" }}-v14- + + - run: + name: build + no_output_timeout: 1200 + command: | + ccache -z + PYODIDE_PACKAGES='micropip' make + ccache -s + + - run: + name: check-size + command: ls -lh build/ + + - run: + name: test + command: | + export PYODIDE_PACKAGES='micropip' + # only check common tests, all other are checked in the main test jobs + pytest test/test_common.py -v -k firefox + + - store_artifacts: + path: /home/circleci/repo/build/ + test-firefox: <<: *defaults steps: @@ -140,6 +177,7 @@ workflows: filters: tags: only: /.*/ + - build-test-minimal - test-chrome: requires: - build diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md index 425c3aca9..45bdc71b6 100644 --- a/docs/building_from_sources.md +++ b/docs/building_from_sources.md @@ -6,7 +6,7 @@ To install Pyodide from sources follow the steps in the ## Partial builds -To build a subset of available packages in pyodide, set the environement +To build a subset of available packages in pyodide, set the environment variable `PYODIDE_PACKAGES` to a comma separated list of packages. For instance, @@ -14,6 +14,10 @@ instance, PYODIDE_PACKAGES="toolz,attrs" make ``` -Note that this environement variables must contain both the packages and their +Note that this environment variables must contain both the packages and their dependencies. The package names must much the folder names in `packages/` exactly; in particular they are case sensitive. + +To build a minimal version of pyodide, set `PYODIDE_PACKAGES="micropip"`. The +micropip package is generally always included for any non empty value of +`PYODIDE_PACKAGES`. diff --git a/pyodide_build/common.py b/pyodide_build/common.py index eef9722f7..3cbbb361d 100644 --- a/pyodide_build/common.py +++ b/pyodide_build/common.py @@ -30,7 +30,7 @@ def parse_package(package): def _parse_package_subset(query: str) -> Optional[List[str]]: """Parse the list of packages specified with PYODIDE_PACKAGES env var. - Also add the list of mandatory packages: ['micropip'] + Also add the list of mandatory packages: ['micropip', 'distlib'] Returns: a list of package names to build, or None if all packages should be @@ -41,5 +41,5 @@ def _parse_package_subset(query: str) -> Optional[List[str]]: return None packages = query.split(',') packages = [el.strip() for el in packages] - return ['micropip'] + packages + return ['micropip', 'distlib'] + packages diff --git a/test/test_common.py b/test/test_common.py index 980051da1..4b95a94b5 100644 --- a/test/test_common.py +++ b/test/test_common.py @@ -1,7 +1,7 @@ import pytest import os from pathlib import Path -from pyodide_build.common import parse_package +from pyodide_build.common import parse_package, _parse_package_subset BASE_DIR = Path(__file__).parent.parent PKG_DIR = BASE_DIR / 'packages' @@ -49,6 +49,11 @@ def test_import(name, selenium_standalone): '{} fails to load and is not supported on {}.' .format(name, selenium_standalone.browser)) + built_packages = os.environ.get('PYODIDE_PACKAGES', "") + # only a subset of packages were built + if built_packages is not None and name not in built_packages: + pytest.skip(f'{name} was skipped due to PYODIDE_PACKAGES') + selenium_standalone.run("import glob, os") baseline_pyc = selenium_standalone.run( From db5ceca12e5861ccf1397d5b9f355b6c1034f002 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 02:06:22 +0200 Subject: [PATCH 3/9] Lint --- pyodide_build/buildall.py | 3 +-- pyodide_build/common.py | 1 - test/pyodide_build/test_common.py | 4 ++-- test/test_common.py | 4 +++- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index d20d88fb2..57114f4b6 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -27,7 +27,6 @@ def build_package(pkgname, dependencies, packagesdir, outputdir, args): outputdir / (pkgname + '.js')) - def build_packages(packagesdir, outputdir, args): # We have to build the packages in the correct order (dependencies first), # so first load in all of the package metadata and build a dependency map. @@ -45,7 +44,7 @@ def build_packages(packagesdir, outputdir, args): for pkgdir in packagesdir.iterdir(): if ( - included_packages is not None + included_packages is not None and pkgdir.name not in included_packages ): print( diff --git a/pyodide_build/common.py b/pyodide_build/common.py index 3cbbb361d..11e154831 100644 --- a/pyodide_build/common.py +++ b/pyodide_build/common.py @@ -42,4 +42,3 @@ def _parse_package_subset(query: str) -> Optional[List[str]]: packages = query.split(',') packages = [el.strip() for el in packages] return ['micropip', 'distlib'] + packages - diff --git a/test/pyodide_build/test_common.py b/test/pyodide_build/test_common.py index 5b0e36578..37a5c1f18 100644 --- a/test/pyodide_build/test_common.py +++ b/test/pyodide_build/test_common.py @@ -3,12 +3,12 @@ from pathlib import Path sys.path.append(str(Path(__file__).parents[2])) -from pyodide_build.common import _parse_package_subset # noqa: E402 +from pyodide_build.common import _parse_package_subset # noqa def test_parse_package_subset(): assert _parse_package_subset("") is None # micropip is always included assert _parse_package_subset("numpy,pandas") == [ - 'micropip', 'numpy', 'pandas' + 'micropip', 'distlib', 'numpy', 'pandas' ] diff --git a/test/test_common.py b/test/test_common.py index 4b95a94b5..bc1141bf2 100644 --- a/test/test_common.py +++ b/test/test_common.py @@ -49,7 +49,9 @@ def test_import(name, selenium_standalone): '{} fails to load and is not supported on {}.' .format(name, selenium_standalone.browser)) - built_packages = os.environ.get('PYODIDE_PACKAGES', "") + 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: pytest.skip(f'{name} was skipped due to PYODIDE_PACKAGES') From 05cafc82dd339e7c9d0396f798dc079bea47e048 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 02:07:39 +0200 Subject: [PATCH 4/9] Fix typo --- docs/building_from_sources.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md index 45bdc71b6..3a71a2ea0 100644 --- a/docs/building_from_sources.md +++ b/docs/building_from_sources.md @@ -15,7 +15,7 @@ PYODIDE_PACKAGES="toolz,attrs" make ``` Note that this environment variables must contain both the packages and their -dependencies. The package names must much the folder names in `packages/` +dependencies. 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 From ac2435a6074e5ad3c876d1403a313bd21e003613 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 12:22:09 +0200 Subject: [PATCH 5/9] Fix argparse nargs --- docs/building_from_sources.md | 4 ++-- pyodide_build/buildall.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md index 3a71a2ea0..cd475b363 100644 --- a/docs/building_from_sources.md +++ b/docs/building_from_sources.md @@ -14,10 +14,10 @@ instance, PYODIDE_PACKAGES="toolz,attrs" make ``` -Note that this environment variables must contain both the packages and their +Note that this environment variable must contain both the packages and their dependencies. 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 -micropip package is generally always included for any non empty value of +micropip and package is generally always included for any non empty value of `PYODIDE_PACKAGES`. diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index 57114f4b6..751479bbf 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -105,7 +105,7 @@ def make_parser(parser): '--target', type=str, nargs='?', default=common.TARGETPYTHON, help='The path to the target Python installation') parser.add_argument( - '--only', type=str, default="", + '--only', type=str, nargs='?', default="", help='Only build the specified packages') return parser From 1fcf5a88f8af45dad9ab244cb253fa235a5880dd Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 12:32:10 +0200 Subject: [PATCH 6/9] Refactor --- pyodide_build/buildall.py | 2 +- pyodide_build/common.py | 9 ++++----- test/pyodide_build/test_common.py | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index 751479bbf..173b21782 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -105,7 +105,7 @@ def make_parser(parser): '--target', type=str, nargs='?', default=common.TARGETPYTHON, help='The path to the target Python installation') parser.add_argument( - '--only', type=str, nargs='?', default="", + '--only', type=str, nargs='?', default=None, help='Only build the specified packages') return parser diff --git a/pyodide_build/common.py b/pyodide_build/common.py index 11e154831..ec9a35834 100644 --- a/pyodide_build/common.py +++ b/pyodide_build/common.py @@ -27,17 +27,16 @@ def parse_package(package): return yaml.load(fd) -def _parse_package_subset(query: str) -> Optional[List[str]]: +def _parse_package_subset(query: Optional[str]) -> Optional[List[str]]: """Parse the list of packages specified with PYODIDE_PACKAGES env var. Also add the list of mandatory packages: ['micropip', 'distlib'] Returns: - a list of package names to build, or None if all packages should be - built. + a list of package names to build or None. The list may contain duplicate + values. """ - if not query: - # build all packages + if query is None: return None packages = query.split(',') packages = [el.strip() for el in packages] diff --git a/test/pyodide_build/test_common.py b/test/pyodide_build/test_common.py index 37a5c1f18..05b2fe0bd 100644 --- a/test/pyodide_build/test_common.py +++ b/test/pyodide_build/test_common.py @@ -7,7 +7,7 @@ from pyodide_build.common import _parse_package_subset # noqa def test_parse_package_subset(): - assert _parse_package_subset("") is None + assert _parse_package_subset(None) is None # micropip is always included assert _parse_package_subset("numpy,pandas") == [ 'micropip', 'distlib', 'numpy', 'pandas' From f3f596f0c16842710ff1ef10b5d503c9d297b65d Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 12:39:05 +0200 Subject: [PATCH 7/9] Do no build CLAPACK if PYODIDE_PACKAGES is defined --- Makefile | 9 ++++++++- docs/building_from_sources.md | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d6929b5a3..533e2f16f 100644 --- a/Makefile +++ b/Makefile @@ -242,7 +242,14 @@ $(PARSO_LIBS): $(CPYTHONLIB) $(CLAPACK): $(CPYTHONLIB) - make -C CLAPACK + ifdef PYODIDE_PACKAGES + echo "Skipping BLAS/LAPACK build due to PYODIDE_PACKAGES being defined." + echo "Build it manually with make -C CLAPACK if needed." + touch $(CLAPACK) + else + make -C CLAPACK + endif + build/packages.json: $(CLAPACK) FORCE diff --git a/docs/building_from_sources.md b/docs/building_from_sources.md index cd475b363..078fad156 100644 --- a/docs/building_from_sources.md +++ b/docs/building_from_sources.md @@ -21,3 +21,6 @@ exactly; in particular they are case sensitive. To build a minimal version of pyodide, set `PYODIDE_PACKAGES="micropip"`. The micropip and package is generally always included for any non empty value of `PYODIDE_PACKAGES`. + +If scipy is included in `PYODIDE_PACKAGES`, BLAS/LAPACK must be manually built +first with `make -c CLAPACK`. From d02a96ece4c4c57802b120762bbd1efb9debf827 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Sat, 9 May 2020 12:48:35 +0200 Subject: [PATCH 8/9] Fix Makefile syntax --- Makefile | 15 ++++++++------- test/test_common.py | 4 +--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 533e2f16f..92a33e34e 100644 --- a/Makefile +++ b/Makefile @@ -242,13 +242,14 @@ $(PARSO_LIBS): $(CPYTHONLIB) $(CLAPACK): $(CPYTHONLIB) - ifdef PYODIDE_PACKAGES - echo "Skipping BLAS/LAPACK build due to PYODIDE_PACKAGES being defined." - echo "Build it manually with make -C CLAPACK if needed." - touch $(CLAPACK) - else - make -C CLAPACK - endif +ifdef PYODIDE_PACKAGES + echo "Skipping BLAS/LAPACK build due to PYODIDE_PACKAGES being defined." + echo "Build it manually with make -C CLAPACK if needed." + mkdir -p CLAPACK/CLAPACK-WA/ + touch $(CLAPACK) +else + make -C CLAPACK +endif diff --git a/test/test_common.py b/test/test_common.py index bc1141bf2..a8e6aab9b 100644 --- a/test/test_common.py +++ b/test/test_common.py @@ -49,9 +49,7 @@ def test_import(name, selenium_standalone): '{} fails to load and is not supported on {}.' .format(name, selenium_standalone.browser)) - built_packages = _parse_package_subset( - os.environ.get('PYODIDE_PACKAGES', "") - ) + 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: pytest.skip(f'{name} was skipped due to PYODIDE_PACKAGES') From 4dc422e2ab330e31e1c6550ff3db2589db8db9b5 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Tue, 12 May 2020 12:33:20 +0200 Subject: [PATCH 9/9] Avoid duplicates in the list of built packages --- pyodide_build/buildall.py | 3 ++- pyodide_build/common.py | 10 +++++----- test/pyodide_build/test_common.py | 9 +++++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py index 173b21782..162374693 100755 --- a/pyodide_build/buildall.py +++ b/pyodide_build/buildall.py @@ -106,7 +106,8 @@ def make_parser(parser): help='The path to the target Python installation') parser.add_argument( '--only', type=str, nargs='?', default=None, - help='Only build the specified packages') + help=('Only build the specified packages, provided as a comma ' + 'separated list')) return parser diff --git a/pyodide_build/common.py b/pyodide_build/common.py index ec9a35834..a82c10d30 100644 --- a/pyodide_build/common.py +++ b/pyodide_build/common.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Optional, List +from typing import Optional, Set ROOTDIR = Path(__file__).parents[1].resolve() / 'tools' @@ -27,17 +27,17 @@ def parse_package(package): return yaml.load(fd) -def _parse_package_subset(query: Optional[str]) -> Optional[List[str]]: +def _parse_package_subset(query: Optional[str]) -> Optional[Set[str]]: """Parse the list of packages specified with PYODIDE_PACKAGES env var. Also add the list of mandatory packages: ['micropip', 'distlib'] Returns: - a list of package names to build or None. The list may contain duplicate - values. + a set of package names to build or None. """ if query is None: return None packages = query.split(',') packages = [el.strip() for el in packages] - return ['micropip', 'distlib'] + packages + packages = ['micropip', 'distlib'] + packages + return set(packages) diff --git a/test/pyodide_build/test_common.py b/test/pyodide_build/test_common.py index 05b2fe0bd..5fd05c1fb 100644 --- a/test/pyodide_build/test_common.py +++ b/test/pyodide_build/test_common.py @@ -9,6 +9,11 @@ from pyodide_build.common import _parse_package_subset # noqa def test_parse_package_subset(): assert _parse_package_subset(None) is None # micropip is always included - assert _parse_package_subset("numpy,pandas") == [ + assert _parse_package_subset("numpy,pandas") == { 'micropip', 'distlib', 'numpy', 'pandas' - ] + } + + # duplicates are removed + assert _parse_package_subset("numpy,numpy") == { + 'micropip', 'distlib', 'numpy' + }