WIP: ENH: Replace the shell wheel build system with Python (supersedes #302) - #309
hjmjohnson wants to merge 16 commits into
Conversation
d2ec481 to
00357a9
Compare
dzenanz
left a comment
There was a problem hiding this comment.
I have taken a look. There is too much code to look carefully. I tried running locally:
M:\a\IPP>pixi run python3 "C:\Misc\ITKPythonPackage\scripts\build_wheels.py" --platform-env windows-py311 --itk-source-dir C:/Dev/ITK-git
Error: × could not find pixi.toml or pyproject.toml with tool.pixi at directory M:\a\IPP
M:\a\IPP>cd C:\Misc\ITKPythonPackage
M:\a\IPP>c:
C:\Misc\ITKPythonPackage>pixi run python3 "C:\Misc\ITKPythonPackage\scripts\build_wheels.py" --platform-env windows-py311 --itk-source-dir C:/Dev/ITK-git
WARN Encountered 2 warnings while parsing the manifest:
⚠ the `[system-requirements]` table is deprecated in favor of virtual packages on `platforms`
╭─[C:\Misc\ITKPythonPackage\pixi.toml:82:1]
81 │
82 │ ╭─▶ [feature.manylinux228-build.system-requirements]
83 │ │ linux = "5.4"
84 │ ├─▶ libc = { family = "glibc", version = "2.28" }
· ╰──── declare these on the `platforms` entries instead
85 │
╰────
help: e.g. platforms = [{ platform = "linux-64", cuda = "12" }]
⚠ the `[system-requirements]` table is deprecated in favor of virtual packages on `platforms`
╭─[C:\Misc\ITKPythonPackage\pixi.toml:95:23]
94 │ platforms = ["osx-arm64"]
95 │ system-requirements = { macos = "14.0" }
· ─────────┬────────
· ╰── declare these on the `platforms` entries instead
96 │
╰────
help: e.g. platforms = [{ platform = "linux-64", cuda = "12" }]
Error launching 'python3': The system cannot find the path specified. (os error 3)
C:\Misc\ITKPythonPackage>pixi run python "C:\Misc\ITKPythonPackage\scripts\build_wheels.py" --platform-env windows-py311 --itk-source-dir C:/Dev/ITK-git
WARN Encountered 2 warnings while parsing the manifest:
⚠ the `[system-requirements]` table is deprecated in favor of virtual packages on `platforms`
╭─[C:\Misc\ITKPythonPackage\pixi.toml:82:1]
81 │
82 │ ╭─▶ [feature.manylinux228-build.system-requirements]
83 │ │ linux = "5.4"
84 │ ├─▶ libc = { family = "glibc", version = "2.28" }
· ╰──── declare these on the `platforms` entries instead
85 │
╰────
help: e.g. platforms = [{ platform = "linux-64", cuda = "12" }]
⚠ the `[system-requirements]` table is deprecated in favor of virtual packages on `platforms`
╭─[C:\Misc\ITKPythonPackage\pixi.toml:95:23]
94 │ platforms = ["osx-arm64"]
95 │ system-requirements = { macos = "14.0" }
· ─────────┬────────
· ╰── declare these on the `platforms` entries instead
96 │
╰────
help: e.g. platforms = [{ platform = "linux-64", cuda = "12" }]
Running >>>>>: git fetch --tags origin ; # in cwd=C:\Dev\ITK-git with check=False
Traceback (most recent call last):
File "C:\Misc\ITKPythonPackage\scripts\build_wheels.py", line 628, in <module>
build_wheels_main()
~~~~~~~~~~~~~~~~~^^
File "C:\Misc\ITKPythonPackage\scripts\build_wheels.py", line 450, in build_wheels_main
raise RuntimeError(
...<2 lines>...
)
RuntimeError: Failed to fetch ITK tags in C:\Dev\ITK-git: fatal: 'origin' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
C:\Misc\ITKPythonPackage>
|
This seems to be doing something: |
|
That failed too: run.log |
ef0c2d4 to
1af2b15
Compare
Windows verification complete — handing back to Linux/macOSEverything Windows-specific on this branch is now verified by real builds, and one cross-platform defect found along the way is fixed. Head is What was verified on Windows 11 / VS 2022 (i7-9800X, 32 GB)
Defects fixed on this branch by the Windows workAll Windows-only unless noted; none were reachable from Linux/macOS testing.
Plus three test fixes so the suite runs on Windows ( For Linux/macOS
|
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
c219d94 to
afdfaf8
Compare
|
Thanks for running it locally — your Your build failure is our bug, not your invocation. The log ends at:
This PR stays WIP until the Doxygen prerequisite is resolved. "Too much Platform verification status as of afdfaf8
97 unit tests pass; One known caveat: a single aarch64 clean build failed while linking with a |
afdfaf8 to
e53db8d
Compare
|
@dzenanz thank you for the log file — it identified a real bug, and that bug is This should have been a What your log found, and the fixYour run failed at:
Two changes:
Six regression tests cover both (103 total, up from 97). Your exact command |
fb3e45b to
42922e0
Compare
1af2b15 to
ca9994a
Compare
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
ca9994a to
f19b202
Compare
Windows x86_64 status at
|
| jobs | step 02 | peak build RSS sum | min free RAM | outcome |
|---|---|---|---|---|
-j16 |
— | — | — | killed, 2 of 3 cold runs |
-j12 |
4842 s | 15.7 GB | 5.75 GB | pass |
-j8 |
5291 s | 12.2 GB | 8.96 GB | pass |
What changed:
scripts/build_jobs.py: jobs =min(cpu_count, floor(ram_gb / 2.5)), dependency-free RAM detection on all three OSes, falls back tocpu_countif RAM is unknown. Overrides:ITK_BUILD_JOBS,ITK_BUILD_LOAD_LIMIT.- ninja
-lis now the thread count rather than the job count. ninja honours-lon Windows (verified on 1.13.2), so-l<jobs>on a box with more threads than jobs throttled the build to one compiler. - ITK wheels on Windows now build with
-GNinja, as the module wheels already did. - No link pool:
link.exepeaked at 1.45 GB and the link phase is ~1.3 min of an ~80 min step.
The 2.5 GB/job constant is calibrated on MSVC only. Linux and macOS builds on this head should report the # Build parallelism: line the builder prints and their step-02 time; a slowdown over ~10 % there would mean gcc/clang deserve a different budget. Inside dockcross, sysconf reports host RAM, not a cgroup limit, so a memory-limited container may need ITK_BUILD_JOBS set explicitly.
Notes for other platforms
- The branch was rewritten today; reset local branches to origin before rebasing.
- Published caches predate the step-table and parallelism changes and carry the old
IPP/scripts/. The documented consumer path re-clones the branch viaITKPYTHONPACKAGE_TAG, so it gets the fixes; a consumer without that variable does not. - Full detail and the task board are in the coordination gist: https://gist.github.com/87d847e38a2c50c48730817042b475d3
f19b202 to
7b77300
Compare
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
Co-Authored-By: Cavan Riley <cavan-riley@uiowa.edu>
Document the build driver's full command line surface so a coding agent can compose a correct invocation without reading the source. CLAUDE.md redirects to AGENTS.md, which is authoritative. The worked example covers the common request shape: a local macOS build against an unmerged ITK pull request, with custom compiler flags and a non-default deployment target. It fetches the pull request ref by hand, because the driver fetches tags only and a failed checkout warns and keeps the commit already present.
Every build_wheels.py example omitted the pixi environment selector, so following the README landed in the default environment and aborted. Also corrects the dockcross image tag defaults, the directory the publish script searches for tarballs, the step count, and the --cleanup description, which read as the opposite of what the flag does. Adds the expected wheel tags and how to confirm a macOS binary matches its tag.
The ITK C++ build ran ninja with -j<cpu_count> -l<cpu_count>. On a 16-thread machine with 32 GB that is 2 GB per job; the template-heavy wrapping units peak at 3.9 GB in a single cl.exe, and the build was killed for memory pressure in 2 of 3 clean attempts. Jobs are now min(cpu_count, floor(ram_gb / 2.5)). The budget comes from a three-point sweep on that machine, working sets sampled every 0.5 s over the whole build: -j16 step 02 4716 s killed in 2 of 3 clean builds -j12 step 02 4842 s peak build-process sum 15.7 GB, min free 5.75 GB -j8 step 02 5291 s peak build-process sum 12.2 GB, min free 8.96 GB Above 12 there is at most 2.7% to gain; below it the cost climbs to 12%. The marginal cost between -j8 and -j12 was 0.84 GB per job because peaks rarely coincide, so 2.5 GB/job lands on the measured optimum (floor(31.8 / 2.5) = 12) with ~5 GB of floor. The value floors rather than rounds so it cannot land one job past the tested point. -l stays at the thread count and must never follow the job count. ninja honours -l on Windows (1.13.2: eight 3 s jobs under `-l 0.5` serialise to 25 s), so -l<jobs> on a box with more threads than jobs throttles ninja to one compiler whenever a saturating compile pushes load past <jobs>. Links are not the pressure -- link.exe peaked at 1.45 GB and the whole link phase is ~1.3 min of an ~80 min step -- so there is no link pool. ITK_BUILD_JOBS and ITK_BUILD_LOAD_LIMIT override for CI runner sizing. physical_memory_gb() is dependency-free so it works in every build env; unknown memory falls back to the previous cpu_count behaviour.
…s are pixi's conda-forge vs2022 activation exports CMAKE_GENERATOR="Visual Studio 17 2022". scikit-build-core honours it, so the seven ITK wheel builds in step 03 ran under MSBuild while ITK itself was configured with -G Ninja. The remote-module wheel build already passes -GNinja explicitly for this reason; do the same for the ITK wheels. Measured on a 16-thread Windows box: step 03 went from 190-216 s under the Visual Studio generator to 159-168 s under Ninja. An explicit -G also makes CMake ignore the ambient CMAKE_GENERATOR_PLATFORM/TOOLSET that the same activation exports and that Ninja would otherwise reject.
The reusable GitHub Actions workflow fetches this file by URL to learn which container image to pre-pull. Removing it left the action without a source for the image tags, and the file is the only place those coordinates are stated once for both architectures. The manylinux2014 branch is dropped: ITK 6 targets manylinux_2_28 only, so that path now reports an unknown version rather than resolving.
b84f9cd to
ec7ab53
Compare
Nothing references docs/make.bat; docs/Makefile remains for building the documentation. The cache-path note claimed the packaging scripts enforce the build path convention without saying that build_wheels.py does not. Its default build root matches the convention only on Windows, so a cache built by calling it directly is unusable by the reusable workflow unless --build-dir-root is set. State the paths as a table and give the command to check a finished cache.
Replaces the shell and PowerShell wheel-build system with a pixi-driven
Python one, and moves ITK 6 to stable-ABI (
abi3) wheels onmanylinux_2_28, macOS arm64 14.0, and Windows x86_64. Supersedes #302.
Status: WIP — refinements pending; not yet ready for in-depth review.
Successfully built on Windows, macOS arm64, Linux x86_64, and Linux
aarch64. The Doxygen/environment defect found by @dzenanz is fixed: build
tools now resolve inside the target
--platform-env, and launching fromthe wrong environment fails immediately with the environment name to use.
One item needs a decision before merge: the rebuilt macOS cache is
verified but not yet published to ITKPythonBuilds. The two Linux caches
and the Windows cache are published.
Platform verification — clean builds from this branch
Every row is a from-scratch build of this branch, not an incremental one.
cp311-abi3-manylinux_2_28_x86_64cp311-abi3-manylinux_2_28_aarch64cp311-abi3-macosx_14_0_arm64cp311-abi3-win_amd64macOS binaries were checked with
otool -l: real.sofiles reportminos 14.0, matching theirmacosx_14_0_arm64tag. That check mattersbecause a component wheel tagged below its true
minosinstalls on oldermacOS and then fails in dyld at import — installable and broken, rather
than loudly uninstallable.
Remote-module end-to-end (the consumer path)
Proven on Windows x86_64 and Linux x86_64, each run as a fresh
consumer with zero ITK wheels present beforehand, so a leftover
artifact could not produce a false pass.
Linux x86_64, ITKBioCell, 8 minutes,
rc=0:itk_biocell-1.0.0-cp311-abi3-manylinux_2_28_x86_64.whl, 0.58 MBRequires-Dist: itk<7,>=6.0.0b1— the dependency rewrite firedauditwheelpyproject.tomlrestored byte-identical after the buildAn earlier Windows pass was found to be riding on leftover wheels; the
zero-wheel precondition was added in response and both platforms were
re-run under it.
Unit tests and hooks
pytest: 97 passed. pytest is a developer-only dependency; thenormal build path does not import it.
pre-commit run --all-files: exit 0, every hookPassed(black, ruff, shellcheck, shfmt, taplo, yaml/toml, whitespace).
Known caveat: intermittent aarch64 link truncation
One clean aarch64 build failed while linking:
The identical clean build then passed, with no truncation and no short
object files. Evidence points at the container file-sharing layer rather
than this branch: 158 GB free at the time, the truncated archive was
exactly 49,152 bytes (12 × 4096, a dropped write tail rather than a
partial compile), and aarch64 is the only platform writing through a
Docker bind mount onto a macOS filesystem.
Docker Desktop updated itself between the two runs (29.7.2 → 29.8.0), so
the version cannot be credited with the fix, and one pass does not
disprove an intermittent fault. Recommended handling: retry a clean
build first; do not treat a truncation as a defect in this branch.
Commit series
Ten topical commits, no fixups and no merge commits. Bug fixes found
during platform testing were folded into the commits that introduced the
code, so the series reads as though the defects never existed.
Cavan Riley is credited via
Co-Authored-Byon the seven commitscarrying his original work.