Skip to content

Update die_library to latest and fix macOS arm64 build - #40

Open
bahlaivlad wants to merge 1 commit into
elastic:mainfrom
bahlaivlad:update-dielib-macos-arm64
Open

bahlaivlad wants to merge 1 commit into
elastic:mainfrom
bahlaivlad:update-dielib-macos-arm64

Conversation

@bahlaivlad

Copy link
Copy Markdown

Bump .dielib_commit to da15bca (DIE engine 3.22) and add the build fixes needed for the new sources to compile on macOS arm64:

  • Put the bundled aqt Qt first on CMAKE_PREFIX_PATH so a system-wide Qt (e.g. Homebrew) cannot leak into transitive Qt6 config lookups.
  • Strip x86-only SIMD flags (-msse/-msse2/-mavx/-mavx2) and the USE_SSE2 define from the new xsimd targets on non-x86 builds; the runtime dispatcher never enables those paths on ARM.
  • Define fdopen=fdopen on the zlib and die targets on macOS: the vendored pre-1.2.12 zlib treats TARGET_OS_MAC (predefined by recent Apple clang) as Mac OS Classic and #defines fdopen to NULL, which breaks the SDK stdio headers.
  • Add -Wno-register for XArchive Algos sources that still use the C++17-removed register keyword.
  • Patch xdeflatedecoder.cpp via FetchContent PATCH_COMMAND to #undef its zlib-style local macro before including Qt headers, which on macOS reach CoreFoundation's CFMessagePort.h where local is a parameter name.

@cla-checker-service

cla-checker-service Bot commented Jun 12, 2026

Copy link
Copy Markdown

💚 CLA has been signed

Bump .dielib_commit to da15bca (DIE engine 3.22) and add the build
fixes needed for the new sources to compile on macOS arm64:

- Put the bundled aqt Qt first on CMAKE_PREFIX_PATH so a system-wide
  Qt (e.g. Homebrew) cannot leak into transitive Qt6 config lookups.
- Strip x86-only SIMD flags (-msse/-msse2/-mavx/-mavx2) and the
  USE_SSE2 define from the new xsimd targets on non-x86 builds; the
  runtime dispatcher never enables those paths on ARM.
- Define fdopen=fdopen on the zlib and die targets on macOS: the
  vendored pre-1.2.12 zlib treats TARGET_OS_MAC (predefined by recent
  Apple clang) as Mac OS Classic and #defines fdopen to NULL, which
  breaks the SDK stdio headers.
- Add -Wno-register for XArchive Algos sources that still use the
  C++17-removed register keyword.
- Patch xdeflatedecoder.cpp via FetchContent PATCH_COMMAND to #undef
  its zlib-style `local` macro before including Qt headers, which on
  macOS reach CoreFoundation's CFMessagePort.h where `local` is a
  parameter name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Add targeted macOS arm64 configure/build coverage for the new non-x86 path.

Pull request overview

Updates the bundled DIE library to 3.22 and adds macOS arm64 build compatibility fixes.

Changes:

  • Bumps DIE to commit da15bca.
  • Prioritizes bundled Qt dependencies.
  • Adds SIMD, zlib, compiler-warning, and Qt-header workarounds.
File summaries
File Summary
cmake/PatchDieLibrary.cmake Patches the XArchive local macro conflict.
cmake/FindDieLibrary.cmake Adds Qt, SIMD, zlib, and platform-specific build fixes.
.dielib_commit Updates the DIE dependency revision.
Review details

Suppressed comments (1)

cmake/FindDieLibrary.cmake:109

  • This new non-x86 branch is not exercised by CI: .github/workflows/build.yml:25-28 only builds macOS x64, while the ARM jobs at lines 37-38 are Linux. Since removing these SIMD flags/definitions is the core macOS arm64 fix, add an arm64 macOS configure/build job (or an equivalent targeted build test) before relying on this path.
if(NOT CMAKE_SYSTEM_PROCESSOR MATCHES "(x86_64|AMD64|i.86)")
  foreach(XSIMD_TARGET xsimd_sse2 xsimd_avx2)
    if(TARGET ${XSIMD_TARGET})
      get_target_property(XSIMD_OPTS ${XSIMD_TARGET} COMPILE_OPTIONS)
      if(XSIMD_OPTS)
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@calladoum-elastic calladoum-elastic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR breaks builds for non macOS 14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants