Fix off64_t portability on musl-based systems - #6779
Conversation
github-actions
Bot
added
the
area:ThirdParty
|
FYI: I'm looking in to a more robust fix in GDCM as an upstream fix. The complex conditionals tend to be fragile, and I think a better solution could be achieved. I am not opposed to this workaround, but I don't think it is the correct permanent fix. |
This was a quick fix to get a build working. GDCM as several places where system depended defines are checked, that could be problematic or incorrectly defined with muslinux. It appears this defined approach was preferred over try compiles in these places, and appears to be a source of trouble and changes needed. |
gdcmFileStreamer.cxx used off64_t and hand-maintained a list of platforms on which it must be #defined to off_t. musl-based distributions (Alpine) were not on that list and fail with "off64_t has not been declared": musl gates its LFS64 names on _LARGEFILE64_SOURCE and, unlike glibc, does not imply that from _GNU_SOURCE. musl is also removing those aliases, so enabling _LARGEFILE64_SOURCE is not a durable answer. Extending the list is not a fix either. off64_t is a typedef, so the preprocessor cannot test for it; every guard is a proxy for "which libc is this", which is why the list grew one platform at a time. The glibc build worked only because the C++ driver implicitly defines _GNU_SOURCE, which turns on __USE_LARGEFILE64; the same block does not compile as C. Separately, the _FILE_OFFSET_BITS 64 definition in this file was a no-op. It sat after the gdcm*.h include block, which transitively pulls <features.h>, so it was ignored. On 32-bit glibc off_t therefore stayed 32-bit and FSeeko/FTello/FTruncate silently narrowed, breaking writes past 2 GB. Move the definition ahead of all includes and replace off64_t with a GDCM-owned gdcm::offset_t: off_t on POSIX, int64_t on the Windows CRT. This deletes the __APPLE__ || __FreeBSD__ || __OpenBSD__ || __NetBSD__ || __EMSCRIPTEN__ list without adding anything. A static_assert makes a 32-bit off_t a build error rather than silent truncation. offset_t does not appear in gdcmFileStreamer.h, so the change is contained to one translation unit and alters no public API. Verified: Alpine/musl reproduces the reported error before the change and compiles clean after; on 32-bit glibc the static_assert fires with the old define placement and passes with the new one; macOS arm64 full build at -Wall -Wextra with all six TestFileStreamer tests passing and no change to the pre-existing failure set. Reported downstream as InsightSoftwareConsortium/ITK#6779. Assisted-by: Claude Code -- cross-platform instrumentation and patch drafting
gdcmFileStreamer.cxx used off64_t and hand-maintained a list of platforms on which it must be #defined to off_t. musl-based distributions (Alpine) were not on that list and fail with "off64_t has not been declared": musl gates its LFS64 names on _LARGEFILE64_SOURCE and, unlike glibc, does not imply that from _GNU_SOURCE. musl is also removing those aliases, so enabling _LARGEFILE64_SOURCE is not a durable answer. Extending the list is not a fix either. off64_t is a typedef, so the preprocessor cannot test for it; every guard is a proxy for "which libc is this", which is why the list grew one platform at a time. The glibc build worked only because the C++ driver implicitly defines _GNU_SOURCE, which turns on __USE_LARGEFILE64; the same block does not compile as C. Separately, the _FILE_OFFSET_BITS 64 definition in this file was a no-op. It sat after the gdcm*.h include block, which transitively pulls <features.h>, so it was ignored. On 32-bit glibc off_t therefore stayed 32-bit and FSeeko/FTello/FTruncate silently narrowed, breaking writes past 2 GB. Move the definition ahead of all includes and replace off64_t with a GDCM-owned gdcm::offset_t: off_t on POSIX, int64_t on the Windows CRT. This deletes the __APPLE__ || __FreeBSD__ || __OpenBSD__ || __NetBSD__ || __EMSCRIPTEN__ list without adding anything. A static_assert makes a 32-bit off_t a build error rather than silent truncation. offset_t does not appear in gdcmFileStreamer.h, so the change is contained to one translation unit and alters no public API. Verified: Alpine/musl reproduces the reported error before the change and compiles clean after; on 32-bit glibc the static_assert fires with the old define placement and passes with the new one; macOS arm64 full build at -Wall -Wextra with all six TestFileStreamer tests passing and no change to the pre-existing failure set. Reported downstream as InsightSoftwareConsortium/ITK#6779. Assisted-by: Claude Code -- cross-platform instrumentation and patch drafting
|
I've opened an upstream fix for this at malaterre/GDCM#228, so ITK can eventually drop the local patch instead of carrying a platform list that grows one distro at a time. To be clear up front: this PR works. I built it on Alpine and it resolves the error. Nothing below is a reason to hold it if you want Alpine unblocked now — that's your call, and option (b) at the bottom is entirely reasonable. Why upstream took a different shape
Worth noting the glibc path currently works somewhat by accident: I checked whether A separate latent bug in the same file, which ITK's vendored copy has verbatimWhile investigating I found that Consequence on 32-bit glibc (armhf, i386, 32-bit MIPS):
The same probe with this PR applied gives Upstream #228 hoists the define and adds Two ways forward, and I don't have a stake in which:
Either way it's worth recording the local change per ITK's ThirdParty patching convention so the next |
3 participants
Problem
Builds on musl-based Linux distributions (like Alpine Linux) were failing with:
The issue occurs in
gdcmFileStreamer.cxxwhere the code definesoff64_tfor Windows and BSD systems, but doesn't handle musl-based systems which don't defineoff64_tby default.Solution
Modified the platform detection condition to check for non-glibc systems. This covers musl and other libc implementations by defining
off64_tasoff_t, which is the standard portability approach.The specific change checks for
!defined(__GLIBC__)to detect non-glibc systems, complementing existing checks for specific BSD and Apple platforms.Testing
This fix should allow musl-based builds to compile without the
off64_terror while maintaining compatibility with existing platforms (Windows, glibc-based Linux, BSD variants, etc.).Fixes SimpleITK build failures on Alpine Linux and other musl-based distributions.