fix wasm cmake problems #591
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #591 +/- ##
=======================================
Coverage 91.41% 91.41%
=======================================
Files 35 35
Lines 4496 4496
=======================================
Hits 4110 4110
Misses 386 386 ☔ View full report in Codecov by Sentry. |
|
@t-paul @Loosetooth please let me know if this works for you. Will merge this after making sure it works on your machines. |
|
Did you successfully try this fix yourself? |
|
yes, did you check the emsdk version? |
|
I (think) I'm using the 'latest' emsdk version as specified in the readme: I made sure to fetch the latest emsdk data and update. Is this version the same as yours? |
|
|
Ah woops, sorry for reading over that. Thanks for pointing it out again. I can confirm that the WASM build now works on my local machine. |
|
Did you file a bug on emscripten? |
|
For what it's worth, on Windows, I'm getting Did the thrust setup change since then? Is this PR meant to fix that on Windows too? |
No I haven't, I can look into this later.
Yes the thrust setup changed, but the failure is weird because we do have CI tests on Windows... Maybe you can apply the following patch: diff --git a/src/utilities/CMakeLists.txt b/src/utilities/CMakeLists.txt
index a67f6af..b192a36 100644
--- a/src/utilities/CMakeLists.txt
+++ b/src/utilities/CMakeLists.txt
@@ -87,11 +87,13 @@ endif()
target_include_directories(${PROJECT_NAME} INTERFACE ${PROJECT_SOURCE_DIR}/include)
target_link_libraries(${PROJECT_NAME} INTERFACE glm::glm)
if(DEFINED thrust_SOURCE_DIR)
+message(thrust_SOURCE_DIR: ${thrust_SOURCE_DIR})
target_include_directories(${PROJECT_NAME} INTERFACE
${thrust_SOURCE_DIR}
${thrust_SOURCE_DIR}/dependencies/libcudacxx/include
)
else()
+ message(thrust_ROOT: ${thrust_ROOT})
target_include_directories(${PROJECT_NAME} INTERFACE
${THRUST_ROOT}
${THRUST_ROOT}/dependencies/libcudacxx/includerun cmake again and show me the output? |
|
@zalo I think I am just going to merge this for now, and you can open a separate issue to track the Windows build issue. |
* fix cmake problems * update emsdk version
We don't use thrust's CMake script as it can cause some weird problems on some machines, so just use
FetchContent_Populateinstead ofFetchContent_MakeAvailable. This also fix some issues about letter case, as the documentation states that<lowercaseName>_SOURCE_DIRthe name is in lowercase...Also adds emsdk version requirement. I found that the latest emsdk (3.1.48) has some issue and 3.1.45 works fine.