From 82fc4944ed6124d7d1c7c082ce24ad32ee045248 Mon Sep 17 00:00:00 2001 From: Kyle Benesch <4b796c65+github@gmail.com> Date: Thu, 12 Dec 2024 03:36:36 -0800 Subject: [PATCH] Add delocate as a generator to wheel info Refactor wheel record updater Using `delocate.__version__` would cause a cyclic import Importlib works better for type checkers Tests need to be changed to work with updated metadata Check for delocate generator header in tests Add to changelog --- Changelog.md | 2 + delocate/delocating.py | 29 +++++++++++--- delocate/fuse.py | 6 +-- delocate/tests/test_fuse.py | 27 +++++++++---- delocate/tests/test_scripts.py | 71 ++++++++++++++++++++++++---------- delocate/wheeltools.py | 65 +++++++++++++------------------ 6 files changed, 125 insertions(+), 75 deletions(-) diff --git a/Changelog.md b/Changelog.md index d68d7f28..4d3b5fd1 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,8 @@ rules on making a good Changelog. - `patch_wheel` function raises `FileNotFoundError` instead of `ValueError` on missing patch files. +- `delocate-wheel` and `delocate-fuse` now appends the delocate version used to + the wheels metadata. ### Deprecated diff --git a/delocate/delocating.py b/delocate/delocating.py index dd755551..2cf53e30 100644 --- a/delocate/delocating.py +++ b/delocate/delocating.py @@ -51,6 +51,11 @@ ) from .wheeltools import InWheel, rewrite_record +try: + from delocate._version import __version__ +except ImportError: # pragma: no cover + __version__ = "" + logger = logging.getLogger(__name__) # Prefix for install_name_id of copied libraries @@ -923,9 +928,15 @@ def _check_and_update_wheel_name( return wheel_path +def _get_delocate_generator_header() -> tuple[str, str]: + """Return Delocate's version info to be appended to the WHEEL metadata.""" + return ("Generator", f"delocate {__version__}".rstrip()) + + def _update_wheelfile(wheel_dir: Path, wheel_name: str) -> None: - """ - Update the WHEEL file in the wheel directory with the new platform tag. + """Update the WHEEL file in the wheel directory with updated metadata. + + Updates the platform tag and marks the wheel as modified by delocate. Parameters ---------- @@ -935,12 +946,20 @@ def _update_wheelfile(wheel_dir: Path, wheel_name: str) -> None: The name of the wheel. Used for determining the new platform tag. """ - platform_tag_set = parse_wheel_filename(wheel_name)[-1] + _name, _version, _, platform_tag_set = parse_wheel_filename(wheel_name) (file_path,) = wheel_dir.glob("*.dist-info/WHEEL") info = read_pkg_info(file_path) + + # Update tags to match current wheel name del info["Tag"] for tag in platform_tag_set: info.add_header("Tag", str(tag)) + + # Mark wheel as modifed by this version of Delocate + delocate_generator = _get_delocate_generator_header() + if delocate_generator not in info.items(): + info.add_header(*delocate_generator) + write_pkg_info(file_path, info) @@ -1073,7 +1092,6 @@ def delocate_wheel( libraries=libraries_in_lib_path, install_id_prefix=DLC_PREFIX + relpath(lib_sdir, wheel_dir), ) - rewrite_record(wheel_dir) out_wheel_ = Path(out_wheel) out_wheel_fixed = _check_and_update_wheel_name( out_wheel_, Path(wheel_dir), require_target_macos_version @@ -1081,7 +1099,8 @@ def delocate_wheel( if out_wheel_fixed != out_wheel_: out_wheel_ = out_wheel_fixed in_place = False - _update_wheelfile(Path(wheel_dir), out_wheel_.name) + _update_wheelfile(Path(wheel_dir), out_wheel_.name) + rewrite_record(wheel_dir) if len(copied_libs) or not in_place: if remove_old: os.remove(in_wheel) diff --git a/delocate/fuse.py b/delocate/fuse.py index 7cfecd43..3b1c98a4 100644 --- a/delocate/fuse.py +++ b/delocate/fuse.py @@ -47,7 +47,7 @@ def _copyfile(in_fname, out_fname): def _retag_wheel(to_wheel: Path, from_wheel: Path, to_tree: Path) -> str: - """Update the name and dist-info to reflect a univeral2 wheel. + """Update the name and dist-info to reflect a universal2 wheel. Parameters ---------- @@ -66,8 +66,8 @@ def _retag_wheel(to_wheel: Path, from_wheel: Path, to_tree: Path) -> str: to_tree = to_tree.resolve() # Add from_wheel platform tags onto to_wheel filename, but make sure to not # add a tag if it is already there - from_wheel_tags = parse_wheel_filename(from_wheel.name)[-1] - to_wheel_tags = parse_wheel_filename(to_wheel.name)[-1] + _, _, _, from_wheel_tags = parse_wheel_filename(from_wheel.name) + _, _, _, to_wheel_tags = parse_wheel_filename(to_wheel.name) add_platform_tags = ( f".{tag.platform}" for tag in from_wheel_tags - to_wheel_tags ) diff --git a/delocate/tests/test_fuse.py b/delocate/tests/test_fuse.py index 9ce86eae..f2129c89 100644 --- a/delocate/tests/test_fuse.py +++ b/delocate/tests/test_fuse.py @@ -1,11 +1,14 @@ """Test fusing two directory trees / wheels.""" +from __future__ import annotations + import os import shutil import subprocess import sys -from os.path import basename, dirname, isdir, relpath +from os.path import basename, dirname from os.path import join as pjoin +from pathlib import Path import pytest @@ -19,17 +22,25 @@ from .test_wheeltools import assert_record_equal -def assert_same_tree(tree1: str, tree2: str) -> None: +def assert_same_tree( + tree1: str | Path, tree2: str | Path, *, updated_metadata: bool = False +) -> None: + """Assert that `tree2` has files with the same content as `tree1`. + + If `updated_metadata` is True then the RECORD and WHEEL files are skipped. + """ for dirpath, dirnames, filenames in os.walk(tree1): - tree2_dirpath = pjoin(tree2, relpath(dirpath, tree1)) + tree2_dirpath = Path(tree2, Path(dirpath).relative_to(tree1)) for dname in dirnames: - assert isdir(pjoin(tree2_dirpath, dname)) + assert Path(tree2_dirpath, dname).is_dir() for fname in filenames: - tree1_path = pjoin(dirpath, fname) + tree1_path = Path(dirpath, fname) with open_readable(tree1_path, "rb") as fobj: - contents1 = fobj.read() - with open_readable(pjoin(tree2_dirpath, fname), "rb") as fobj: - contents2 = fobj.read() + contents1: bytes = fobj.read() + with open_readable(Path(tree2_dirpath, fname), "rb") as fobj: + contents2: bytes = fobj.read() + if updated_metadata and fname in {"RECORD", "WHEEL"}: + continue if fname == "RECORD": # Record can have different line orders assert_record_equal(contents1, contents2) else: diff --git a/delocate/tests/test_scripts.py b/delocate/tests/test_scripts.py index 6062e65e..1dc73adf 100644 --- a/delocate/tests/test_scripts.py +++ b/delocate/tests/test_scripts.py @@ -13,6 +13,7 @@ import shutil import subprocess import sys +import tempfile from os.path import basename, exists, realpath, splitext from os.path import join as pjoin from pathlib import Path @@ -46,6 +47,13 @@ assert_winfo_similar, ) +try: + from delocate._version import __version__ + + DELOCATE_GENERATOR_HEADER = f"Generator: delocate {__version__}" +except ImportError: + DELOCATE_GENERATOR_HEADER = "Generator: delocate" + DATA_PATH = (Path(__file__).parent / "data").resolve(strict=True) @@ -224,11 +232,14 @@ def test_path_dylibs(script_runner: ScriptRunner) -> None: def _check_wheel(wheel_fname: str | Path, lib_sdir: str | Path) -> None: wheel_fname = Path(wheel_fname).resolve(strict=True) - with InTemporaryDirectory(): - zip2dir(str(wheel_fname), "plat_pkg") - dylibs = Path("plat_pkg", "fakepkg1", lib_sdir) + with tempfile.TemporaryDirectory() as temp_dir: + plat_pkg_path = Path(temp_dir, "plat_pkg") + zip2dir(wheel_fname, plat_pkg_path) + dylibs = Path(plat_pkg_path, "fakepkg1", lib_sdir) assert dylibs.exists() assert os.listdir(dylibs) == ["libextfunc.dylib"] + (wheel_info,) = plat_pkg_path.glob("*.dist-info/WHEEL") + assert DELOCATE_GENERATOR_HEADER in wheel_info.read_text() @pytest.mark.xfail( # type: ignore[misc] @@ -252,7 +263,7 @@ def test_wheel(script_runner: ScriptRunner) -> None: script_runner.run( ["delocate-wheel", "-w", "fixed", fixed_wheel], check=True ) - _check_wheel(Path("fixed", basename(fixed_wheel)), ".dylibs") + _check_wheel(Path("fixed", Path(fixed_wheel).name), ".dylibs") # More than one wheel copy_name = "fakepkg1_copy-1.0-cp36-abi3-macosx_10_9_universal2.whl" shutil.copy2(fixed_wheel, copy_name) @@ -263,14 +274,14 @@ def test_wheel(script_runner: ScriptRunner) -> None: assert _proc_lines(result.stdout) == [ "Fixing: " + name for name in (fixed_wheel, copy_name) ] - _check_wheel(Path("fixed2", basename(fixed_wheel)), ".dylibs") + _check_wheel(Path("fixed2", Path(fixed_wheel).name), ".dylibs") _check_wheel(Path("fixed2", copy_name), ".dylibs") # Verbose - single wheel result = script_runner.run( ["delocate-wheel", "-w", "fixed3", fixed_wheel, "-v"], check=True ) - _check_wheel(Path("fixed3", basename(fixed_wheel)), ".dylibs") + _check_wheel(Path("fixed3", Path(fixed_wheel).name), ".dylibs") wheel_lines1 = [ "Fixing: " + fixed_wheel, "Copied to package .dylibs directory:", @@ -401,29 +412,49 @@ def _fix_break_fix(arch: str) -> None: ) def test_fuse_wheels(script_runner: ScriptRunner) -> None: # Some tests for wheel fusing - with InTemporaryDirectory(): + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) # Wheels need proper wheel filename for delocate-merge - to_wheel = "to_" + basename(PLAT_WHEEL) - from_wheel = "from_" + basename(PLAT_WHEEL) - zip2dir(PLAT_WHEEL, "to_wheel") - zip2dir(PLAT_WHEEL, "from_wheel") - dir2zip("to_wheel", to_wheel) - dir2zip("from_wheel", from_wheel) + to_wheel = temp_path / f"to_{Path(PLAT_WHEEL).name}" + from_wheel = temp_path / f"from_{Path(PLAT_WHEEL).name}" + zip2dir(PLAT_WHEEL, temp_path / "to_wheel") + zip2dir(PLAT_WHEEL, temp_path / "from_wheel") + dir2zip(temp_path / "to_wheel", to_wheel) + dir2zip(temp_path / "from_wheel", from_wheel) # Make sure delocate-fuse returns a non-zero exit code, it is no longer # supported - result = script_runner.run(["delocate-fuse", to_wheel, from_wheel]) + result = script_runner.run( + ["delocate-fuse", to_wheel, from_wheel], cwd=temp_path + ) assert result.returncode != 0 - script_runner.run(["delocate-merge", to_wheel, from_wheel], check=True) - zip2dir(to_wheel, "to_wheel_fused") - assert_same_tree("to_wheel_fused", "from_wheel") + + script_runner.run( + ["delocate-merge", to_wheel, from_wheel], check=True, cwd=temp_path + ) + zip2dir(to_wheel, temp_path / "to_wheel_fused") + assert_same_tree( + temp_path / "to_wheel_fused", + temp_path / "from_wheel", + updated_metadata=True, + ) # Test output argument - os.mkdir("wheels") script_runner.run( ["delocate-merge", to_wheel, from_wheel, "-w", "wheels"], check=True, + cwd=temp_path, + ) + zip2dir( + Path(temp_path, "wheels", to_wheel), temp_path / "to_wheel_refused" + ) + (wheel_info,) = Path(temp_path, "to_wheel_refused").glob( + "*.dist-info/WHEEL" + ) + assert DELOCATE_GENERATOR_HEADER in wheel_info.read_text() + assert_same_tree( + temp_path / "to_wheel_refused", + temp_path / "from_wheel", + updated_metadata=True, ) - zip2dir(pjoin("wheels", to_wheel), "to_wheel_refused") - assert_same_tree("to_wheel_refused", "from_wheel") @pytest.mark.xfail( # type: ignore[misc] diff --git a/delocate/wheeltools.py b/delocate/wheeltools.py index 49cc98c3..654635ac 100644 --- a/delocate/wheeltools.py +++ b/delocate/wheeltools.py @@ -7,16 +7,14 @@ import base64 import csv -import glob import hashlib import os -import sys -from collections.abc import Iterable +from collections.abc import Iterable, Iterator from itertools import product from os import PathLike -from os.path import abspath, basename, dirname, exists, relpath, splitext +from os.path import abspath, basename, dirname, exists, splitext from os.path import join as pjoin -from os.path import sep as psep +from pathlib import Path, PurePosixPath from typing import overload from packaging.utils import parse_wheel_filename @@ -24,21 +22,14 @@ from delocate.pkginfo import read_pkg_info, write_pkg_info from .tmpdirs import InTemporaryDirectory -from .tools import _unique_everseen, dir2zip, open_rw, zip2dir +from .tools import _unique_everseen, dir2zip, zip2dir class WheelToolsError(Exception): """Errors raised when reading or writing wheel files.""" -def _open_for_csv(name, mode): - """Deal with Python 2/3 open API differences.""" - if sys.version_info[0] < 3: - return open_rw(name, mode + "b") - return open_rw(name, mode, newline="", encoding="utf-8") - - -def rewrite_record(bdist_dir: str | PathLike) -> None: +def rewrite_record(bdist_dir: str | PathLike[str]) -> None: """Rewrite RECORD file with hashes for all files in `wheel_sdir`. Copied from :method:`wheel.bdist_wheel.bdist_wheel.write_record`. @@ -50,42 +41,38 @@ def rewrite_record(bdist_dir: str | PathLike) -> None: bdist_dir : str or Path-like Path of unpacked wheel file """ - info_dirs = glob.glob(pjoin(bdist_dir, "*.dist-info")) - if len(info_dirs) != 1: - raise WheelToolsError("Should be exactly one `*.dist_info` directory") - record_path = pjoin(info_dirs[0], "RECORD") - record_relpath = relpath(record_path, bdist_dir) + bdist_dir = Path(bdist_dir).resolve(strict=True) + try: + (info_dir,) = bdist_dir.glob("*.dist-info") + except ValueError: + msg = "Should be exactly one `*.dist_info` directory" + raise WheelToolsError(msg) from None + record_path = info_dir / "RECORD" # Unsign wheel - because we're invalidating the record hash - sig_path = pjoin(info_dirs[0], "RECORD.jws") - if exists(sig_path): - os.unlink(sig_path) - - def walk(): - for dir, dirs, files in os.walk(bdist_dir): - for f in files: - yield pjoin(dir, f) + Path(info_dir, "RECORD.jws").unlink(missing_ok=True) - def skip(path): - """Wheel hashes every possible file.""" - return path == record_relpath + def walk() -> Iterator[tuple[Path, str]]: + """Walk `(path, relative_posix_str)` for each file in `bdist_dir`.""" + for dirpath_, _dirnames, filenames in os.walk(bdist_dir): + dirpath = Path(dirpath_).resolve() + for file in filenames: + path = dirpath / file + yield path, str(PurePosixPath(path.relative_to(bdist_dir))) - with _open_for_csv(record_path, "w+") as record_file: + with record_path.open("w+", encoding="utf-8", newline="") as record_file: writer = csv.writer(record_file) - for path in walk(): - relative_path = relpath(path, bdist_dir) - if skip(relative_path): + for path, relative_path_for_record in walk(): + if path == record_path: hash = "" size: int | str = "" else: - with open(path, "rb") as f: - data = f.read() + data = path.read_bytes() digest = hashlib.sha256(data).digest() hash = "sha256={}".format( - base64.urlsafe_b64encode(digest).decode("ascii").strip("=") + base64.urlsafe_b64encode(digest).decode("ascii").rstrip("=") ) size = len(data) - path_for_record = relpath(path, bdist_dir).replace(psep, "/") - writer.writerow((path_for_record, hash, size)) + writer.writerow((relative_path_for_record, hash, size)) class InWheel(InTemporaryDirectory):