diff --git a/pyodide_build/recipe/builder.py b/pyodide_build/recipe/builder.py index ad604830..e7745313 100755 --- a/pyodide_build/recipe/builder.py +++ b/pyodide_build/recipe/builder.py @@ -531,7 +531,7 @@ def _package_wheel( ) -> None: """Package a wheel - This unpacks the wheel, unvendors tests if necessary, runs and "build.post" + This unpacks the wheel, runs and "build.post" script, and then repacks the wheel. Parameters @@ -585,18 +585,6 @@ def _package_wheel( host_site_packages / cross_build_file, ) - try: - test_dir = self.src_dist_dir / "tests" - if self.build_metadata.unvendor_tests: - nmoved = unvendor_tests( - wheel_dir, test_dir, self.build_metadata.retain_test_patterns - ) - if nmoved: - with chdir(self.src_dist_dir): - shutil.make_archive(f"{self.name}-tests", "tar", test_dir) - finally: - shutil.rmtree(test_dir, ignore_errors=True) - class RecipeBuilderStaticLibrary(RecipeBuilder): """ @@ -724,60 +712,6 @@ def copy_sharedlibs( return {} -def unvendor_tests( - install_prefix: Path, test_install_prefix: Path, retain_test_patterns: list[str] -) -> int: - """Unvendor test files and folders - - This function recursively walks through install_prefix and moves anything - that looks like a test folder under test_install_prefix. - - - Parameters - ---------- - install_prefix - the folder where the package was installed - test_install_prefix - the folder where to move the tests. If it doesn't exist, it will be - created. - - Returns - ------- - n_moved - number of files or folders moved - """ - n_moved = 0 - out_files = [] - shutil.rmtree(test_install_prefix, ignore_errors=True) - for root, _dirs, files in os.walk(install_prefix): - root_rel = Path(root).relative_to(install_prefix) - if root_rel.name == "__pycache__" or root_rel.name.endswith(".egg_info"): - continue - if root_rel.name in ["test", "tests"]: - # This is a test folder - (test_install_prefix / root_rel).parent.mkdir(exist_ok=True, parents=True) - shutil.move(install_prefix / root_rel, test_install_prefix / root_rel) - n_moved += 1 - continue - out_files.append(root) - for fpath in files: - if ( - fnmatch.fnmatchcase(fpath, "test_*.py") - or fnmatch.fnmatchcase(fpath, "*_test.py") - or fpath == "conftest.py" - ): - if any(fnmatch.fnmatchcase(fpath, pat) for pat in retain_test_patterns): - continue - (test_install_prefix / root_rel).mkdir(exist_ok=True, parents=True) - shutil.move( - install_prefix / root_rel / fpath, - test_install_prefix / root_rel / fpath, - ) - n_moved += 1 - - return n_moved - - # TODO: move this to common.py or somewhere else def needs_rebuild( pkg_root: Path, buildpath: Path, source_metadata: _SourceSpec diff --git a/pyodide_build/recipe/graph_builder.py b/pyodide_build/recipe/graph_builder.py index b408dd08..2485c23e 100755 --- a/pyodide_build/recipe/graph_builder.py +++ b/pyodide_build/recipe/graph_builder.py @@ -38,7 +38,7 @@ repack_zip_archive, ) from pyodide_build.logger import console_stdout, logger -from pyodide_build.recipe import loader +from pyodide_build.recipe import loader, unvendor from pyodide_build.recipe.builder import needs_rebuild from pyodide_build.recipe.spec import MetaConfig, _BuildSpecTypes @@ -65,7 +65,7 @@ class BasePackage: dependencies: set[str] # run + host dependencies unbuilt_host_dependencies: set[str] host_dependents: set[str] - unvendored_tests: Path | None = None + unvendor_tests: bool = False file_name: str | None = None install_dir: str = "site" _queue_idx: int | None = None @@ -165,18 +165,14 @@ def dist_artifact_path(self) -> Path | None: """ raise NotImplementedError() - def tests_path(self) -> Path | None: - """ - Return the path to the unvendored tests of the package. - """ - raise NotImplementedError() - @dataclasses.dataclass class PythonPackage(BasePackage): def __init__(self, pkgdir: Path, config: MetaConfig) -> None: super().__init__(pkgdir, config) + self.unvendor_tests = self.meta.build.unvendor_tests + def dist_artifact_path(self) -> Path | None: dist_dir = self.pkgdir / "dist" candidates = list( @@ -190,13 +186,6 @@ def dist_artifact_path(self) -> Path | None: return candidates[0] - def tests_path(self) -> Path | None: - tests = list((self.pkgdir / "dist").glob("*-tests.tar")) - assert len(tests) <= 1 - if tests: - return tests[0] - return None - @dataclasses.dataclass class SharedLibrary(BasePackage): @@ -216,9 +205,6 @@ def dist_artifact_path(self) -> Path | None: return candidates[0] - def tests_path(self) -> Path | None: - return None - @dataclasses.dataclass class StaticLibrary(BasePackage): @@ -228,9 +214,6 @@ def __init__(self, pkgdir: Path, config: MetaConfig) -> None: def dist_artifact_path(self) -> Path | None: return None - def tests_path(self) -> Path | None: - return None - class PackageStatus: def __init__( @@ -829,7 +812,9 @@ def generate_packagedata( if not pkg.file_name or pkg.package_type == "static_library": continue - if not Path(output_dir, pkg.file_name).exists(): + + wheel_file = output_dir / pkg.file_name + if not wheel_file.exists(): continue pkg_entry = PackageLockSpec( name=name, @@ -839,8 +824,6 @@ def generate_packagedata( package_type=pkg.package_type, ) - update_package_sha256(pkg_entry, output_dir / pkg.file_name) - pkg_entry.depends = [x.lower() for x in pkg.run_dependencies] if pkg.package_type not in ("static_library", "shared_library"): @@ -848,23 +831,24 @@ def generate_packagedata( pkg.meta.package.top_level if pkg.meta.package.top_level else [name] ) - packages[normalized_name.lower()] = pkg_entry - - if pkg.unvendored_tests: - packages[normalized_name.lower()].unvendored_tests = True - - # Create the test package if necessary - pkg_entry = PackageLockSpec( - name=name + "-tests", - version=pkg.version, - depends=[name.lower()], - file_name=pkg.unvendored_tests.name, - install_dir=pkg.install_dir, - ) + packages[normalized_name] = pkg_entry + + if pkg.unvendor_tests: + unvendored_test_file = unvendor.unvendor_tests_in_wheel(wheel_file) + if unvendored_test_file: + packages[normalized_name].unvendored_tests = True + test_file_entry = PackageLockSpec( + name=f"{name}-tests", + version=pkg.version, + depends=[name], + file_name=unvendored_test_file.name, + install_dir=pkg.install_dir, + ) + update_package_sha256(test_file_entry, unvendored_test_file) - update_package_sha256(pkg_entry, output_dir / pkg.unvendored_tests.name) + packages[normalized_name + "-tests"] = pkg_entry - packages[normalized_name.lower() + "-tests"] = pkg_entry + update_package_sha256(pkg_entry, wheel_file) # sort packages by name packages = dict(sorted(packages.items())) @@ -912,10 +896,6 @@ def copy_packages_to_dist_dir( output_dir / f"{dist_artifact_path.name}.metadata", ) - test_path = pkg.tests_path() - if test_path: - shutil.copy(test_path, output_dir) - def build_packages( packages_dir: Path, @@ -934,14 +914,10 @@ def build_packages( build_from_graph(pkg_map, build_args, build_dir, n_jobs, force_rebuild) for pkg in pkg_map.values(): dist_path = pkg.dist_artifact_path() - test_path = pkg.tests_path() if dist_path: pkg.file_name = dist_path.name - if test_path: - pkg.unvendored_tests = test_path - return pkg_map diff --git a/pyodide_build/recipe/unvendor.py b/pyodide_build/recipe/unvendor.py new file mode 100644 index 00000000..e1058bd7 --- /dev/null +++ b/pyodide_build/recipe/unvendor.py @@ -0,0 +1,114 @@ +import fnmatch +import os +import shutil +from pathlib import Path +from tempfile import TemporaryDirectory + +from packaging.utils import parse_wheel_filename + +from pyodide_build.common import ( + chdir, + modify_wheel, +) + + +def unvendor_tests_in_wheel( + wheel: Path, retain_patterns: list[str] | None = None +) -> Path | None: + """ + Unvendor tests from a wheel file. + + This function finds the tests in the wheel file, and extracts them to a separate + tar file. The tar file is placed in the same directory as the wheel file, with the -tests.tar suffix. + + Parameters + ---------- + wheel + The path to the wheel file. + + retain_patterns + A list of patterns to retain in the tests. If a pattern is found in the tests, it will be retained. + + Returns + ------- + The path to the tar file containing the tests. If no tests were found, returns None. + """ + retain_patterns = retain_patterns or [] + + name = parse_wheel_filename(wheel.name)[0] + file_format = "tar" + basename = f"{name}-tests" + fullname = f"{basename}.{file_format}" + destination = wheel.parent / fullname + + with TemporaryDirectory() as _tmpdir: + tmpdir = Path(_tmpdir) + test_dir = tmpdir / "tests" + with modify_wheel(wheel) as wheel_extract_dir: + nmoved = unvendor_tests( + wheel_extract_dir, + test_dir, + retain_patterns, + ) + if not nmoved: + return None + + with chdir(tmpdir): + generated_file = shutil.make_archive(basename, file_format, test_dir) + shutil.move(tmpdir / generated_file, destination) + + return destination + + +def unvendor_tests( + install_prefix: Path, test_install_prefix: Path, retain_test_patterns: list[str] +) -> int: + """Unvendor test files and folders + + This function recursively walks through install_prefix and moves anything + that looks like a test folder under test_install_prefix. + + + Parameters + ---------- + install_prefix + the folder where the package was installed + test_install_prefix + the folder where to move the tests. If it doesn't exist, it will be + created. + + Returns + ------- + n_moved + number of files or folders moved + """ + n_moved = 0 + out_files = [] + shutil.rmtree(test_install_prefix, ignore_errors=True) + for root, _dirs, files in os.walk(install_prefix): + root_rel = Path(root).relative_to(install_prefix) + if root_rel.name == "__pycache__" or root_rel.name.endswith(".egg_info"): + continue + if root_rel.name in {"test", "tests"}: + # This is a test folder + (test_install_prefix / root_rel).parent.mkdir(exist_ok=True, parents=True) + shutil.move(install_prefix / root_rel, test_install_prefix / root_rel) + n_moved += 1 + continue + out_files.append(root) + for fpath in files: + if ( + fnmatch.fnmatchcase(fpath, "test_*.py") + or fnmatch.fnmatchcase(fpath, "*_test.py") + or fpath == "conftest.py" + ): + if any(fnmatch.fnmatchcase(fpath, pat) for pat in retain_test_patterns): + continue + (test_install_prefix / root_rel).mkdir(exist_ok=True, parents=True) + shutil.move( + install_prefix / root_rel / fpath, + test_install_prefix / root_rel / fpath, + ) + n_moved += 1 + + return n_moved diff --git a/pyodide_build/tests/_test_wheels/wheel/package_with_bunch_of_test_directories-0.1.0-py3-none-any.whl b/pyodide_build/tests/_test_wheels/wheel/package_with_bunch_of_test_directories-0.1.0-py3-none-any.whl new file mode 100644 index 00000000..de07eaf7 Binary files /dev/null and b/pyodide_build/tests/_test_wheels/wheel/package_with_bunch_of_test_directories-0.1.0-py3-none-any.whl differ diff --git a/pyodide_build/tests/recipe/test_builder.py b/pyodide_build/tests/recipe/test_builder.py index a1f90658..48deda10 100644 --- a/pyodide_build/tests/recipe/test_builder.py +++ b/pyodide_build/tests/recipe/test_builder.py @@ -175,47 +175,6 @@ def test_get_helper_vars(tmp_path): ) -def test_unvendor_tests(tmpdir): - def touch(path: Path) -> None: - if path.is_dir(): - raise ValueError("Only files, not folders are supported") - path.parent.mkdir(parents=True, exist_ok=True) - path.touch() - - def rlist(input_dir): - """Recursively list files in input_dir""" - paths = sorted(input_dir.rglob("*")) - res = [] - - for el in paths: - if el.is_file(): - res.append(str(el.relative_to(input_dir))) - return res - - install_prefix = Path(str(tmpdir / "install")) - test_install_prefix = Path(str(tmpdir / "install-tests")) - - # create the example package - touch(install_prefix / "ex1" / "base.py") - touch(install_prefix / "ex1" / "conftest.py") - touch(install_prefix / "ex1" / "test_base.py") - touch(install_prefix / "ex1" / "tests" / "data.csv") - touch(install_prefix / "ex1" / "tests" / "test_a.py") - - n_moved = _builder.unvendor_tests(install_prefix, test_install_prefix, []) - - assert rlist(install_prefix) == ["ex1/base.py"] - assert rlist(test_install_prefix) == [ - "ex1/conftest.py", - "ex1/test_base.py", - "ex1/tests/data.csv", - "ex1/tests/test_a.py", - ] - - # One test folder and two test file - assert n_moved == 3 - - def test_create_constraints_file_no_override(tmp_path, dummy_xbuildenv): builder = RecipeBuilder.get_builder( recipe=RECIPE_DIR diff --git a/pyodide_build/tests/recipe/test_graph_builder.py b/pyodide_build/tests/recipe/test_graph_builder.py index f34cbca0..d14688e6 100644 --- a/pyodide_build/tests/recipe/test_graph_builder.py +++ b/pyodide_build/tests/recipe/test_graph_builder.py @@ -1,3 +1,4 @@ +import base64 import hashlib import zipfile from pathlib import Path @@ -53,7 +54,6 @@ def test_generate_lockfile(tmp_path, dummy_xbuildenv): pkg_map = graph_builder.generate_dependency_graph( RECIPE_DIR, {"pkg_1", "pkg_2", "libtest", "libtest_shared"} ) - hashes = {} for pkg in pkg_map.values(): if not pkg.file_name: match pkg.package_type: @@ -63,10 +63,36 @@ def test_generate_lockfile(tmp_path, dummy_xbuildenv): pkg.file_name = pkg.name + ".zip" # Write dummy package file for SHA-256 hash verification with zipfile.ZipFile(tmp_path / pkg.file_name, "w") as whlzip: - whlzip.writestr(pkg.file_name, data=pkg.file_name) - - with open(tmp_path / pkg.file_name, "rb") as f: - hashes[pkg.name] = hashlib.sha256(f.read()).hexdigest() + if pkg.package_type == "package": + # Make a proper .dist-info directory to make `python -m wheel pack/unpack` happy + dist_info_dir = f"{pkg.name}-{pkg.version}.dist-info" + metadata_content = f"name: {pkg.name}\nversion: {pkg.version}\n" + metadata_shasum = ( + base64.urlsafe_b64encode( + hashlib.sha256(metadata_content.encode()).digest() + ) + .decode() + .rstrip("=") + ) + wheel_content = "Wheel-Version: 1.0\nTag: py3-none-any\n" + wheel_shasum = ( + base64.urlsafe_b64encode( + hashlib.sha256(wheel_content.encode()).digest() + ) + .decode() + .rstrip("=") + ) + + whlzip.writestr(f"{dist_info_dir}/METADATA", metadata_content) + whlzip.writestr(f"{dist_info_dir}/WHEEL", wheel_content) + whlzip.writestr( + f"{dist_info_dir}/RECORD", + ( + f"{dist_info_dir}/METADATA,sha256={metadata_shasum},{len(metadata_content)}\n" + f"{dist_info_dir}/WHEEL,sha256={wheel_shasum},{len(wheel_content)}\n" + f"{dist_info_dir}/RECORD,,\n" + ), + ) package_data = graph_builder.generate_lockfile(tmp_path, pkg_map) assert package_data.info.arch == "wasm32" @@ -89,7 +115,7 @@ def test_generate_lockfile(tmp_path, dummy_xbuildenv): imports=["pkg_1"], package_type="package", install_dir="site", - sha256=hashes["pkg_1"], + sha256=package_data.packages["pkg-1"].sha256, # don't care ) assert package_data.packages["libtest-shared"].package_type == "shared_library" diff --git a/pyodide_build/tests/recipe/test_unvendor.py b/pyodide_build/tests/recipe/test_unvendor.py new file mode 100644 index 00000000..bd57a68c --- /dev/null +++ b/pyodide_build/tests/recipe/test_unvendor.py @@ -0,0 +1,118 @@ +import shutil +from pathlib import Path + +from pyodide_build.recipe import unvendor + + +def test_unvendor_tests_in_wheel(tmp_path): + test_wheel_file = "package_with_bunch_of_test_directories-0.1.0-py3-none-any.whl" + test_wheel_path_orig = ( + Path(__file__).parent.parent / "_test_wheels" / "wheel" / test_wheel_file + ) + shutil.copy(test_wheel_path_orig, tmp_path) + + test_wheel_path = tmp_path / test_wheel_file + test_tar_path = unvendor.unvendor_tests_in_wheel(test_wheel_path, []) + + assert test_tar_path.exists() + assert test_tar_path.name == "package-with-bunch-of-test-directories-tests.tar" + + # Check the contents of the tar file + shutil.unpack_archive(test_tar_path, extract_dir=tmp_path / "extracted_tests") + files = [f.name for f in (tmp_path / "extracted_tests").rglob("*")] + + should_exist = { + "tests", + "test", + "its_a_test.py", + "test_example.py", + "conftest.py", + "test_example2.py", + "keep_this_test.py", + } + + for f in should_exist: + assert f in files + + +def test_unvendor_tests_in_wheel_retain(tmp_path): + test_wheel_file = "package_with_bunch_of_test_directories-0.1.0-py3-none-any.whl" + test_wheel_path_orig = ( + Path(__file__).parent.parent / "_test_wheels" / "wheel" / test_wheel_file + ) + shutil.copy(test_wheel_path_orig, tmp_path) + + test_wheel_path = tmp_path / test_wheel_file + test_tar_path = unvendor.unvendor_tests_in_wheel( + test_wheel_path, + [ + "*test_example.py", + "*keep_this_test.py", + ], + ) + + assert test_tar_path.exists() + assert test_tar_path.name == "package-with-bunch-of-test-directories-tests.tar" + + # Check the contents of the tar file + shutil.unpack_archive(test_tar_path, extract_dir=tmp_path / "extracted_tests") + files = [f.name for f in (tmp_path / "extracted_tests").rglob("*")] + + should_exist = { + "tests", + "test", + "its_a_test.py", + "conftest.py", + "test_example2.py", + } + + for f in should_exist: + assert f in files + + should_not_exist = { + "keep_this_test.py", + } + + for f in should_not_exist: + assert f not in files + + +def test_unvendor_tests(tmp_path): + def touch(path: Path) -> None: + if path.is_dir(): + raise ValueError("Only files, not folders are supported") + path.parent.mkdir(parents=True, exist_ok=True) + path.touch() + + def rlist(input_dir): + """Recursively list files in input_dir""" + paths = sorted(input_dir.rglob("*")) + res = [] + + for el in paths: + if el.is_file(): + res.append(str(el.relative_to(input_dir))) + return res + + install_prefix = tmp_path / "install" + test_install_prefix = tmp_path / "install-tests" + + # create the example package + touch(install_prefix / "ex1" / "base.py") + touch(install_prefix / "ex1" / "conftest.py") + touch(install_prefix / "ex1" / "test_base.py") + touch(install_prefix / "ex1" / "tests" / "data.csv") + touch(install_prefix / "ex1" / "tests" / "test_a.py") + + n_moved = unvendor.unvendor_tests(install_prefix, test_install_prefix, []) + + assert rlist(install_prefix) == ["ex1/base.py"] + assert rlist(test_install_prefix) == [ + "ex1/conftest.py", + "ex1/test_base.py", + "ex1/tests/data.csv", + "ex1/tests/test_a.py", + ] + + # One test folder and two test file + assert n_moved == 3