Skip to content

[iceberg-cpp] Add int128 namespace and MSVC 14.42 patches; make static export headers self-contained - #54315

Merged
Billy O'Neal (BillyONeal) merged 4 commits into
microsoft:masterfrom
rohanjain101:users/rohanjain/iceberg-cpp-int128-static-headers
Oct 6, 2026
Merged

Billy O'Neal (BillyONeal) merged 4 commits into
microsoft:masterfrom
rohanjain101:users/rohanjain/iceberg-cpp-int128-static-headers

Conversation

@rohanjain101

@rohanjain101 rohanjain101 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #54251.

int128 namespace. Adds int128-namespace.patch, a backport of apache/iceberg-cpp#972 (open). iceberg-cpp 0.4.0 declares int128_t/uint128_t in the global namespace in the installed iceberg/util/int128.h, which public headers such as iceberg/util/decimal.h and iceberg/expression/literal.h include. Consumers that declare their own int128_t with a different type therefore conflict as soon as they include iceberg. The patch moves the aliases into namespace iceberg. Every use inside iceberg-cpp is already in that namespace.

Static export headers. Addresses #54251 (review). The installed iceberg_*_export.h headers only select the static (non-dllimport) branch when ICEBERG_STATIC, ICEBERG_BUNDLE_STATIC, ICEBERG_DATA_STATIC or ICEBERG_REST_STATIC is defined. These are only propagated through the CMake targets' INTERFACE_COMPILE_DEFINITIONS. Consumers that don't use the CMake targets (MSBuild integration, hand-written build files) therefore compile __declspec(dllimport) references against the static libraries and fail with LNK2019. On static triplets, the port now rewrites the installed headers so the static branch is always taken. Dynamic triplets are unchanged.

MSVC 14.42 compatibility. Adds msvc-14.42-compat.patch, the library-source part of apache/iceberg-cpp#986. iceberg-cpp 0.4.0 does not compile with MSVC 14.42 (VS 2022 17.12); newer compilers are unaffected. The patch makes no functional change:

  • ParallelCollect (util/executor_util_internal.h) used a lambda in its requires-clause and lambdas expanded over a parameter pack. 14.42 rejects both (C3546/C2326/C3520). They are replaced with a class-template trait and helper function templates.
  • manifest_group.cc used a conditional operator over two move-only Result<ManifestEntryStreamPtr> prvalues. 14.42 tries to copy them (C2280). It is replaced with a small if/else helper.
  • snapshot_update.cc used the C++23 0UZ literal, which 14.42 does not support (C3688). It is replaced with size_t{0}.

Tested locally with iceberg-cpp[core,bundle,rest] on x64-windows and x64-windows-static (MSVC 14.51):

  • Both builds succeed. The installed static headers contain # if 1; the dynamic headers are unchanged.

  • A translation unit compiled with plain cl /MT (no ICEBERG_*_STATIC defines) against the static headers now has 0 __imp_ references to iceberg symbols (previously 14). static_assert(sizeof(::iceberg::int128_t) == 16) compiles.

  • A CMake consumer of the iceberg, bundle and rest static/shared targets builds and runs in Release and Debug. Static linking also needs [avro-cpp] Link compiled fmt and make config safe to find twice #54314 for the separate fmt duplicate-symbol issue.

  • Upstream util_test (376 tests, including the 19 ParallelCollect/executor tests) passes on the patched sources.

  • With MSVC 14.42 (VisualCppTools 14.42.34439, VS 2022 17.12), the full library build succeeds and executor_util_test.cc compiles.

  • Changes comply with the maintainer guide.

  • SHA512s are updated for each updated download. (No downloads changed.)

  • The "supports" clause reflects platforms that may be fixed by this new version. (Unchanged.)

  • Any fixed CI baseline entries are removed from that file. (None.)

  • Any patches that are no longer applied are deleted from the port's directory. (None.)

  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.

  • Only one version is added to each modified port's versions file.

This change was prepared with help from GitHub Copilot.

Rohan Jain and others added 2 commits October 5, 2026 11:48
…c export headers self-contained

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Static core-only packaging can fail when export headers for disabled optional targets are rewritten unconditionally.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Updates iceberg-cpp packaging for namespace safety, older MSVC compatibility, and static-library consumers.

Changes:

  • Moves 128-bit aliases into iceberg.
  • Adds MSVC 14.42 compatibility patches.
  • Makes installed static export headers self-contained.

Review used static inspection; builds and consumer examples were not run.

File Description
ports/​iceberg-cpp/​portfile.cmake Applies patches and rewrites static export headers.
ports/​iceberg-cpp/​int128-namespace.patch Namespaces 128-bit aliases.
ports/​iceberg-cpp/​msvc-14.42-compat.patch Adds MSVC compatibility changes.
ports/​iceberg-cpp/​vcpkg.json Increments the port revision.
versions/​i-/​iceberg-cpp.json Records port version 2.
versions/​baseline.json Updates the baseline revision.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ports/iceberg-cpp/portfile.cmake
Comment thread ports/iceberg-cpp/portfile.cmake Outdated
Rohan Jain and others added 2 commits October 5, 2026 12:53
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@BillyONeal Billy O'Neal (BillyONeal) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@BillyONeal
Billy O'Neal (BillyONeal) merged commit 2976266 into microsoft:master Oct 6, 2026
16 checks passed
@rohanjain101
rohanjain101 deleted the users/rohanjain/iceberg-cpp-int128-static-headers branch October 7, 2026 00:29
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