Skip to content

Commit fee1884

Browse files
abnobdossAbanoub Doss
andauthored
ci: build Windows test leg in Debug (#806)
## What Let the CMake CI scripts use `ICEBERG_BUILD_TYPE` and default to `Debug`, including on Windows. MSVC builds with debug info now use embedded `/Z7` debug info through `CMP0141` and `CMAKE_MSVC_DEBUG_INFORMATION_FORMAT` so Debug objects remain cacheable by sccache. The build scripts also use the same single-config Ninja build path on every platform. Since these jobs pass `-G Ninja`, `CMAKE_BUILD_TYPE` controls the build and `--config Release` / `ctest -C Release` were not needed. ## Why Windows was the only main `Test` workflow leg still forcing the helper scripts through `Release`. After this change, the helper-script CMake test jobs default to `Debug` on pull requests and `main`; `ICEBERG_BUILD_TYPE` remains available if the project wants an override. Release verification and the SQL Catalog workflow's explicit Windows `Release` matrix entry are unchanged. The original Windows Debug blocker in #39 was an `IMPORTED_LOCATION` failure from a multi-config CMake path. That specific failure no longer applies because the helper scripts now use Ninja single-config builds. The remaining Debug-only blocker was MSVC rejecting a `constexpr static PartitionValues` test helper: `PartitionValues` owns a `std::vector`, and MSVC Debug's checked-iterator mode (`_ITERATOR_DEBUG_LEVEL != 0`) makes that construction fail constant evaluation. This PR constructs the empty `PartitionValues` at the call site instead. This follows the CI direction discussed in #799. ## Validation Fork validation passed: - `Test / AMD64 Windows 2025` passed in 23m48s on the pull-request run: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474339/job/84929642907 - `SQL Catalog / AMD64 Windows 2025` passed in 4m13s: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474303/job/84929643407 - `Meson / AMD64 Windows 2025` passed in 3m21s: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474339/job/84929642897 The Windows Debug test job reported 45% sccache hits, 385 hits, 477 misses, and 0 errors. The first post-merge Apache `main` run will reseed Windows cache entries because the previous entries were built from Release objects. Closes #39. --------- Co-authored-by: Abanoub Doss <abanoub.doss@gmail.com>
1 parent 0825f56 commit fee1884

5 files changed

Lines changed: 29 additions & 54 deletions

File tree

.github/workflows/sql_catalog_test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ jobs:
5858
cmake_extra_args: ""
5959
- title: AMD64 Windows 2025
6060
runs-on: windows-2025
61-
cmake_build_type: Release
61+
cmake_build_type: Debug
6262
cmake_extra_args: -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake
6363
env:
6464
SCCACHE_DIR: ${{ github.workspace }}/.sccache

ci/scripts/build_example.sh

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,26 +49,19 @@ CMAKE_ARGS=(
4949

5050
if is_windows; then
5151
CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake")
52-
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Release")
53-
else
54-
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Debug")
5552
fi
5653

54+
build_type="${ICEBERG_BUILD_TYPE:-Debug}"
55+
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}")
56+
5757
cmake "${CMAKE_ARGS[@]}" ${source_dir}
58-
if is_windows; then
59-
cmake --build . --config Release
60-
if [[ "${run_example}" == "ON" ]]; then
61-
if [[ -x ./demo_example.exe ]]; then
62-
./demo_example.exe
58+
cmake --build .
59+
if [[ "${run_example}" == "ON" ]]; then
60+
if is_windows; then
61+
./demo_example.exe
6362
else
64-
./Release/demo_example.exe
63+
./demo_example
6564
fi
66-
fi
67-
else
68-
cmake --build .
69-
if [[ "${run_example}" == "ON" ]]; then
70-
./demo_example
71-
fi
7265
fi
7366

7467
popd

ci/scripts/build_iceberg.sh

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ is_windows() {
3737
[[ "${OSTYPE}" == "msys" || "${OSTYPE}" == "win32" || "${OSTYPE}" == "cygwin" ]]
3838
}
3939

40-
if is_windows; then
41-
build_type=${7:-Release}
42-
else
43-
build_type=${7:-Debug}
44-
fi
40+
build_type=${7:-Debug}
4541

4642
CMAKE_ARGS=(
4743
"-G Ninja"
@@ -70,16 +66,16 @@ else
7066
fi
7167

7268
if is_windows; then
73-
CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake")
74-
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}")
75-
else
76-
# Pass an externally provided toolchain (e.g. vcpkg for the SigV4 job)
77-
if [[ -n "${CMAKE_TOOLCHAIN_FILE:-}" ]]; then
78-
CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}")
79-
fi
80-
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}")
69+
CMAKE_TOOLCHAIN_FILE="${CMAKE_TOOLCHAIN_FILE:-C:/vcpkg/scripts/buildsystems/vcpkg.cmake}"
8170
fi
8271

