samiandClaude Sonnet 5
e8a6b64c2f
tests/conformance/cpp/CMakeLists.txt (PROTO's lane) compiles core/generated/velox_proto.cpp directly while its comment claims it links libveloxproto. Now that the veloxproto target exists it should link velox::proto, with a TARGET-guarded fallback to the direct-compile for standalone configures. Filed, not edited — not CORE's file. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
3.0 KiB
CORE → PROTO — tests/conformance/cpp/CMakeLists.txt should link veloxproto now
Status: open. Small, mechanical. Filed rather than fixed because tests/conformance/
is PROTO's lane.
What's stale
tests/conformance/cpp/CMakeLists.txt says in its header comment:
Links libveloxproto (the generated protocol code in core/generated/), not libveloxcore
…but it actually compiles core/generated/velox_proto.cpp straight into the
executable and finds nlohmann_json itself:
add_executable(velox_conformance_cpp
conformance_main.cpp
${CMAKE_SOURCE_DIR}/core/generated/velox_proto.cpp)
target_include_directories(velox_conformance_cpp PRIVATE
${CMAKE_SOURCE_DIR}/core/generated
${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(velox_conformance_cpp PRIVATE nlohmann_json::nlohmann_json)
That was the only option while ADR 0009's libveloxproto target didn't exist. It exists
now — core/CMakeLists.txt defines veloxproto / velox::proto (commit adding it on
lane/core), with core/generated/ as a PUBLIC include dir and nlohmann_json linked
PUBLIC. The comment and the code now agree only if the runner links the target.
Requested change
if(TARGET velox::proto)
add_executable(velox_conformance_cpp conformance_main.cpp)
target_link_libraries(velox_conformance_cpp PRIVATE velox::proto)
else()
# Standalone configure of tests/conformance/ (no core/ in the tree): fall back to
# compiling the generated source directly, as today.
if(NOT TARGET nlohmann_json::nlohmann_json)
find_package(nlohmann_json 3.11 REQUIRED)
endif()
add_executable(velox_conformance_cpp
conformance_main.cpp
${CMAKE_SOURCE_DIR}/core/generated/velox_proto.cpp)
target_include_directories(velox_conformance_cpp PRIVATE
${CMAKE_SOURCE_DIR}/core/generated)
target_link_libraries(velox_conformance_cpp PRIVATE nlohmann_json::nlohmann_json)
endif()
target_include_directories(velox_conformance_cpp PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_compile_features(velox_conformance_cpp PRIVATE cxx_std_23)
add_test(NAME conformance_cpp COMMAND velox_conformance_cpp ${CMAKE_SOURCE_DIR})
set_tests_properties(conformance_cpp PROPERTIES LABELS "conformance")
The if(TARGET ...) branch keeps the suite configurable on its own (the property the
current comment says it wants) while using the real library in the normal full-tree build.
The root CMake already add_subdirectory(core)s before tests/conformance, so the target
is present in that path.
Why it matters beyond tidiness
GUI is blocked on libveloxproto being a real link target (it can't add_subdirectory a
sibling lane's core/generated/ and re-guess the nlohmann find). Once GUI links
velox::proto, the conformance runner linking the same target is what guarantees the
GUI and the conformance suite are exercising byte-identical generated code — compiling the
.cpp twice into two executables with two different warning/flag sets is exactly the kind
of skew a conformance suite exists to catch.