New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Add build script update step to download script #233
Conversation
scripts/dockcross-manylinux-download-cache-and-build-module-wheels.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I updated zarr CI to match. Windows packages fail immediately.
Thanks @dzenanz . Note that Windows CI workflows don't seem to use the |
Seeing an issue where ITK is being rebuilt on MacOS and Linux after a different version of ITKPythonPackage is checked out. Investigating. EDIT: There is a disparity where |
1f29eb4
to
4ecbb4f
Compare
After a minor update, now even 3.7 fails with |
@dzenanz The issue you've described happening in your branch is tracked in #235 . It looks like your branch is fixed at c98c5e3 which does not revert the manylinux image tag. I am now seeing a successful Python 3.7 build in ITKSplitComponents after reverting the manylinux tag. I am convinced that the newer manylinux image is the source of the error you're describing. I understand that simply reverting the tag will not allow ITKIOOMEZarrNGFF CI to complete as it requires NASM. The resolution for the sudo issue needs to happen in dockcross rather than in ITKPythonPackage. The workaround for other modules that do not require NASM for now is to revert to an earlier image. Let's continue discussion in #235. I will verify that other ITKSplitComponents builds also pass with these changes and then get this PR cleaned up for easier review. |
4ecbb4f
to
329438a
Compare
@thewtex @dzenanz PR is ready for review at your convenience. Tested by ITKSplitComponents: https://github.com/InsightSoftwareConsortium/ITKSplitComponents/actions/runs/3586129456 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good.
scripts/dockcross-manylinux-download-cache-and-build-module-wheels.sh
Outdated
Show resolved
Hide resolved
Addresses shortfall where ITKPythonBuilds archives are packaged with ITKPythonPackage build scripts from the time of archive generation, but no clear mechanism was available for updates to apply build script patches. Adds mechanism in download scripts where ITKPythonBuilds archives are fetched, decompressed, and optionally patched prior to continuing with module builds.
Replaces `TARBALL_SPECIALIZATION` parameter with distinct `MANYLINUX_VERSION` and `IMAGE_TAG` parameters to be set by the user prior to build. The resulting script fetches the correct dockcross image for building specialized wheels and artifacts. Resolves an issue where `manylinux2014` wheels were attempted to build on `-manylinux_2_28` images.
This reverts commit ecdc6b2.
329438a
to
2d3a552
Compare
Most recent push applies requested documentation fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @tbirdso !
MANYLINUX_VERSION=_2_28 | ||
IMAGE_TAG=20221128-2024e4b | ||
MANYLINUX_VERSION=${MANYLINUX_VERSION:=_2_28} | ||
IMAGE_TAG=${IMAGE_TAG:=20221108-102ebcc} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
Several related updates:
sudo
issue tracked in Latest manylinux images missingsudo
#235 and Latestmanylinux_2_28-x64
image missingsudo
dockcross/dockcross#746These changes are tested at https://github.com/InsightSoftwareConsortium/ITKSplitComponents/actions/runs/3586129456