Conversation
|
What was the used cmake configuration command? Can you paste that along with the error from ctest |
Used CMake configure command: cmake -DBUILD_SHARED_LIBS=ON -DCPPINTEROP_USE_CLING=ON -DCPPINTEROP_USE_REPL=Off -DCling_DIR=$LLVM_DIR/build/tools/cling -DLLVM_DIR=$LLVM_DIR/build/lib/cmake/llvm -DClang_DIR=$LLVM_DIR/build/lib/cmake/clang -DCMAKE_INSTALL_PREFIX=$CPPINTEROP_DIR ..CTest/check target error before the fix: Full tail from the failing run: |
You're missing a build type. What this boils down to is the fact that CppInterOp doesn't spell out a default build type when not provided. |
aaronj0
left a comment
aaronj0
left a comment
There was a problem hiding this comment.
What we need is something like https://github.com/llvm/llvm-project/blob/5ff5a1f14761ecc7160125aadb38c59d29f58e5c/llvm/CMakeLists.txt#L81-L87
unittests/CMakeLists.txt
Outdated
| add_custom_target(check-cppinterop COMMAND ${CMAKE_CTEST_COMMAND} -V --build-config $<CONFIG> | ||
| DEPENDS CppInterOpUnitTests WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) | ||
| # macOS single-configuration generators produce an empty $<CONFIG>, so omit | ||
| # --build-config there. Keep existing behavior on other platforms. |
There was a problem hiding this comment.
I don't understand the point of duplicating the command without $<CONFIG>, that's a hacky solution.
Can you try patching the CMake with the suggestion above and see if that solves your build?
There was a problem hiding this comment.
Hi,
I’ve updated the PR to follow the LLVM-style approach instead of duplicating the check-cppinterop command
unittests/CMakeLists.txt
Outdated
| add_subdirectory(CppInterOp) | ||
|
|
||
| add_custom_target(check-cppinterop COMMAND ${CMAKE_CTEST_COMMAND} -V --build-config $<CONFIG> | ||
| if(CMAKE_CONFIGURATION_TYPES) |
There was a problem hiding this comment.
This shouldn't be required
cmake/modules/GoogleTest.cmake
Outdated
| else() | ||
| set(config_cmd ${CMAKE_COMMAND}) | ||
| set(build_cmd ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}/unittests/googletest-prefix/src/googletest-build/ --config $<CONFIG>) | ||
| # Only add --config for multi-configuration generators (Visual Studio, Xcode) |
There was a problem hiding this comment.
Please drop these changes. CMake will propagate the right config since you set the default build type
|
Can you make this a single commit with a more useful/correct commit message? something like "Set default build type if not provided" |
tryh4rd-26
force-pushed
the
fix-macos-cmake-build
branch
from
9c73e11 to
c0f4a18
Compare
@aaronj0 made changes as requested |
aaronj0
left a comment
aaronj0
left a comment
There was a problem hiding this comment.
I'd not want to point to LLVM's cmake options for CppInterOp
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #829 +/- ##
=======================================
Coverage 79.56% 79.56%
=======================================
Files 11 11
Lines 4013 4013
=======================================
Hits 3193 3193
Misses 820 820 🚀 New features to boost your workflow:
|
tryh4rd-26
force-pushed
the
fix-macos-cmake-build
branch
from
6eac892 to
7ef721a
Compare
|
@aaronj0 |
2 participants
Fixed macOS single-config CMake test invocation by avoiding an empty config value in the check-cppinterop path.
On macOS with Unix Makefiles/Ninja, $ can resolve to empty, which breaks CTest when passed via --build-config; the target now uses the non-config form in that case.
Behavior for multi-config generators remains unchanged.
Validation: cmake --build build --target check-cppinterop --parallel 4 → 100% tests passed (3/3), 0 failed.