Add meta.yaml validation (pyodide#1105)

jmsmdy · Jan 11, 2021 · a68833f · a68833f
1 parent 9f2ce2b
commit a68833f
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 33 deletions.
diff --git a/docs/new_packages.md b/docs/new_packages.md
@@ -161,6 +161,12 @@ Shell commands to run after building the library. These are run inside of
 
 (This key is not in the Conda spec).
 
+#### `build/replace-libs`
+
+A list of strings of the form `<old_name>=<new_name>`, to rename libraries when linking. This in particular
+might be necessary when using emscripten ports.
+For instance, `png16=png` is currently used in matplotlib.
+
 ### `requirements`
 
 #### `requirements/run`
@@ -169,6 +175,12 @@ A list of required packages.
 
 (Unlike conda, this only supports package names, not versions).
 
+### `test`
+
+#### `test/imports`
+
+List of imports to test after the package is built.
+
 ## C library dependencies
 Some python packages depend on certain C libraries, e.g. `lxml` depends on
 `libxml`.

diff --git a/packages/libiconv/meta.yaml b/packages/libiconv/meta.yaml
@@ -1,6 +1,6 @@
 package:
   name: libiconv
-  version: 1.16
+  version: "1.16"
 
 source:
   url: https://ftp.gnu.org/pub/gnu/libiconv/libiconv-1.16.tar.gz

diff --git a/packages/test_common.py b/packages/test_common.py
@@ -1,7 +1,8 @@
 import pytest
 import os
 from pathlib import Path
-from pyodide_build.common import parse_package, _parse_package_subset
+from pyodide_build.common import _parse_package_subset
+from pyodide_build.io import parse_package_config
 
 PKG_DIR = Path(__file__).parent
 
@@ -20,7 +21,9 @@ def registered_packages_meta():
     for each registered package
     """
     packages = registered_packages
-    return {name: parse_package(PKG_DIR / name / "meta.yaml") for name in packages}
+    return {
+        name: parse_package_config(PKG_DIR / name / "meta.yaml") for name in packages
+    }
 
 
 UNSUPPORTED_PACKAGES = {"chrome": ["pandas", "scipy", "scikit-learn"], "firefox": []}
@@ -29,7 +32,7 @@ def registered_packages_meta():
 @pytest.mark.parametrize("name", registered_packages())
 def test_parse_package(name):
     # check that we can parse the meta.yaml
-    meta = parse_package(PKG_DIR / name / "meta.yaml")
+    meta = parse_package_config(PKG_DIR / name / "meta.yaml")
 
     skip_host = meta.get("build", {}).get("skip_host", True)
     if name == "numpy":
@@ -41,7 +44,7 @@ def test_parse_package(name):
 @pytest.mark.parametrize("name", registered_packages())
 def test_import(name, selenium_standalone):
     # check that we can parse the meta.yaml
-    meta = parse_package(PKG_DIR / name / "meta.yaml")
+    meta = parse_package_config(PKG_DIR / name / "meta.yaml")
 
     if name in UNSUPPORTED_PACKAGES[selenium_standalone.browser]:
         pytest.xfail(

diff --git a/pyodide_build/buildall.py b/pyodide_build/buildall.py
@@ -17,6 +17,7 @@
 from typing import Dict, Set, Optional, List
 
 from . import common
+from .io import parse_package_config
 
 
 @total_ordering
@@ -28,7 +29,7 @@ def __init__(self, pkgdir: Path):
         if not pkgpath.is_file():
             raise ValueError(f"Directory {pkgdir} does not contain meta.yaml")
 
-        self.meta: dict = common.parse_package(pkgpath)
+        self.meta: dict = parse_package_config(pkgpath)
         self.name: str = self.meta["package"]["name"]
         self.library: bool = self.meta.get("build", {}).get("library", False)
 

diff --git a/pyodide_build/buildpkg.py b/pyodide_build/buildpkg.py
@@ -18,6 +18,7 @@
 
 
 from . import common
+from .io import parse_package_config
 
 
 def check_checksum(path: Path, pkg: Dict[str, Any]):
@@ -256,11 +257,11 @@ def source_files():
 
 
 def build_package(path: Path, args):
-    pkg = common.parse_package(path)
+    pkg = parse_package_config(path)
     name = pkg["package"]["name"]
     t0 = datetime.now()
     print("[{}] Building package {}...".format(t0.strftime("%Y-%m-%d %H:%M:%S"), name))
-    packagedir = name + "-" + str(pkg["package"]["version"])
+    packagedir = name + "-" + pkg["package"]["version"]
     dirpath = path.parent
     orig_path = Path.cwd()
     os.chdir(dirpath)

diff --git a/pyodide_build/common.py b/pyodide_build/common.py
@@ -26,16 +26,6 @@
 # fmt: on
 
 
-def parse_package(package):
-    # Import yaml here because pywasmcross needs to run in the built native
-    # Python, which won't have PyYAML
-    import yaml
-
-    # TODO: Validate against a schema
-    with open(package) as fd:
-        return yaml.safe_load(fd)
-
-
 def _parse_package_subset(query: Optional[str]) -> Optional[Set[str]]:
     """Parse the list of packages specified with PYODIDE_PACKAGES env var.
 

diff --git a/pyodide_build/io.py b/pyodide_build/io.py
@@ -0,0 +1,137 @@
+from pathlib import Path
+from typing import Dict, Any, List, Optional
+
+
+# TODO: support more complex types for validation
+
+PACKAGE_CONFIG_SPEC: Dict[str, Dict[str, Any]] = {
+    "package": {
+        "name": str,
+        "version": str,
+    },
+    "source": {
+        "url": str,
+        "extract_dir": str,
+        "path": str,
+        "patches": list,  # List[str]
+        "md5": str,
+        "sha256": str,
+        "extras": list,  # List[Tuple[str, str]],
+    },
+    "build": {
+        "skip_host": bool,
+        "cflags": str,
+        "cxxflags": str,
+        "ldflags": str,
+        "library": bool,
+        "script": str,
+        "post": str,
+        "replace-libs": list,
+    },
+    "requirements": {
+        "run": list,  # List[str],
+    },
+    "test": {
+        "imports": list,  # List[str]
+    },
+}
+
+
+def check_package_config(
+    config: Dict[str, Any], raise_errors: bool = True, file_path: Optional[Path] = None
+) -> List[str]:
+    """Check the validity of a loaded meta.yaml file
+
+    Currently the following checks are applied:
+     -
+
+    TODO:
+     - check for mandatory fields
+
+    Parameter
+    ---------
+    config
+        loaded meta.yaml as a dict
+    raise_errors
+        if true raise errors, otherwise return the list of error messages.
+    file_path
+        optional meta.yaml file path. Only used for more explicit error output,
+        when raise_errors = True.
+    """
+    errors_msg = []
+
+    # Check top level sections
+    wrong_keys = set(config.keys()).difference(PACKAGE_CONFIG_SPEC.keys())
+    if wrong_keys:
+        errors_msg.append(
+            f"Found unknown sections {list(wrong_keys)}. Expected "
+            f"sections are {list(PACKAGE_CONFIG_SPEC)}."
+        )
+
+    # Check subsections
+    for section_key in config:
+        if section_key not in PACKAGE_CONFIG_SPEC:
+            # Don't check subsections is the main section is invalid
+            continue
+        actual_keys = set(config[section_key].keys())
+        expected_keys = set(PACKAGE_CONFIG_SPEC[section_key].keys())
+
+        wrong_keys = set(actual_keys).difference(expected_keys)
+        if wrong_keys:
+            errors_msg.append(
+                f"Found unknown keys "
+                f"{[section_key + '/' + key for key in wrong_keys]}. "
+                f"Expected keys are "
+                f"{[section_key + '/' + key for key in expected_keys]}."
+            )
+
+    # Check value types
+    for section_key, section in config.items():
+        for subsection_key, value in section.items():
+            try:
+                expected_type = PACKAGE_CONFIG_SPEC[section_key][subsection_key]
+            except KeyError:
+                # Unkown key, which was already reported previously, don't
+                # check types
+                continue
+            if not isinstance(value, expected_type):
+                errors_msg.append(
+                    f"Wrong type for '{section_key}/{subsection_key}': "
+                    f"expected {expected_type.__name__}, got {type(value).__name__}."
+                )
+
+    if raise_errors and errors_msg:
+        if file_path is None:
+            file_path = Path("meta.yaml")
+        raise ValueError(
+            f"{file_path} validation failed: \n  - " + "\n - ".join(errors_msg)
+        )
+
+    return errors_msg
+
+
+def parse_package_config(path: Path, check: bool = True) -> Dict[str, Any]:
+    """Load a meta.yaml file
+
+    Parameters
+    ----------
+    path
+       path to the meta.yaml file
+    check
+       check the consitency of the config file
+
+    Returns
+    -------
+    the loaded config as a Dict
+    """
+    # Import yaml here because pywasmcross needs to run in the built native
+    # Python, which won't have PyYAML
+    import yaml
+
+    with open(path, "rb") as fd:
+        config = yaml.safe_load(fd)
+
+    if check:
+        check_package_config(config, file_path=path)
+
+    return config
diff --git a/pyodide_build/mkpkg.py b/pyodide_build/mkpkg.py
@@ -9,6 +9,8 @@
 from pathlib import Path
 from typing import Dict, Tuple, Any, Optional
 
+from .io import parse_package_config
+
 PACKAGES_ROOT = Path(__file__).parent.parent / "packages"
 
 
@@ -80,12 +82,7 @@ def update_package(package: str):
     import yaml
 
     meta_path = PACKAGES_ROOT / package / "meta.yaml"
-    if not meta_path.exists():
-        print(f"Skipping: {meta_path} does not exist!")
-        sys.exit(1)
-
-    with open(meta_path, "r") as fd:
-        yaml_content = yaml.safe_load(fd)
+    yaml_content = parse_package_config(meta_path)
 
     if "url" not in yaml_content["source"]:
         print(f"Skipping: {package} is a local package!")

diff --git a/pyodide_build/tests/test_buildpkg.py b/pyodide_build/tests/test_buildpkg.py
@@ -5,7 +5,8 @@
 
 import pytest
 
-from pyodide_build import buildpkg, common
+from pyodide_build import buildpkg
+from pyodide_build.io import parse_package_config
 
 
 def test_download_and_extract(monkeypatch):
@@ -16,8 +17,8 @@ def test_download_and_extract(monkeypatch):
     test_pkgs = []
 
     # tarballname == version
-    test_pkgs.append(common.parse_package("./packages/scipy/meta.yaml"))
-    test_pkgs.append(common.parse_package("./packages/numpy/meta.yaml"))
+    test_pkgs.append(parse_package_config("./packages/scipy/meta.yaml"))
+    test_pkgs.append(parse_package_config("./packages/numpy/meta.yaml"))
 
     # tarballname != version
     test_pkgs.append(

diff --git a/pyodide_build/tests/test_mkpkg.py b/pyodide_build/tests/test_mkpkg.py
@@ -5,23 +5,22 @@
 from pkg_resources import parse_version
 
 import pyodide_build.mkpkg
+from pyodide_build.io import parse_package_config
 
 # Following tests make real network calls to the PyPi JSON API.
 # Since the response is fully cached, and small, it is very fast and is
 # unlikely to fail.
 
 
 def test_mkpkg(tmpdir, monkeypatch):
-    # TODO: parametrize to check version
     base_dir = Path(str(tmpdir))
     monkeypatch.setattr(pyodide_build.mkpkg, "PACKAGES_ROOT", base_dir)
     pyodide_build.mkpkg.make_package("idna")
     assert os.listdir(base_dir) == ["idna"]
     meta_path = base_dir / "idna" / "meta.yaml"
     assert meta_path.exists()
 
-    with open(meta_path, "rb") as fh:
-        db = yaml.safe_load(fh)
+    db = parse_package_config(meta_path)
 
     assert db["package"]["name"] == "idna"
     assert db["source"]["url"].endswith(".tar.gz")
@@ -37,7 +36,7 @@ def test_mkpkg_update(tmpdir, monkeypatch):
             "sha256": "b307872f855b18632ce0c21c5e45be78c0ea7ae4c15c828c20788b26921eb3f6",
             "url": "https://<some>/idna-2.0.tar.gz",
         },
-        "test": {"imports": ["idna"], "custom": 2},
+        "test": {"imports": ["idna"]},
     }
 
     os.mkdir(base_dir / "idna")
@@ -46,8 +45,7 @@ def test_mkpkg_update(tmpdir, monkeypatch):
         yaml.dump(db_init, fh)
     pyodide_build.mkpkg.update_package("idna")
 
-    with open(meta_path, "rb") as fh:
-        db = yaml.safe_load(fh)
+    db = parse_package_config(meta_path)
     assert list(db.keys()) == list(db_init.keys())
     assert parse_version(db["package"]["version"]) > parse_version(
         db_init["package"]["version"]