From 9012711ba003e2ca78b39792d64f863ce56ed10f Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 17 Jun 2023 13:39:33 -0700 Subject: [PATCH] Run replace_so_abi_tags on out of tree builds (#3927) --- docs/project/changelog.md | 6 + packages/distlib/meta.yaml | 2 +- packages/matplotlib/meta.yaml | 2 +- packages/setuptools/meta.yaml | 2 +- pyodide-build/pyodide_build/buildpkg.py | 109 ++++++------------ pyodide-build/pyodide_build/common.py | 61 ++++++++++ pyodide-build/pyodide_build/io.py | 2 + .../pyodide_build/out_of_tree/build.py | 7 +- .../mypkg/__init__.py | 0 .../blah.cpython-311-x86_64-linux-gnu.so | 0 .../pyproject.toml | 7 ++ .../pyodide_build/tests/test_buildpkg.py | 11 +- pyodide-build/pyodide_build/tests/test_cli.py | 37 ++++++ 13 files changed, 165 insertions(+), 81 deletions(-) create mode 100644 pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/__init__.py create mode 100644 pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/blah.cpython-311-x86_64-linux-gnu.so create mode 100644 pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/pyproject.toml diff --git a/docs/project/changelog.md b/docs/project/changelog.md index 8bcaa9810..0a2518239 100644 --- a/docs/project/changelog.md +++ b/docs/project/changelog.md @@ -58,6 +58,12 @@ myst: {pr}`3617`. {pr}`3926` +- {{ Fix }} `pyodide build` now replaces native `.so` slugs with Emscripten + slugs. Usually `.so`s in the generated wheels are actually Emscripten `.so`s + so this is good. If they are actually native `.so`s then there is a problem + either way. + {pr}`3903` + ### Packages - OpenBLAS has been added and scipy now uses OpenBLAS rather than CLAPACK diff --git a/packages/distlib/meta.yaml b/packages/distlib/meta.yaml index 30ed76801..94926eb47 100644 --- a/packages/distlib/meta.yaml +++ b/packages/distlib/meta.yaml @@ -8,7 +8,7 @@ source: url: https://files.pythonhosted.org/packages/76/cb/6bbd2b10170ed991cf64e8c8b85e01f2fb38f95d1bc77617569e0b0b26ac/distlib-0.3.6-py2.py3-none-any.whl build: post: | - find build/distlib-$PKG_VERSION/dist -name '*.exe' -delete + find $WHEELDIR -name '*.exe' -delete about: home: https://github.com/pypa/distlib PyPI: https://pypi.org/project/distlib diff --git a/packages/matplotlib/meta.yaml b/packages/matplotlib/meta.yaml index bd3f3a682..b98395e6e 100644 --- a/packages/matplotlib/meta.yaml +++ b/packages/matplotlib/meta.yaml @@ -30,7 +30,7 @@ build: -fno-lto script: export SETUPTOOLS_SCM_PRETEND_VERSION=$PKG_VERSION post: | - cd build/matplotlib-$PKG_VERSION/dist/matplotlib-$PKG_VERSION/ + cd $WHEELDIR rm -rf matplotlib/backends/qt_editor rm -rf matplotlib/backends/web_backend rm -rf sphinxext diff --git a/packages/setuptools/meta.yaml b/packages/setuptools/meta.yaml index c0e5652c1..d5774e03f 100644 --- a/packages/setuptools/meta.yaml +++ b/packages/setuptools/meta.yaml @@ -10,7 +10,7 @@ source: sha256: e728ca814a823bf7bf60162daf9db95b93d532948c4c0bea762ce62f60189078 build: post: | - find build/setuptools-*/dist -name '*.exe' -delete + find $WHEELDIR -name '*.exe' -delete requirements: run: - distutils diff --git a/pyodide-build/pyodide_build/buildpkg.py b/pyodide-build/pyodide_build/buildpkg.py index a181b1527..9cf807ee9 100755 --- a/pyodide-build/pyodide_build/buildpkg.py +++ b/pyodide-build/pyodide_build/buildpkg.py @@ -12,7 +12,6 @@ import os import shutil import subprocess import sys -import sysconfig import textwrap import urllib from collections.abc import Iterator @@ -36,6 +35,8 @@ from .common import ( get_build_environment_vars, get_pyodide_root, make_zip_archive, + modify_wheel, + replace_so_abi_tags, ) from .io import MetaConfig, _BuildSpec, _SourceSpec from .logger import logger @@ -369,30 +370,6 @@ def patch(pkg_root: Path, srcpath: Path, src_metadata: _SourceSpec) -> None: fd.write(b"\n") -def unpack_wheel(path: Path) -> None: - result = subprocess.run( - [sys.executable, "-m", "wheel", "unpack", path.name], - check=False, - encoding="utf-8", - cwd=path.parent, - ) - if result.returncode != 0: - logger.error(f"ERROR: Unpacking wheel {path.name} failed") - exit_with_stdio(result) - - -def pack_wheel(path: Path) -> None: - result = subprocess.run( - [sys.executable, "-m", "wheel", "pack", path.name], - check=False, - encoding="utf-8", - cwd=path.parent, - ) - if result.returncode != 0: - logger.error(f"ERROR: Packing wheel {path} failed") - exit_with_stdio(result) - - def compile( name: str, srcpath: Path, @@ -458,18 +435,6 @@ def compile( build(srcpath, outpath, build_env, backend_flags) -def replace_so_abi_tags(wheel_dir: Path) -> None: - """Replace native abi tag with emscripten abi tag in .so file names""" - build_soabi = sysconfig.get_config_var("SOABI") - assert build_soabi - ext_suffix = sysconfig.get_config_var("EXT_SUFFIX") - assert ext_suffix - build_triplet = "-".join(build_soabi.split("-")[2:]) - host_triplet = common.get_build_flag("PLATFORM_TRIPLET") - for file in wheel_dir.glob(f"**/*{ext_suffix}"): - file.rename(file.with_name(file.name.replace(build_triplet, host_triplet))) - - def copy_sharedlibs( wheel_file: Path, wheel_dir: Path, lib_dir: Path ) -> dict[str, Path]: @@ -537,50 +502,46 @@ def package_wheel( f"Unexpected number of wheels {len(rest) + 1} when building {pkg_name}" ) logger.info(f"Unpacking wheel to {str(wheel)}") - unpack_wheel(wheel) - wheel.unlink() + name, ver, _ = wheel.name.split("-", 2) - wheel_dir_name = f"{name}-{ver}" - wheel_dir = distdir / wheel_dir_name - # update so abi tags after build is complete but before running post script - # to maximize sanity. - replace_so_abi_tags(wheel_dir) + with modify_wheel(wheel) as wheel_dir: + # update so abi tags after build is complete but before running post script + # to maximize sanity. + replace_so_abi_tags(wheel_dir) - bash_runner.env.update({"WHEELDIR": str(wheel_dir)}) - post = build_metadata.post - bash_runner.run(post, script_name="post script") + bash_runner.env.update({"WHEELDIR": str(wheel_dir)}) + post = build_metadata.post + bash_runner.run(post, script_name="post script") - vendor_sharedlib = build_metadata.vendor_sharedlib - if vendor_sharedlib: - lib_dir = Path(common.get_build_flag("WASM_LIBRARY_DIR")) - copy_sharedlibs(wheel, wheel_dir, lib_dir) + vendor_sharedlib = build_metadata.vendor_sharedlib + if vendor_sharedlib: + lib_dir = Path(common.get_build_flag("WASM_LIBRARY_DIR")) + copy_sharedlibs(wheel, wheel_dir, lib_dir) - python_dir = f"python{sys.version_info.major}.{sys.version_info.minor}" - host_site_packages = Path(host_install_dir) / f"lib/{python_dir}/site-packages" - if build_metadata.cross_build_env: - subprocess.check_call( - ["pip", "install", "-t", str(host_site_packages), f"{name}=={ver}"] - ) + python_dir = f"python{sys.version_info.major}.{sys.version_info.minor}" + host_site_packages = Path(host_install_dir) / f"lib/{python_dir}/site-packages" + if build_metadata.cross_build_env: + subprocess.run( + ["pip", "install", "-t", str(host_site_packages), f"{name}=={ver}"], + check=True, + ) - cross_build_files = build_metadata.cross_build_files - if cross_build_files: - for file_ in cross_build_files: - shutil.copy((wheel_dir / file_), host_site_packages / file_) + cross_build_files = build_metadata.cross_build_files + if cross_build_files: + for file_ in cross_build_files: + shutil.copy((wheel_dir / file_), host_site_packages / file_) - test_dir = distdir / "tests" - nmoved = 0 - if build_metadata.unvendor_tests: - nmoved = unvendor_tests(wheel_dir, test_dir) - if nmoved: - with chdir(distdir): - shutil.make_archive(f"{pkg_name}-tests", "tar", test_dir) - pack_wheel(wheel_dir) - # wheel_dir causes pytest collection failures for in-tree packages like - # micropip. To prevent these, we get rid of wheel_dir after repacking the - # wheel. - shutil.rmtree(wheel_dir) - shutil.rmtree(test_dir, ignore_errors=True) + try: + test_dir = distdir / "tests" + nmoved = 0 + if build_metadata.unvendor_tests: + nmoved = unvendor_tests(wheel_dir, test_dir) + if nmoved: + with chdir(distdir): + shutil.make_archive(f"{pkg_name}-tests", "tar", test_dir) + finally: + shutil.rmtree(test_dir, ignore_errors=True) def unvendor_tests(install_prefix: Path, test_install_prefix: Path) -> int: diff --git a/pyodide-build/pyodide_build/common.py b/pyodide-build/pyodide_build/common.py index 5106884f0..b45f22a68 100644 --- a/pyodide-build/pyodide_build/common.py +++ b/pyodide-build/pyodide_build/common.py @@ -566,3 +566,64 @@ def _get_sha256_checksum(archive: Path) -> str: if len(chunk) < CHUNK_SIZE: break return h.hexdigest() + + +def unpack_wheel(wheel_path: Path, target_dir: Path | None = None) -> None: + if target_dir is None: + target_dir = wheel_path.parent + result = subprocess.run( + [sys.executable, "-m", "wheel", "unpack", wheel_path, "-d", target_dir], + check=False, + encoding="utf-8", + ) + if result.returncode != 0: + logger.error(f"ERROR: Unpacking wheel {wheel_path.name} failed") + exit_with_stdio(result) + + +def pack_wheel(wheel_dir: Path, target_dir: Path | None = None) -> None: + if target_dir is None: + target_dir = wheel_dir.parent + result = subprocess.run( + [sys.executable, "-m", "wheel", "pack", wheel_dir, "-d", target_dir], + check=False, + encoding="utf-8", + ) + if result.returncode != 0: + logger.error(f"ERROR: Packing wheel {wheel_dir} failed") + exit_with_stdio(result) + + +@contextmanager +def modify_wheel(wheel: Path) -> Iterator[Path]: + """Unpacks the wheel into a temp directory and yields the path to the + unpacked directory. + + The body of the with block is expected to inspect the wheel contents and + possibly change it. If the body of the "with" block is successful, on + exiting the with block the wheel contents are replaced with the updated + contents of unpacked directory. If an exception is raised, then the original + wheel is left unchanged. + """ + with TemporaryDirectory() as temp_dir: + unpack_wheel(wheel, Path(temp_dir)) + name, ver, _ = wheel.name.split("-", 2) + wheel_dir_name = f"{name}-{ver}" + wheel_dir = Path(temp_dir) / wheel_dir_name + yield wheel_dir + wheel.unlink() + pack_wheel(wheel_dir, wheel.parent) + + +def replace_so_abi_tags(wheel_dir: Path) -> None: + """Replace native abi tag with emscripten abi tag in .so file names""" + import sysconfig + + build_soabi = sysconfig.get_config_var("SOABI") + assert build_soabi + ext_suffix = sysconfig.get_config_var("EXT_SUFFIX") + assert ext_suffix + build_triplet = "-".join(build_soabi.split("-")[2:]) + host_triplet = get_build_flag("PLATFORM_TRIPLET") + for file in wheel_dir.glob(f"**/*{ext_suffix}"): + file.rename(file.with_name(file.name.replace(build_triplet, host_triplet))) diff --git a/pyodide-build/pyodide_build/io.py b/pyodide-build/pyodide_build/io.py index 7fb1a0f35..10b2341ef 100644 --- a/pyodide-build/pyodide_build/io.py +++ b/pyodide-build/pyodide_build/io.py @@ -176,6 +176,8 @@ class MetaConfig(BaseModel): config_raw = yaml.safe_load(stream) config = cls(**config_raw) + if config.source.path: + config.source.path = path.parent / config.source.path return config def to_yaml(self, path: Path) -> None: diff --git a/pyodide-build/pyodide_build/out_of_tree/build.py b/pyodide-build/pyodide_build/out_of_tree/build.py index 9645f23a0..623677b9c 100644 --- a/pyodide-build/pyodide_build/out_of_tree/build.py +++ b/pyodide-build/pyodide_build/out_of_tree/build.py @@ -28,4 +28,9 @@ def run(srcdir: Path, outdir: Path, exports: Any, args: list[str]) -> Path: with build_env_ctx as env: built_wheel = pypabuild.build(srcdir, outdir, env, " ".join(args)) - return Path(built_wheel) + + wheel_path = Path(built_wheel) + with common.modify_wheel(wheel_path) as wheel_dir: + common.replace_so_abi_tags(wheel_dir) + + return wheel_path diff --git a/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/__init__.py b/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/blah.cpython-311-x86_64-linux-gnu.so b/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/mypkg/blah.cpython-311-x86_64-linux-gnu.so new file mode 100644 index 000000000..e69de29bb diff --git a/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/pyproject.toml b/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/pyproject.toml new file mode 100644 index 000000000..21e9fc5d3 --- /dev/null +++ b/pyodide-build/pyodide_build/tests/replace_so_abi_tags_test_package/pyproject.toml @@ -0,0 +1,7 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "mypkg" +version = "1.0.0" diff --git a/pyodide-build/pyodide_build/tests/test_buildpkg.py b/pyodide-build/pyodide_build/tests/test_buildpkg.py index 48ad6547d..f13fd359d 100644 --- a/pyodide-build/pyodide_build/tests/test_buildpkg.py +++ b/pyodide-build/pyodide_build/tests/test_buildpkg.py @@ -5,7 +5,7 @@ from pathlib import Path import pydantic -from pyodide_build import buildpkg +from pyodide_build import buildpkg, common from pyodide_build.io import MetaConfig, _SourceSpec RECIPE_DIR = Path(__file__).parent / "_test_recipes" @@ -85,7 +85,12 @@ def test_subprocess_with_shared_env_logging(capfd, tmp_path): def test_prepare_source(monkeypatch): - monkeypatch.setattr(subprocess, "run", lambda *args, **kwargs: True) + class subprocess_result: + returncode = 0 + stdout = "" + + common.get_build_environment_vars() + monkeypatch.setattr(subprocess, "run", lambda *args, **kwargs: subprocess_result) monkeypatch.setattr(buildpkg, "check_checksum", lambda *args, **kwargs: True) monkeypatch.setattr(shutil, "unpack_archive", lambda *args, **kwargs: True) monkeypatch.setattr(shutil, "move", lambda *args, **kwargs: True) @@ -228,7 +233,7 @@ def test_copy_sharedlib(tmp_path): wheel_copy = tmp_path / wheel_file_name shutil.copy(wheel, wheel_copy) - buildpkg.unpack_wheel(wheel_copy) + common.unpack_wheel(wheel_copy) name, ver, _ = wheel.name.split("-", 2) wheel_dir_name = f"{name}-{ver}" wheel_dir = tmp_path / wheel_dir_name diff --git a/pyodide-build/pyodide_build/tests/test_cli.py b/pyodide-build/pyodide_build/tests/test_cli.py index 181693893..d2db755a8 100644 --- a/pyodide-build/pyodide_build/tests/test_cli.py +++ b/pyodide-build/pyodide_build/tests/test_cli.py @@ -435,7 +435,12 @@ def test_build1(tmp_path, monkeypatch): results["backend_flags"] = backend_flags return str(outdir / "a.whl") + from contextlib import nullcontext + monkeypatch.setattr(common, "check_emscripten_version", lambda: None) + monkeypatch.setattr(common, "modify_wheel", lambda whl: nullcontext()) + monkeypatch.setattr(common, "replace_so_abi_tags", lambda whl: None) + monkeypatch.setattr(pypabuild, "build", mocked_build) results: dict[str, Any] = {} @@ -451,3 +456,35 @@ def test_build1(tmp_path, monkeypatch): assert results["srcdir"] == srcdir assert results["outdir"] == outdir assert results["backend_flags"] == "x y z" + + +def test_build2_replace_so_abi_tags(selenium, tmp_path, monkeypatch): + """ + We intentionally include an "so" (actually an empty file) with Linux slug in + the name into the wheel generated from the package in + replace_so_abi_tags_test_package. Test that `pyodide build` renames it to + have the Emscripten slug. In order to ensure that this works on non-linux + machines too, we monkey patch config vars to look like a linux machine. + """ + import sysconfig + + config_vars = sysconfig.get_config_vars() + config_vars["EXT_SUFFIX"] = ".cpython-311-x86_64-linux-gnu.so" + config_vars["SOABI"] = "cpython-311-x86_64-linux-gnu" + + def my_get_config_vars(*args): + return config_vars + + monkeypatch.setattr(sysconfig, "get_config_vars", my_get_config_vars) + + srcdir = Path(__file__).parent / "replace_so_abi_tags_test_package" + outdir = tmp_path / "out" + app = typer.Typer() + app.command(**build.main.typer_kwargs)(build.main) # type:ignore[attr-defined] + result = runner.invoke(app, [str(srcdir), "--outdir", str(outdir)]) + wheel_file = next(outdir.glob("*.whl")) + print(zipfile.ZipFile(wheel_file).namelist()) + so_file = next( + x for x in zipfile.ZipFile(wheel_file).namelist() if x.endswith(".so") + ) + assert so_file.endswith(".cpython-311-wasm32-emscripten.so")