Repository navigation
Build and publish an arm64 Linux package - #556
Merged
Merged
Conversation
dcmqi released binaries for Linux and Windows on x86_64 only, so downstream consumers (notably the dcmqi PyPI wheels) had no arm64 package to repackage even though macOS has shipped both architectures since v1.5.1. Linux arm64 is built natively on ubuntu-24.04-arm inside the official manylinux_2_28_aarch64 image rather than through docker/Makefile: the dockcross images driving the x86_64 build are amd64-only cross toolchains, so they can neither run on an arm64 host nor execute the binaries they produce. The container also holds the glibc floor of the shipped binaries at 2.28, well below the runner's own Ubuntu 24.04; building bare on the runner would raise it to 2.39 and make the package unusable on most current distributions. Windows gains an arch matrix mirroring the existing macOS one, building natively on windows-11-arm with the Visual Studio ARM64 toolset. The x86_64 asset names (dcmqi-X-linux.tar.gz, dcmqi-X-win64.zip) are deliberately left unqualified so existing download URLs keep working; only the new architectures carry a suffix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds ARM64 build/release support for Linux and Windows to align release assets across platforms, while preserving the historical (unqualified) x86_64 asset names for Linux and Windows to avoid breaking existing download URLs.
Changes:
- Extend
CPACK_SYSTEM_NAMEto emitlinux-arm64andwin-arm64while keepinglinuxandwin64for x86_64. - Add a native manylinux-based
build-linux-arm64job to produce Linux ARM64 release assets. - Convert the Windows workflow into an x64/arm64 matrix and update release documentation/comments accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
RELEASE.md |
Documents the expanded set of six release assets (platform × architecture) and the naming constraints. |
CMakeLists.txt |
Updates packaging name logic to distinguish ARM64 for Linux/Windows without changing existing x86_64 names. |
.github/workflows/finalize-latest.yml |
Updates stale comment to reflect six parallel build jobs. |
.github/workflows/cmake-win.yml |
Adds an arch matrix to build/publish both Windows x64 and ARM64 artifacts. |
.github/workflows/cmake-linux.yml |
Adds a native manylinux container job to build/test/package/publish Linux ARM64 artifacts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+339
to
+340
| gh_version=$(curl -sSL https://api.github.com/repos/cli/cli/releases/latest \ | ||
| | sed -n 's/.*"tag_name": *"v\([^"]*\)".*/\1/p' | head -n 1) |
The job failed at configure with "source directory does not appear to contain CMakeLists.txt". In a container job the `github.workspace` expression and `$GITHUB_WORKSPACE` disagree: the expression is evaluated on the host and yields /home/runner/work/..., while the repository is mounted inside the container at /__w/.... The job was copied from the macOS build, which uses the expression throughout, so every path pointed at an empty host directory -- the mkdir steps quietly created one, and cmake then found nothing there. Shell steps now use $GITHUB_WORKSPACE, and the cache and artifact paths are workspace-relative, which is container-agnostic. Also address the review of the publish step: pin the gh CLI version instead of discovering it through an unauthenticated API call subject to the anonymous rate limit, mirroring the pinned ninja download in the macOS build, and enforce https on the transfer and on any redirect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
The Windows arm64 job cannot build. ITK's vendored GDCM fails to compile for ARM64 with MSVC: gdcmFilename.cxx includes <windows.h> from inside `namespace gdcm`, and on ARM64 winbase.h declares real inline _Interlocked* wrappers where x64 has only intrinsics. Those declarations land in the namespace, so later unqualified calls resolve to gdcm::_InterlockedIncrement and friends with the wrong signature -- a dozen C2664/C2665 errors. Disabling the module is not an option: dcmqi's own sources never touch GDCM, but ITKReview (enabled explicitly in CMakeExternals/ITK.cmake) hard-DEPENDS on ITKIOGDCM, which PRIVATE_DEPENDS on ITKGDCM. The fix belongs upstream in GDCM or the Slicer/ITK fork pinned here, so Windows arm64 waits for that rather than patching third-party source out of the superbuild. Linux arm64 is unaffected and stays: it builds green in 15m46s uncached. Also reverts this branch's cmake-macos.yml and cmake-win.yml changes. They duplicated part of #555 without its `if:` fix, leaving each file with a comment promising the save step is gated on build success while the condition was still `always()`. #555 carries all of it correctly. CMakeLists.txt keeps the win-arm64 CPACK_SYSTEM_NAME branch even though nothing builds it: an ARM64 target is 64-bit, so a local build would otherwise emit a package named "win64" containing arm64 binaries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both jobs declare their own permissions, and a job-level block replaces the workflow-level one rather than merging with it, so this block never applied to anything. cmake-macos.yml and cmake-win.yml already declare none, so removing it also makes the three CI workflows consistent. Clears the two githubactions:S8233 alerts SonarCloud raises on this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
| deployments: write | ||
| packages: write | ||
| # Permissions are declared per job. A job-level block replaces the workflow-level one | ||
| # rather than merging with it, so a workflow-level block here would have had no effect |
| packages: write | ||
| # Permissions are declared per job. A job-level block replaces the workflow-level one | ||
| # rather than merging with it, so a workflow-level block here would have had no effect | ||
| # on either job below -- and the macOS and Windows workflows already declare none. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
build-linux-arm64job so releases carry adcmqi-<version>-linux-arm64.tar.gz. Requested in ImagingDataCommons/dcmqi-python-distributions#44 — 3D Slicer has Linux arm64 users who currently have to build from source because no wheel exists for their platform.What this does
.github/workflows/cmake-linux.yml— newbuild-linux-arm64job on the nativeubuntu-24.04-armrunner, inside aquay.io/pypa/manylinux_2_28_aarch64container so the binaries carry the same glibc floor as the x86_64 package. Mirrors the existing macOS job's cache/validate/configure/build/test/package/publish shape.CMakeLists.txt—CPACK_SYSTEM_NAMEgains an arm64 branch. The x86_64 names stay unqualified (linux,win64) so existing download URLs keep working.finalize-latest.yml,RELEASE.md— documentation kept in step with the new asset.Container paths
The job deliberately uses
$GITHUB_WORKSPACEin shell steps and workspace-relative paths in action inputs, rather than the${{ github.workspace }}expression the other jobs use. In acontainer:job the two disagree: the expression is evaluated on the host and yields/home/runner/work/..., while the repository is mounted inside the container at/__w/.... The first push here used the expression and failed withThe source directory "/home/runner/work/dcmqi/dcmqi" does not appear to contain CMakeLists.txt— themkdir -psteps had quietly created the empty host path. There is a comment on the job saying so.Windows arm64 is not included
It was in the first version of this PR and had to come out. ITK's vendored GDCM does not compile for ARM64 with MSVC (failing job):
plus ~11 more (
_InterlockedDecrement,_InterlockedExchange,_InterlockedExchangeAdd,_InterlockedCompareExchange). Note the namespace.gdcmFilename.cxxincludes<windows.h>at line 100, insidenamespace gdcm(lines 20–187). On x64 the_Interlocked*functions are intrinsics andwinbase.hdeclares nothing; on ARM64 it declares real inline wrappers, which then land innamespace gdcmand shadow the intrinsics with the wrong signature.-DModule_ITKIOGDCM:BOOL=OFFdoes not help — dcmqi's sources never reference GDCM, butITKReview(turned on explicitly inCMakeExternals/ITK.cmake) hard-DEPENDSonITKIOGDCM, whichPRIVATE_DEPENDSonITKGDCM.The fix is one line in the right place, but it belongs upstream in GDCM or the Slicer/ITK fork pinned here, not in a
PATCH_COMMANDagainst third-party source in our superbuild. Windows arm64 follows once ITK carries it.CPACK_SYSTEM_NAMEalready handles the naming, so only the build job will be needed — it is retained deliberately, because an ARM64 target is 64-bit and a local build would otherwise emit a package calledwin64full of arm64 binaries.Relationship to #555
This branch originally carried part of #555's
cmake-macos.yml/cmake-win.ymlchanges but not itsif:fix, which left both files with a comment promising the cache save was gated on build success while the condition was stillalways(). Those files are now reverted tomasterhere; #555 has all of it, correctly. The two PRs no longer overlap.Testing
build-linux-arm64passes end to end, 15m46s with a cold cache. All other jobs unaffected.🤖 Generated with Claude Code