diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7f8b923..b271d1d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -73,10 +73,5 @@ repos: files: ^(conftest.py|src/tests|pyodide-build/pyodide_build/tests) additional_dependencies: *mypy-deps - - repo: https://github.com/pre-commit/mirrors-prettier - rev: "v4.0.0-alpha.8" - hooks: - - id: prettier - ci: autoupdate_schedule: "quarterly" diff --git a/pyodide_build/buildpkg.py b/pyodide_build/buildpkg.py index 0567f51..89d7149 100755 --- a/pyodide_build/buildpkg.py +++ b/pyodide_build/buildpkg.py @@ -13,6 +13,7 @@ from collections.abc import Iterator from datetime import datetime from email.message import Message +from functools import cache from pathlib import Path from typing import Any, cast @@ -102,19 +103,20 @@ def __init__( If True, continue a build from the middle. For debugging. Implies "force_rebuild". """ recipe = Path(recipe).resolve() - self.pkg_root, self.recipe = self._load_recipe(recipe) + self.pkg_root, self.recipe = _load_recipe(recipe) self.name = self.recipe.package.name self.version = self.recipe.package.version self.fullname = f"{self.name}-{self.version}" - self.build_args = build_args + + self.source_metadata = self.recipe.source + self.build_metadata = self.recipe.build + self.package_type = self.build_metadata.package_type self.build_dir = ( Path(build_dir).resolve() if build_dir else self.pkg_root / "build" ) - self.library_install_prefix = self.build_dir.parent.parent / ".libs" - self.src_extract_dir = ( self.build_dir / self.fullname ) # where we extract the source @@ -127,14 +129,35 @@ def __init__( # where Pyodide will look for the built artifacts when building pyodide-lock.json. # after building packages, artifacts in src_dist_dir will be copied to dist_dir self.dist_dir = self.pkg_root / "dist" - - self.source_metadata = self.recipe.source - self.build_metadata = self.recipe.build - self.package_type = self.build_metadata.package_type - + self.build_args = build_args self.force_rebuild = force_rebuild or continue_ self.continue_ = continue_ + @classmethod + def get_builder( + cls, + recipe: str | Path, + build_args: BuildArgs, + build_dir: str | Path | None = None, + force_rebuild: bool = False, + continue_: bool = False, + ) -> "RecipeBuilder": + recipe = Path(recipe).resolve() + _, config = _load_recipe(recipe) + match config.build.package_type: + case "package": + builder = RecipeBuilderPackage + case "static_library": + builder = RecipeBuilderStaticLibrary + case "shared_library": + builder = RecipeBuilderSharedLibrary + case _: + raise ValueError( + f"Unknown package type: {config.build_metadata.package_type}" + ) + + return builder(recipe, build_args, build_dir, force_rebuild, continue_) + def build(self) -> None: """ Build the package. This is the only public method of this class. @@ -166,6 +189,9 @@ def build(self) -> None: else: logger.error(msg) + def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None: + raise NotImplementedError("Subclasses must implement this method") + def _build(self) -> None: if not self.force_rebuild and not needs_rebuild( self.pkg_root, self.build_dir, self.source_metadata @@ -188,70 +214,7 @@ def _build(self) -> None: chdir(self.pkg_root), get_bash_runner(self._get_helper_vars() | os.environ.copy()) as bash_runner, ): - if self.recipe.is_rust_package(): - bash_runner.run( - RUST_BUILD_PRELUDE, - script_name="rust build prelude", - cwd=self.src_extract_dir, - ) - - bash_runner.run( - self.build_metadata.script, - script_name="build script", - cwd=self.src_extract_dir, - ) - - # TODO: maybe subclass this for different package types? - if self.package_type == "static_library": - # Nothing needs to be done for a static library - pass - - elif self.package_type == "shared_library": - # If shared library, we copy .so files to dist_dir - # and create a zip archive of the .so files - shutil.rmtree(self.dist_dir, ignore_errors=True) - self.dist_dir.mkdir(parents=True) - make_zip_archive( - self.dist_dir / f"{self.fullname}.zip", self.src_dist_dir - ) - - else: # wheel - url = self.source_metadata.url - prebuilt_wheel = url and url.endswith(".whl") - if not prebuilt_wheel: - self._compile(bash_runner) - - self._package_wheel(bash_runner) - shutil.rmtree(self.dist_dir, ignore_errors=True) - shutil.copytree(self.src_dist_dir, self.dist_dir) - - def _load_recipe(self, package_dir: Path) -> tuple[Path, MetaConfig]: - """ - Load the package configuration from the given directory. - - Parameters - ---------- - package_dir - The directory containing the package configuration, or the path to the - package configuration file. - - Returns - ------- - pkg_dir - The directory containing the package configuration. - pkg - The package configuration. - """ - if not package_dir.exists(): - raise FileNotFoundError(f"Package directory {package_dir} does not exist") - - if package_dir.is_dir(): - meta_file = package_dir / "meta.yaml" - else: - meta_file = package_dir - package_dir = meta_file.parent - - return package_dir, MetaConfig.from_yaml(meta_file) + self._build_package(bash_runner) def _check_executables(self) -> None: """ @@ -424,6 +387,109 @@ def _compile( self.src_extract_dir, self.src_dist_dir, build_env, config_settings ) + def _patch(self) -> None: + """ + Apply patches to the source. + """ + token_path = self.src_extract_dir / ".patched" + if token_path.is_file(): + return + + patches = self.source_metadata.patches + extras = self.source_metadata.extras + cast(str, self.source_metadata.url) + + if not patches and not extras: + return + + # Apply all the patches + for patch in patches: + patch_abspath = self.pkg_root / patch + result = subprocess.run( + ["patch", "-p1", "--binary", "--verbose", "-i", patch_abspath], + check=False, + encoding="utf-8", + cwd=self.src_extract_dir, + ) + if result.returncode != 0: + logger.error("ERROR: Patch %s failed", patch_abspath) + exit_with_stdio(result) + + # Add any extra files + for src, dst in extras: + shutil.copyfile(self.pkg_root / src, self.src_extract_dir / dst) + + token_path.touch() + + def _redirect_stdout_stderr_to_logfile(self) -> None: + """ + Redirect stdout and stderr to a log file. + """ + try: + stdout_fileno = sys.stdout.fileno() + stderr_fileno = sys.stderr.fileno() + + tee = subprocess.Popen( + ["tee", self.pkg_root / "build.log"], stdin=subprocess.PIPE + ) + + # Cause tee's stdin to get a copy of our stdin/stdout (as well as that + # of any child processes we spawn) + os.dup2(tee.stdin.fileno(), stdout_fileno) # type: ignore[union-attr] + os.dup2(tee.stdin.fileno(), stderr_fileno) # type: ignore[union-attr] + except OSError: + # This normally happens when testing + logger.warning("stdout/stderr does not have a fileno, not logging to file") + + def _get_helper_vars(self) -> dict[str, str]: + """ + Get the helper variables for the build script. + """ + return { + "PKGDIR": str(self.pkg_root), + "PKG_VERSION": self.version, + "PKG_BUILD_DIR": str(self.src_extract_dir), + "DISTDIR": str(self.src_dist_dir), + # TODO: rename this to something more compatible with Makefile or CMake conventions + "WASM_LIBRARY_DIR": str(self.library_install_prefix), + # Emscripten will use this variable to configure pkg-config in emconfigure + "EM_PKG_CONFIG_PATH": str(self.library_install_prefix / "lib/pkgconfig"), + # This variable is usually overwritten by emconfigure + # The value below will only be used if pkg-config is called without emconfigure + # We use PKG_CONFIG_LIBDIR instead of PKG_CONFIG_PATH, + # so pkg-config will not look in the default system directories + "PKG_CONFIG_LIBDIR": str(self.library_install_prefix / "lib/pkgconfig"), + } + + +class RecipeBuilderPackage(RecipeBuilder): + """ + Recipe builder for python packages. + """ + + def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None: + if self.recipe.is_rust_package(): + bash_runner.run( + RUST_BUILD_PRELUDE, + script_name="rust build prelude", + cwd=self.src_extract_dir, + ) + + bash_runner.run( + self.build_metadata.script, + script_name="build script", + cwd=self.src_extract_dir, + ) + + url = self.source_metadata.url + prebuilt_wheel = url and url.endswith(".whl") + if not prebuilt_wheel: + self._compile(bash_runner) + + self._package_wheel(bash_runner) + shutil.rmtree(self.dist_dir, ignore_errors=True) + shutil.copytree(self.src_dist_dir, self.dist_dir) + def _package_wheel( self, bash_runner: BashRunnerWithSharedEnvironment, @@ -496,79 +562,67 @@ def _package_wheel( finally: shutil.rmtree(test_dir, ignore_errors=True) - def _patch(self) -> None: - """ - Apply patches to the source. - """ - token_path = self.src_extract_dir / ".patched" - if token_path.is_file(): - return - patches = self.source_metadata.patches - extras = self.source_metadata.extras - cast(str, self.source_metadata.url) +class RecipeBuilderStaticLibrary(RecipeBuilder): + """ + Recipe builder for static libraries. + """ - if not patches and not extras: - return + def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None: + bash_runner.run( + self.build_metadata.script, + script_name="build script", + cwd=self.src_extract_dir, + ) - # Apply all the patches - for patch in patches: - patch_abspath = self.pkg_root / patch - result = subprocess.run( - ["patch", "-p1", "--binary", "--verbose", "-i", patch_abspath], - check=False, - encoding="utf-8", - cwd=self.src_extract_dir, - ) - if result.returncode != 0: - logger.error("ERROR: Patch %s failed", patch_abspath) - exit_with_stdio(result) - # Add any extra files - for src, dst in extras: - shutil.copyfile(self.pkg_root / src, self.src_extract_dir / dst) +class RecipeBuilderSharedLibrary(RecipeBuilder): + """ + Recipe builder for shared libraries. + """ - token_path.touch() + def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None: + bash_runner.run( + self.build_metadata.script, + script_name="build script", + cwd=self.src_extract_dir, + ) - def _redirect_stdout_stderr_to_logfile(self) -> None: - """ - Redirect stdout and stderr to a log file. - """ - try: - stdout_fileno = sys.stdout.fileno() - stderr_fileno = sys.stderr.fileno() + # copy .so files to dist_dir + # and create a zip archive of the .so files + shutil.rmtree(self.dist_dir, ignore_errors=True) + self.dist_dir.mkdir(parents=True) + make_zip_archive(self.dist_dir / f"{self.fullname}.zip", self.src_dist_dir) - tee = subprocess.Popen( - ["tee", self.pkg_root / "build.log"], stdin=subprocess.PIPE - ) - # Cause tee's stdin to get a copy of our stdin/stdout (as well as that - # of any child processes we spawn) - os.dup2(tee.stdin.fileno(), stdout_fileno) # type: ignore[union-attr] - os.dup2(tee.stdin.fileno(), stderr_fileno) # type: ignore[union-attr] - except OSError: - # This normally happens when testing - logger.warning("stdout/stderr does not have a fileno, not logging to file") +@cache +def _load_recipe(package_dir: Path) -> tuple[Path, MetaConfig]: + """ + Load the package configuration from the given directory. - def _get_helper_vars(self) -> dict[str, str]: - """ - Get the helper variables for the build script. - """ - return { - "PKGDIR": str(self.pkg_root), - "PKG_VERSION": self.version, - "PKG_BUILD_DIR": str(self.src_extract_dir), - "DISTDIR": str(self.src_dist_dir), - # TODO: rename this to something more compatible with Makefile or CMake conventions - "WASM_LIBRARY_DIR": str(self.library_install_prefix), - # Emscripten will use this variable to configure pkg-config in emconfigure - "EM_PKG_CONFIG_PATH": str(self.library_install_prefix / "lib/pkgconfig"), - # This variable is usually overwritten by emconfigure - # The value below will only be used if pkg-config is called without emconfigure - # We use PKG_CONFIG_LIBDIR instead of PKG_CONFIG_PATH, - # so pkg-config will not look in the default system directories - "PKG_CONFIG_LIBDIR": str(self.library_install_prefix / "lib/pkgconfig"), - } + Parameters + ---------- + package_dir + The directory containing the package configuration, or the path to the + package configuration file. + + Returns + ------- + pkg_dir + The directory containing the package configuration. + pkg + The package configuration. + """ + if not package_dir.exists(): + raise FileNotFoundError(f"Package directory {package_dir} does not exist") + + if package_dir.is_dir(): + meta_file = package_dir / "meta.yaml" + else: + meta_file = package_dir + package_dir = meta_file.parent + + return package_dir, MetaConfig.from_yaml(meta_file) def check_checksum(archive: Path, checksum: str) -> None: diff --git a/pyodide_build/cli/build_recipes.py b/pyodide_build/cli/build_recipes.py index 7ae0ec9..b92a622 100644 --- a/pyodide_build/cli/build_recipes.py +++ b/pyodide_build/cli/build_recipes.py @@ -4,8 +4,9 @@ import typer -from pyodide_build import build_env, buildall, buildpkg +from pyodide_build import build_env, buildall from pyodide_build.build_env import BuildArgs, init_environment +from pyodide_build.buildpkg import RecipeBuilder from pyodide_build.common import get_num_cores from pyodide_build.logger import logger @@ -124,7 +125,7 @@ def build_recipes_no_deps_impl( for package in packages: package_path = args.recipe_dir / package package_build_dir = args.build_dir / package / "build" - builder = buildpkg.RecipeBuilder( + builder = RecipeBuilder.get_builder( package_path, args.build_args, package_build_dir, diff --git a/pyodide_build/tests/test_buildpkg.py b/pyodide_build/tests/test_buildpkg.py index b86c4a0..5812994 100644 --- a/pyodide_build/tests/test_buildpkg.py +++ b/pyodide_build/tests/test_buildpkg.py @@ -9,7 +9,13 @@ from pyodide_build import buildpkg, common from pyodide_build.build_env import BuildArgs -from pyodide_build.buildpkg import RecipeBuilder +from pyodide_build.buildpkg import ( + RecipeBuilder, + RecipeBuilderPackage, + RecipeBuilderSharedLibrary, + RecipeBuilderStaticLibrary, + _load_recipe, +) from pyodide_build.io import _SourceSpec RECIPE_DIR = Path(__file__).parent / "_test_recipes" @@ -18,8 +24,7 @@ @pytest.fixture def tmp_builder(tmp_path): - # Dummy builder to test other functions - builder = RecipeBuilder( + builder = RecipeBuilder.get_builder( recipe=RECIPE_DIR / "pkg_1", build_args=BuildArgs(), build_dir=tmp_path, @@ -31,7 +36,7 @@ def tmp_builder(tmp_path): def test_constructor(tmp_path): - builder = RecipeBuilder( + builder = RecipeBuilder.get_builder( recipe=RECIPE_DIR / "beautifulsoup4", build_args=BuildArgs(), build_dir=tmp_path / "beautifulsoup4" / "build", @@ -57,12 +62,47 @@ def test_constructor(tmp_path): assert builder.library_install_prefix == tmp_path / ".libs" -def test_load_recipe(tmp_builder): - root, recipe = tmp_builder._load_recipe(RECIPE_DIR / "pkg_1") +def test_get_builder(tmp_path): + builder = RecipeBuilder.get_builder( + recipe=RECIPE_DIR / "pkg_1", + build_args=BuildArgs(), + build_dir=tmp_path, + force_rebuild=False, + continue_=False, + ) + + assert isinstance(builder, RecipeBuilder) + assert isinstance(builder, RecipeBuilderPackage) + + builder = RecipeBuilder.get_builder( + recipe=RECIPE_DIR / "libtest", + build_args=BuildArgs(), + build_dir=tmp_path, + force_rebuild=False, + continue_=False, + ) + + assert isinstance(builder, RecipeBuilder) + assert isinstance(builder, RecipeBuilderStaticLibrary) + + builder = RecipeBuilder.get_builder( + recipe=RECIPE_DIR / "libtest_shared", + build_args=BuildArgs(), + build_dir=tmp_path, + force_rebuild=False, + continue_=False, + ) + + assert isinstance(builder, RecipeBuilder) + assert isinstance(builder, RecipeBuilderSharedLibrary) + + +def test_load_recipe(): + root, recipe = _load_recipe(RECIPE_DIR / "pkg_1") assert root == RECIPE_DIR / "pkg_1" assert recipe.package.name == "pkg_1" - root, recipe = tmp_builder._load_recipe(RECIPE_DIR / "pkg_1" / "meta.yaml") + root, recipe = _load_recipe(RECIPE_DIR / "pkg_1" / "meta.yaml") assert root == RECIPE_DIR / "pkg_1" assert recipe.package.name == "pkg_1" @@ -83,7 +123,7 @@ class subprocess_result: ] for pkg in test_pkgs: - builder = RecipeBuilder( + builder = RecipeBuilder.get_builder( recipe=pkg, build_args=BuildArgs(), build_dir=tmp_path, @@ -93,7 +133,7 @@ class subprocess_result: def test_check_executables(tmp_path, monkeypatch): - builder = RecipeBuilder( + builder = RecipeBuilder.get_builder( recipe=RECIPE_DIR / "pkg_test_executable", build_args=BuildArgs(), build_dir=tmp_path, @@ -109,7 +149,7 @@ def test_check_executables(tmp_path, monkeypatch): def test_get_helper_vars(tmp_path): - builder = RecipeBuilder( + builder = RecipeBuilder.get_builder( recipe=RECIPE_DIR / "pkg_1", build_args=BuildArgs(), build_dir=tmp_path / "pkg_1" / "build",