Skip to content

fix(cmake): generate version.h under PROJECT_BINARY_DIR to support add_subdirectory() builds - #906

Merged
wgtmac merged 1 commit into
apache:mainfrom
Angelia-Wang:fix/version-header-project-binary-dir
Sep 3, 2026
Merged

fix(cmake): generate version.h under PROJECT_BINARY_DIR to support add_subdirectory() builds#906
wgtmac merged 1 commit into
apache:mainfrom
Angelia-Wang:fix/version-header-project-binary-dir

Conversation

@Angelia-Wang

Copy link
Copy Markdown
Contributor

What

CMakeLists.txt generates the versioned header at ${CMAKE_BINARY_DIR}/src/iceberg/version.h, but the include path used to compile the iceberg target (ICEBERG_INCLUDES in src/iceberg/CMakeLists.txt) is ${PROJECT_BINARY_DIR}/src. This change makes both consistently use PROJECT_BINARY_DIR.

Why

When iceberg-cpp is built as the top-level project, CMAKE_BINARY_DIR and PROJECT_BINARY_DIR happen to be the same directory, so this mismatch is invisible.

When iceberg-cpp is consumed via add_subdirectory() from another CMake project (e.g. vendoring it under third/iceberg-cpp/), CMAKE_BINARY_DIR resolves to the outermost project's build directory, while PROJECT_BINARY_DIR correctly resolves to the build directory nested under the outer project (e.g. <outer_build_dir>/third/iceberg-cpp/). The generated version.h then ends up in the wrong place and any translation unit that does #include "iceberg/version.h" (e.g. catalog/rest/constant.h, deletes/dv_util.cc, deletes/dv_writer.cc) fails with a missing header when building as a nested dependency.

How

Use PROJECT_BINARY_DIR instead of CMAKE_BINARY_DIR as the output path passed to iceberg_configure_version_header(), matching the directory already used by ICEBERG_INCLUDES.

Testing

  • Standalone build: unaffected, since CMAKE_BINARY_DIR == PROJECT_BINARY_DIR at the top level.
  • Verified by consuming this repo via add_subdirectory() from an external CMake project; the build previously failed to locate iceberg/version.h and now succeeds.

…d_subdirectory() builds

CMakeLists.txt generates the versioned header at
${CMAKE_BINARY_DIR}/src/iceberg/version.h, but the include path used
to compile the iceberg target (ICEBERG_INCLUDES in
src/iceberg/CMakeLists.txt) is ${PROJECT_BINARY_DIR}/src. This change
makes both consistently use PROJECT_BINARY_DIR.

When iceberg-cpp is built as the top-level project, CMAKE_BINARY_DIR
and PROJECT_BINARY_DIR happen to be the same directory, so this
mismatch is invisible.

When iceberg-cpp is consumed via add_subdirectory() from another
CMake project (e.g. vendoring it under third/iceberg-cpp/),
CMAKE_BINARY_DIR resolves to the outermost project's build directory,
while PROJECT_BINARY_DIR correctly resolves to the build directory
nested under the outer project (e.g.
<outer_build_dir>/third/iceberg-cpp/). The generated version.h then
ends up in the wrong place and any translation unit that does
#include "iceberg/version.h" (e.g. catalog/rest/constant.h,
deletes/dv_util.cc, deletes/dv_writer.cc) fails with a missing header
when building as a nested dependency.

@wgtmac wgtmac 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.

Good catch!

@wgtmac
wgtmac merged commit b88b893 into apache:main Sep 3, 2026
18 checks passed
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.

2 participants