InsightSoftwareConsortium / ITKPythonPackage

A setup script to generate ITK Python Wheels
https://itkpythonpackage.readthedocs.io
Apache License 2.0
63 stars 21 forks source link

Fix macOS itk wheel size explicitly removing _skbuild directory #256

Closed jcfr closed 1 year ago

jcfr commented 1 year ago

This commit works around an issue with the "clean" command (see [1]) that was not properly considering the current MACOSX_DEPLOYMENT_TARGET env. variable.

It does by explicitly deleting the scikit-build directory without relying on the "setup.py clean" command not respecting the selected deployment target.

Considering that

  1. the libraries installed while building the dependent itk-* wheels were accumulated in the install directory (e.g _skbuild/macosx-X.Y-<arch>-3.9/setuptools/lib)

  2. the wheel is generated by systematically archiving the content of the install directory. The manifest files are used to select which file to "copy" into the install directory but these are not used by the function creating the wheel archive. See [2]

  3. following https://github.com/scikit-build/scikit-build/commit/ac9648bf4 (post 0.8.1), the platform name was properly considered on macOS only when building the wheel (using "setup.py build") but not when cleaning the build directory (using "setup.py clean")

... all the wheels depending on "itk-core" where unexpectedly large because there are including the cumulative set of files from all the wheels built beforehand.

[1] https://github.com/scikit-build/scikit-build/issues/1012 [2] https://github.com/pypa/wheel/blob/0.40.0/src/wheel/wheelfile.py#L121-L141

jcfr commented 1 year ago

@thewtex Builds using these branches have been started on both misty (amd64) and grax (arm64) build machines :rocket:

dzenanz commented 1 year ago

Looks good on a glance, but preferably someone else should review too.

jcfr commented 1 year ago

For future reference, here is the comment from @thewtex

checking in the dist/ of both misty and grax the wheels are the right size!