fix(seekdb): Fix Windows SDK OPTIONAL macro naming conflict with C++ enum value#917
Open
ep-12221 wants to merge 5 commits into
Open
fix(seekdb): Fix Windows SDK OPTIONAL macro naming conflict with C++ enum value#917ep-12221 wants to merge 5 commits into
OPTIONAL macro naming conflict with C++ enum value#917ep-12221 wants to merge 5 commits into
Conversation
…cro (SAL annotation). This caused the OPTIONAL enum value in the `enum class ObReqCheckSumCheckLevel` within ob_req_packet_code.h to be expanded to nothing by the preprocessor, resulting in a syntax error (expected identifier). Additionally, CMakeLists.txt had duplicate links to zstd_objs.lib and lz4-all.lib. Fix: Added `#ifdef`/`#undef OPTIONAL` before the enum definition to resolve the macro conflict. Removed the duplicate library references in CMakeLists.txt (which were already covered by zstd_1_3_8_objs.lib). Impact: Affects only Windows platform compilation; no impact on Linux/macOS. DIMA: 2026061800116825248
The zstd target was replaced by zstd_1_3_8; the stale link reference broke clean builds (ninja: missing and no known rule to make it). Existing dirty build dirs masked the issue via a leftover .lib file. Co-Authored-By: Claude Opus 4.6 <[REDACTED_EMAIL]>
Contributor
Author
|
The mapping Dima issue is [[windows] Windows compilation failure] |
hnwyllmm
approved these changes
Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task Description
Fix a compilation error on Windows caused by a naming conflict between a Windows SDK macro and a C++ enumerator.
Solution Description
Root Cause: The Windows SDK header file
winnt.hdefines an empty macroOPTIONALfor SAL annotations, such as_In_opt_. When the C++ code inob_req_packet_code.husesOPTIONALas an enumerator inenum class ObReqCheckSumCheckLevel, the MSVC/clang-cl preprocessor expands it into an empty string.Error File:
deps/oblib/src/rpc/frame/ob_req_packet_code.h:102Error Line
Impact Scope: All compilation units that directly or indirectly
#includeob_req_packet_code.h(approximately 200+.cppfiles). This only occurs when building on Windows; Linux/macOS are not affected.Passed Regressions
ob_geo_func_covered_by2.cpp.objand 15+ related geo files in the same include chain compiled successfully, with no furtherOPTIONAL-related syntax errors.#undef OPTIONAL, the enum expands correctly, and references toObReqCheckSumCheckLevel::OPTIONALresolve normally.zstd_objs.libandlz4-all.libentries inCMakeLists.txthave been confirmed to be covered byzstd_1_3_8_objs.lib; no additional linking is required.protoc.exe) cannot be executed due to CI sandbox restrictions. This is an existing environment issue and was not introduced by this change.Upgrade Compatibility
Other Information
DIMA: 2026061800116825248
Release Note