72+
# Pass an externally provided toolchain, or the default Windows vcpkg toolchain.
73+
if [[ -n "${CMAKE_TOOLCHAIN_FILE:-}" ]]; then
74+
CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}")
75+
fi
76+
77+
CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}")
78+
8379
if [[ "${build_enable_sccache}" == "ON" ]]; then
8480
CMAKE_ARGS+=("-DCMAKE_CXX_COMPILER_LAUNCHER=sccache")
8581
CMAKE_ARGS+=("-DCMAKE_C_COMPILER_LAUNCHER=sccache")
@@ -91,16 +87,10 @@ if [[ -n "${ICEBERG_EXTRA_CMAKE_ARGS:-}" ]]; then
9187
fi
9288

9389
cmake "${CMAKE_ARGS[@]}" ${source_dir}
94-
if is_windows; then
95-
cmake --build . --config "${build_type}" --target install
96-
if [[ "${run_tests}" == "ON" ]]; then
97-
ctest --output-on-failure -C "${build_type}"
98-
fi
99-
else
100-
cmake --build . --target install
101-
if [[ "${run_tests}" == "ON" ]]; then
90+
91+
cmake --build . --target install
92+
if [[ "${run_tests}" == "ON" ]]; then
10293
ctest --output-on-failure
103-
fi
10494
fi
10595

10696
popd

cmake_modules/IcebergSccache.cmake

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,12 @@
1818
if(MSVC_TOOLCHAIN AND "${CMAKE_CXX_COMPILER_LAUNCHER}" STREQUAL "sccache")
1919
message(STATUS "Configuring sccache for MSVC")
2020

21-
# Remove /Zi or /ZI
22-
string(REGEX REPLACE "/Z[iI]" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}")
23-
string(REGEX REPLACE "/Z[iI]" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
21+
# Keep MSVC Debug objects cacheable by sccache without affecting Release.
22+
set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "$<$<CONFIG:Debug,RelWithDebInfo>:Embedded>")
2423

25-
string(REGEX REPLACE "/Z[iI]" "" CMAKE_C_FLAGS_RELWITHDEBINFO
26-
"${CMAKE_C_FLAGS_RELWITHDEBINFO}")
27-
string(REGEX REPLACE "/Z[iI]" "" CMAKE_CXX_FLAGS_RELWITHDEBINFO
28-
"${CMAKE_CXX_FLAGS_RELWITHDEBINFO}")
29-
30-
# Add /Z7
31-
set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} /Z7")
32-
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Z7")
33-
34-
set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} /Z7")
35-
set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} /Z7")
24+
# CMP0141 normally handles these flags; normalize any flags injected elsewhere.
25+
foreach(flag_var CMAKE_C_FLAGS_DEBUG CMAKE_CXX_FLAGS_DEBUG CMAKE_C_FLAGS_RELWITHDEBINFO
26+
CMAKE_CXX_FLAGS_RELWITHDEBINFO)
27+
string(REGEX REPLACE "/Z[iI]" "/Z7" ${flag_var} "${${flag_var}}")
28+
endforeach()
3629
endif()

src/iceberg/test/scan_test_base.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ class ScanTestBase : public testing::TestWithParam<int8_t> {
221221
std::shared_ptr<PartitionSpec> spec = nullptr) {
222222
std::vector<std::pair<std::string, PartitionValues>> files_with_partitions;
223223
for (const auto& path : added_files) {
224-
files_with_partitions.emplace_back(path, kEmptyPartition);
224+
files_with_partitions.emplace_back(path, PartitionValues{});
225225
}
226226
return MakeAppendSnapshotWithPartitionValues(format_version, snapshot_id,
227227
parent_snapshot_id, sequence_number,
@@ -350,7 +350,6 @@ class ScanTestBase : public testing::TestWithParam<int8_t> {
350350
private:
351351
int manifest_counter_ = 0;
352352
int manifest_list_counter_ = 0;
353-
constexpr static PartitionValues kEmptyPartition{};
354353
};
355354

356355
} // namespace iceberg

0 commit comments

Comments
 (0)