From 7f46f89591981eb9e4bfb33ef2bcdf0767abac9d Mon Sep 17 00:00:00 2001 From: George Waters Date: Fri, 7 Jun 2024 04:24:28 -0400 Subject: [PATCH] Retag wheels automatically when fusing (#215) * Add '--retag' flag to delocate-fuse command This adds the ability to "retag" a universal2 fused wheel. When running delocate-fuse with this flag, it will update the filename and the dist-info file of the fused wheel to reflect that it is a universal2 wheel. * Update _update_wheelfile to use pkginfo functions * Detect if a wheel's name should use universal2 This updates '_get_archs_and_version_from_wheel_name' to check if both arm64 and x86_64 platform tags are in the wheel's filename. If they are both present, it changes the returned arch to be universal2 with the appropriate version. * Update retagging to use functions from delocating This is now much simpler and more generalized. It first updates the name to contain platform tags from both from_wheel and to_wheel. Next it calls '_check_and_update_wheel_name' which fixes any issues with the name AND will convert it to universal2 if necessary. And finally, it calls '_update_wheelfile' to update the info in the WHEEL file to match the new name. * Simplify set comparison syntax * Make fuse retagging the default behavior This also updates the fuse function to support pathlib paths and updates the retagging function to use pathlib paths. * Update Changelog & README for retagging * Update fuse_trees temp context This also required some typing changes to functions called by fused_trees. * Add another test for universal2 wheel version Depending on the version of each wheel being fused, the universal2 wheel's version can be either the x86_64 or the arm64 wheel's version. The first test for this is testing the scenario where the x86_64 wheel's version should be used. This new test is testing the scenario where the arm64 wheel's version should be used. * Move delocate-fuse functionality to delocate-merge This moves the old 'delocate-fuse' functionality into the new 'delocate-merge' command. Running 'delocate-fuse' will now print a message notifying the user of this change and then exit with an exit code of 1. This also updates relevant tests, the changelog, and the README. --- Changelog.md | 6 ++ README.rst | 28 ++++---- delocate/cmd/delocate_fuse.py | 43 ++++-------- delocate/cmd/delocate_merge.py | 43 ++++++++++++ delocate/delocating.py | 22 +++--- delocate/fuse.py | 112 ++++++++++++++++++++++++------ delocate/tests/test_delocating.py | 10 +++ delocate/tests/test_scripts.py | 21 +++--- delocate/wheeltools.py | 7 +- pyproject.toml | 1 + 10 files changed, 210 insertions(+), 83 deletions(-) create mode 100755 delocate/cmd/delocate_merge.py diff --git a/Changelog.md b/Changelog.md index d5631a0f..a9b4644a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,12 @@ rules on making a good Changelog. - Improved error message for when a MacOS target version is not met. [#211](https://github.com/matthew-brett/delocate/issues/211) +- `delocate-fuse` is no longer available and will throw an error when invoked. + To fuse two wheels together use `delocate-merge`. `delocate-merge` does not + overwrite the first wheel. It creates a new wheel with an automatically + determined name. If the old behavior is needed (not recommended), pin the + version to `delocate==0.11.0`. + [#215](https://github.com/matthew-brett/delocate/pull/215) ## [0.11.0] - 2024-03-22 diff --git a/README.rst b/README.rst index f3ab0ff3..711c358f 100644 --- a/README.rst +++ b/README.rst @@ -183,34 +183,36 @@ One solution to this problem is to do an entire ``arm64`` wheel build, and then an entire ``x86_64`` wheel build, and *fuse* the two wheels into a universal wheel. -That is what the ``delocate-fuse`` command does. +That is what the ``delocate-merge`` command does. Let's say you have built an ARM and Intel wheel, called, respectively: * ``scipy-1.9.3-cp311-cp311-macosx_12_0_arm64.whl`` * ``scipy-1.9.3-cp311-cp311-macosx_10_9_x86_64.whl`` -Then you could create a new fused (``universal2``) wheel in the `tmp` +Then you could create a new fused (``universal2``) wheel in the ``tmp`` subdirectory with:: - delocate-fuse scipy-1.9.3-cp311-cp311-macosx_12_0_arm64.whl scipy-1.9.3-cp311-cp311-macosx_10_9_x86_64.whl -w tmp + delocate-merge scipy-1.9.3-cp311-cp311-macosx_12_0_arm64.whl scipy-1.9.3-cp311-cp311-macosx_10_9_x86_64.whl -w tmp The output wheel in that case would be: -* ``tmp/scipy-1.9.3-cp311-cp311-macosx_12_0_arm64.whl`` - -Note that we specified an output directory above with the ``-w`` flag. If we -had not done that, then we overwrite the first wheel with the fused wheel. And -note that the wheel written into the ``tmp`` subdirectory has the same name as -the first-specified wheel. +* ``tmp/scipy-1.9.3-cp311-cp311-macosx_12_0_universal2.whl`` In the new wheel, you will find, using ``lipo -archs`` - that all binaries with the same name in each wheel are now universal (``x86_64`` and ``arm64``). -To be useful, you should rename the output wheel to reflect the fact that it is -now a universal wheel - in this case to: - -* ``tmp/scipy-1.9.3-cp311-cp311-macosx_12_0_universal2.whl`` + `:warning:` **Note:** In previous versions (``<0.12.0``) making dual architecture binaries was + performed with the ``delocate-fuse`` command. This commannd would overwrite the + first wheel passed in by default. This led to the user needing to rename the + wheel to correctly describe what platforms it supported. For this and other + reasons, wheels created with this were often incorrect. From version ``0.12.0`` + and on, the ``delocate-fuse`` command has been removed and replaced with + ``delocate-merge``. The ``delocate-merge`` command will create a new wheel with an + automatically generated name based on the wheels that were merged together. + There is no need to perform any further changes to the merged wheel's name. If + the old behavior is needed (not recommended), pin the version to + ``delocate==0.11.0``. Troubleshooting =============== diff --git a/delocate/cmd/delocate_fuse.py b/delocate/cmd/delocate_fuse.py index 0ad4e66c..718ee274 100755 --- a/delocate/cmd/delocate_fuse.py +++ b/delocate/cmd/delocate_fuse.py @@ -1,42 +1,25 @@ #!/usr/bin/env python3 """Fuse two (probably delocated) wheels. -Overwrites the first wheel in-place by default. +Command is no longer available. To fuse two wheels together use +'delocate-merge'. NOTE: 'delocate-merge' does not overwrite the first wheel. It +creates a new wheel with an automatically determined name. If the old behavior +is needed (not recommended), pin the version to 'delocate==0.11.0'. """ # vim: ft=python -from __future__ import absolute_import, division, print_function - -from argparse import ArgumentParser -from os.path import abspath, basename, expanduser -from os.path import join as pjoin - -from delocate.cmd.common import common_parser, verbosity_config -from delocate.fuse import fuse_wheels - -parser = ArgumentParser(description=__doc__, parents=[common_parser]) -parser.add_argument( - "wheels", nargs=2, metavar="WHEEL", type=str, help="Wheels to fuse" -) -parser.add_argument( - "-w", - "--wheel-dir", - action="store", - type=str, - help="Directory to store delocated wheels" - " (default is to overwrite 1st WHEEL input with 2nd)", -) +from __future__ import annotations def main() -> None: # noqa: D103 - args = parser.parse_args() - verbosity_config(args) - wheel1, wheel2 = [abspath(expanduser(wheel)) for wheel in args.wheels] - if args.wheel_dir is None: - out_wheel = wheel1 - else: - out_wheel = pjoin(abspath(expanduser(args.wheel_dir)), basename(wheel1)) - fuse_wheels(wheel1, wheel2, out_wheel) + print( + "'delocate-fuse' is no longer available. To fuse two wheels together" + " use 'delocate-merge'. NOTE: 'delocate-merge' does not overwrite the" + " first wheel. It creates a new wheel with an automatically determined" + " name. If the old behavior is needed (not recommended), pin the" + " version to 'delocate==0.11.0'." + ) + raise SystemExit(1) if __name__ == "__main__": diff --git a/delocate/cmd/delocate_merge.py b/delocate/cmd/delocate_merge.py new file mode 100755 index 00000000..fdf096f1 --- /dev/null +++ b/delocate/cmd/delocate_merge.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +"""Fuse two (probably delocated) wheels. + +Writes to a new wheel with an automatically determined name by default. +""" + +# vim: ft=python +from __future__ import annotations + +from argparse import ArgumentParser +from pathlib import Path + +from delocate.cmd.common import common_parser, verbosity_config +from delocate.fuse import fuse_wheels + +parser = ArgumentParser(description=__doc__, parents=[common_parser]) +parser.add_argument( + "wheels", nargs=2, metavar="WHEEL", type=str, help="Wheels to fuse" +) +parser.add_argument( + "-w", + "--wheel-dir", + action="store", + type=str, + help="Directory to store delocated wheels" + " (default is to store in the same directory as the 1st WHEEL with an" + " automatically determined name).", +) + + +def main() -> None: # noqa: D103 + args = parser.parse_args() + verbosity_config(args) + wheel1, wheel2 = [Path(wheel).resolve(strict=True) for wheel in args.wheels] + out_wheel = Path( + args.wheel_dir if args.wheel_dir is not None else wheel1.parent + ).resolve() + out_wheel.mkdir(parents=True, exist_ok=True) + fuse_wheels(wheel1, wheel2, out_wheel) + + +if __name__ == "__main__": + main() diff --git a/delocate/delocating.py b/delocate/delocating.py index 0678f4af..6e540bac 100644 --- a/delocate/delocating.py +++ b/delocate/delocating.py @@ -45,6 +45,7 @@ tree_libs, tree_libs_from_directory, ) +from .pkginfo import read_pkg_info, write_pkg_info from .tmpdirs import TemporaryDirectory from .tools import ( _is_macho_file, @@ -653,6 +654,14 @@ def _get_archs_and_version_from_wheel_name( raise ValueError(f"Invalid platform tag: {platform_tag.platform}") major, minor, arch = match.groups() platform_requirements[arch] = Version(f"{major}.{minor}") + # If we have a wheel name with arm64 and x86_64 we have to convert that to + # universal2 + if platform_requirements.keys() == {"arm64", "x86_64"}: + version = platform_requirements["arm64"] + if version == Version("11.0"): + version = platform_requirements["x86_64"] + platform_requirements = {"universal2": version} + return platform_requirements @@ -867,14 +876,11 @@ def _update_wheelfile(wheel_dir: Path, wheel_name: str) -> None: """ platform_tag_set = parse_wheel_filename(wheel_name)[-1] (file_path,) = wheel_dir.glob("*.dist-info/WHEEL") - with file_path.open(encoding="utf-8") as f: - lines = f.readlines() - with file_path.open("w", encoding="utf-8") as f: - for line in lines: - if line.startswith("Tag:"): - f.write(f"Tag: {'.'.join(str(x) for x in platform_tag_set)}\n") - else: - f.write(line) + info = read_pkg_info(file_path) + del info["Tag"] + for tag in platform_tag_set: + info.add_header("Tag", str(tag)) + write_pkg_info(file_path, info) def delocate_wheel( diff --git a/delocate/fuse.py b/delocate/fuse.py index 75bf3abf..7cfecd43 100644 --- a/delocate/fuse.py +++ b/delocate/fuse.py @@ -12,12 +12,19 @@ libraries. """ +from __future__ import annotations + import os import shutil -from os.path import abspath, exists, relpath, splitext +import tempfile +from os import PathLike +from os.path import exists, relpath, splitext from os.path import join as pjoin +from pathlib import Path + +from packaging.utils import parse_wheel_filename -from .tmpdirs import InTemporaryDirectory +from .delocating import _check_and_update_wheel_name, _update_wheelfile from .tools import ( chmod_perms, cmp_contents, @@ -39,7 +46,47 @@ def _copyfile(in_fname, out_fname): os.chmod(out_fname, perms) -def fuse_trees(to_tree, from_tree, lib_exts=(".so", ".dylib", ".a")): +def _retag_wheel(to_wheel: Path, from_wheel: Path, to_tree: Path) -> str: + """Update the name and dist-info to reflect a univeral2 wheel. + + Parameters + ---------- + to_wheel : Path + The path of the wheel to fuse into. + from_wheel : Path + The path of the wheel to fuse from. + to_tree : Path + The path of the directory tree to fuse into (update into). + + Returns + ------- + retag_name : str + The new, retagged name the out wheel should be. + """ + 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] + add_platform_tags = ( + f".{tag.platform}" for tag in from_wheel_tags - to_wheel_tags + ) + retag_name = to_wheel.stem + "".join(add_platform_tags) + ".whl" + + retag_name = _check_and_update_wheel_name( + Path(retag_name), to_tree, None + ).name + + _update_wheelfile(to_tree, retag_name) + + return retag_name + + +def fuse_trees( + to_tree: str | PathLike, + from_tree: str | PathLike, + lib_exts=(".so", ".dylib", ".a"), +): """Fuse path `from_tree` into path `to_tree`. For each file in `from_tree` - check for library file extension (in @@ -50,14 +97,14 @@ def fuse_trees(to_tree, from_tree, lib_exts=(".so", ".dylib", ".a")): Parameters ---------- - to_tree : str + to_tree : str or Path-like path of tree to fuse into (update into) - from_tree : str + from_tree : str or Path-like path of tree to fuse from (update from) lib_exts : sequence, optional filename extensions for libraries """ - for from_dirpath, dirnames, filenames in os.walk(from_tree): + for from_dirpath, dirnames, filenames in os.walk(Path(from_tree)): to_dirpath = pjoin(to_tree, relpath(from_dirpath, from_tree)) # Copy any missing directories in to_path for dirname in tuple(dirnames): @@ -83,24 +130,45 @@ def fuse_trees(to_tree, from_tree, lib_exts=(".so", ".dylib", ".a")): _copyfile(from_path, to_path) -def fuse_wheels(to_wheel, from_wheel, out_wheel): +def fuse_wheels( + to_wheel: str | PathLike, + from_wheel: str | PathLike, + out_wheel: str | PathLike, +) -> Path: """Fuse `from_wheel` into `to_wheel`, write to `out_wheel`. Parameters ---------- - to_wheel : str - filename of wheel to fuse into - from_wheel : str - filename of wheel to fuse from - out_wheel : str - filename of new wheel from fusion of `to_wheel` and `from_wheel` + to_wheel : str or Path-like + The path of the wheel to fuse into. + from_wheel : str or Path-like + The path of the wheel to fuse from. + out_wheel : str or Path-like + The path of the new wheel from fusion of `to_wheel` and `from_wheel`. If + a full path is given, (including the filename) it will be used as is. If + a directory is given, the fused wheel will be stored in the directory, + with the name of the wheel automatically determined. + + Returns + ------- + out_wheel : Path + The path of the new wheel from fusion of `to_wheel` and `from_wheel`. + + .. versionchanged:: 0.12 + `out_wheel` can now take a directory or None. """ - to_wheel, from_wheel, out_wheel = [ - abspath(w) for w in (to_wheel, from_wheel, out_wheel) - ] - with InTemporaryDirectory(): - zip2dir(to_wheel, "to_wheel") - zip2dir(from_wheel, "from_wheel") - fuse_trees("to_wheel", "from_wheel") - rewrite_record("to_wheel") - dir2zip("to_wheel", out_wheel) + to_wheel = Path(to_wheel).resolve(strict=True) + from_wheel = Path(from_wheel).resolve(strict=True) + out_wheel = Path(out_wheel) + with tempfile.TemporaryDirectory() as temp_dir: + to_wheel_dir = Path(temp_dir, "to_wheel") + from_wheel_dir = Path(temp_dir, "from_wheel") + zip2dir(to_wheel, to_wheel_dir) + zip2dir(from_wheel, from_wheel_dir) + fuse_trees(to_wheel_dir, from_wheel_dir) + if out_wheel.is_dir(): + out_wheel_name = _retag_wheel(to_wheel, from_wheel, to_wheel_dir) + out_wheel = out_wheel / out_wheel_name + rewrite_record(to_wheel_dir) + dir2zip(to_wheel_dir, out_wheel) + return out_wheel diff --git a/delocate/tests/test_delocating.py b/delocate/tests/test_delocating.py index 3ce23cb4..2ea0f3ee 100644 --- a/delocate/tests/test_delocating.py +++ b/delocate/tests/test_delocating.py @@ -722,6 +722,16 @@ def test_get_archs_and_version_from_wheel_name() -> None: ) == { "arm64": Version("12.0"), } + assert _get_archs_and_version_from_wheel_name( + "foo-1.0-py310-abi3-macosx_10_9_x86_64.macosx_11_0_arm64.whl" + ) == { + "universal2": Version("10.9"), + } + assert _get_archs_and_version_from_wheel_name( + "foo-1.0-py310-abi3-macosx_10_9_x86_64.macosx_12_0_arm64.whl" + ) == { + "universal2": Version("12.0"), + } with pytest.raises(InvalidWheelFilename, match="Invalid wheel filename"): _get_archs_and_version_from_wheel_name("foo.whl") diff --git a/delocate/tests/test_scripts.py b/delocate/tests/test_scripts.py index 75d3ba22..bd3a3ddd 100644 --- a/delocate/tests/test_scripts.py +++ b/delocate/tests/test_scripts.py @@ -402,22 +402,27 @@ def _fix_break_fix(arch: Text) -> None: def test_fuse_wheels(script_runner: ScriptRunner) -> None: # Some tests for wheel fusing with InTemporaryDirectory(): + # 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.whl") - dir2zip("from_wheel", "from_wheel.whl") - script_runner.run( - ["delocate-fuse", "to_wheel.whl", "from_wheel.whl"], check=True - ) - zip2dir("to_wheel.whl", "to_wheel_fused") + dir2zip("to_wheel", to_wheel) + dir2zip("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]) + 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") # Test output argument os.mkdir("wheels") script_runner.run( - ["delocate-fuse", "to_wheel.whl", "from_wheel.whl", "-w", "wheels"], + ["delocate-merge", to_wheel, from_wheel, "-w", "wheels"], check=True, ) - zip2dir(pjoin("wheels", "to_wheel.whl"), "to_wheel_refused") + zip2dir(pjoin("wheels", to_wheel), "to_wheel_refused") assert_same_tree("to_wheel_refused", "from_wheel") diff --git a/delocate/wheeltools.py b/delocate/wheeltools.py index e749301a..82fd9a46 100644 --- a/delocate/wheeltools.py +++ b/delocate/wheeltools.py @@ -3,6 +3,8 @@ Tools that aren't specific to delocation. """ +from __future__ import annotations + import base64 import csv import glob @@ -10,6 +12,7 @@ import os import sys from itertools import product +from os import PathLike from os.path import abspath, basename, dirname, exists, relpath, splitext from os.path import join as pjoin from os.path import sep as psep @@ -34,7 +37,7 @@ def _open_for_csv(name, mode): return open_rw(name, mode, newline="", encoding="utf-8") -def rewrite_record(bdist_dir: str) -> None: +def rewrite_record(bdist_dir: str | PathLike) -> None: """Rewrite RECORD file with hashes for all files in `wheel_sdir`. Copied from :method:`wheel.bdist_wheel.bdist_wheel.write_record`. @@ -43,7 +46,7 @@ def rewrite_record(bdist_dir: str) -> None: Parameters ---------- - bdist_dir : str + bdist_dir : str or Path-like Path of unpacked wheel file """ info_dirs = glob.glob(pjoin(bdist_dir, "*.dist-info")) diff --git a/pyproject.toml b/pyproject.toml index 5cb71aa6..2dc95453 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,7 @@ classifiers = [ delocate-addplat = "delocate.cmd.delocate_addplat:main" delocate-fuse = "delocate.cmd.delocate_fuse:main" delocate-listdeps = "delocate.cmd.delocate_listdeps:main" +delocate-merge = "delocate.cmd.delocate_merge:main" delocate-patch = "delocate.cmd.delocate_patch:main" delocate-path = "delocate.cmd.delocate_path:main" delocate-wheel = "delocate.cmd.delocate_wheel:main"