Emscripten toolchain should point to cache sys root

[pezcode]

I mentioned this on gitter a while back but these things tend to disappear in the mist of time, so here's a trackable issue 🍉

Corrade's Emscripten toolchain file sets CMAKE_FIND_ROOT_PATH to upstream/emscripten/system. In contrast, Emscripten's own toolchain file sets it to the cache sys root (defaults to upstream/emscripten/cache/sysroot but is configurable). The latter contains generated headers like version.h added in 3.1.4 and included directly by emscripten.h.

This breaks when you link to a library whose include path is the root include directory (like EGL) and then directly include emscripten.h. In that case, the Corrade CMAKE_FIND_ROOT_PATH has priority over the compiler's default include path, and while emscripten.h is found there, compilation fails because it now can't find version.h.

Repro: add #include <emscripten.h> to TextureGLTest.cpp on Emscripten 3.1.4 and up.

There's a rather lengthy gitter thread with more detail here.

Edit: forgot to add the patch we've been using, been working fine so far:

diff --git a/generic/Emscripten-wasm.cmake b/generic/Emscripten-wasm.cmake
index 7b6865dcee27b73cea9e2849231f5f24bb541a79..aac9d9c2c22956efd861ceb769ede19d452508f4 100644
--- a/generic/Emscripten-wasm.cmake
+++ b/generic/Emscripten-wasm.cmake
@@ -40,8 +40,6 @@ if(NOT EMSCRIPTEN_PREFIX)
     endif()
 endif()
 
-set(EMSCRIPTEN_TOOLCHAIN_PATH "${EMSCRIPTEN_PREFIX}/system")
-
 if(CMAKE_HOST_WIN32)
     set(EMCC_SUFFIX ".bat")
 else()
@@ -58,8 +56,24 @@ set(CMAKE_CXX_COMPILER "${EMSCRIPTEN_PREFIX}/em++${EMCC_SUFFIX}")
 set(CMAKE_AR "${EMSCRIPTEN_PREFIX}/emar${EMCC_SUFFIX}" CACHE PATH "Path to Emscripten ar")
 set(CMAKE_RANLIB "${EMSCRIPTEN_PREFIX}/emranlib${EMCC_SUFFIX}" CACHE PATH "Path to Emscripten ranlib")
 
+# The root path should point to the cache sysroot instead of
+# ${EMSCRIPTEN_PREFIX}/system. Otherwise, find modules may end up finding
+# include dirs in ${EMSCRIPTEN_PREFIX}/system, which is missing some generated
+# headers (e.g. version.h since 3.1.4). For example, FindEGL finds and sets the
+# root path as its include dir, and anything that links to it and includes
+# emscripten.h will fail to compile, because emscripten.h can't find version.h.
+# Taken from https://github.com/emscripten-core/emscripten/blob/74d6a15644e7f6e76ed6a1da9c6937b5cb7aef6e/cmake/Modules/Platform/Emscripten.cmake#L223
+execute_process(COMMAND "${EMSCRIPTEN_PREFIX}/em-config${EMCC_SUFFIX}" "CACHE"
+  RESULT_VARIABLE _emcache_result
+  OUTPUT_VARIABLE _emcache_output
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
+if (NOT _emcache_result EQUAL 0)
+  message(FATAL_ERROR "Failed to find emscripten cache directory with command \"'${EMSCRIPTEN_PREFIX}/em-config${EMCC_SUFFIX}' CACHE\"! Process returned with error code ${_emcache_result}.")
+endif()
+set(EMSCRIPTEN_SYSROOT "${_emcache_output}/sysroot")
+
 set(CMAKE_FIND_ROOT_PATH ${CMAKE_FIND_ROOT_PATH}
-    "${EMSCRIPTEN_TOOLCHAIN_PATH}"
+    "${EMSCRIPTEN_SYSROOT}"
     "${EMSCRIPTEN_PREFIX}")
 
 set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)

[mosra]

Ah, yes, thanks for the reminder.

There's still the problem with headers/libraries of 3rd party projects installed into Emscripten sysroot getting stale that I didn't have time to investigate further, which is why I didn't merge this patch yet. But, just yesterday, while working on the cursed issues from mosra/magnum#560 I was browsing Emscripten changelog and found this in 2.0.24:

CMake projects (those that either use emcmake or use Emscripten.cmake directly) are new configured to install (by default) directly into the emscripten sysroot. This means that running cmake --install (or running the install target, via make install for example) will install resources into the sysroot such that they can later be found and used by find_path, find_file, find_package, etc. Previously the default was to attempt to install into the host system (e.g /usr/local) which is almost always not desirable. Folks that were previously using CMAKE_INSTALL_PREFIX to build their own secondary sysroot may be able to simplify their build system by removing this completely and relying on the new default.

Yes, we're not using the upstream toolchain, so it doesn't affect us, but what I consider important about this note is that upstream treats installing into Emscripten's own sysroot as the recommended practice. Which means that either they should hopefully know about this potential "stale" issue and having a builtin solution for it (that I'm not aware of yet), or the toolchain handles that somehow (and because we don't use it, we don't have that handled), or there might at least be some issues open about this. I'll try to look back into this when I get time.

[pezcode]

After applying this change, you might run into issues with implicit C headers being included by the C++ compiler. Those headers are not in system/include, but they are in cache/sysroot/include.

The fix can be found in https://github.com/emscripten-core/emscripten/pull/17137, the important part:

if("${CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES}" STREQUAL "")
  set(CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES "${EMSCRIPTEN_SYSROOT}/include")
endif()
if("${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES}" STREQUAL "")
  set(CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "${EMSCRIPTEN_SYSROOT}/include")
endif()

This makes sure that explicitly adding the cache sysroot include doesn't have preference over the implicit C++ include directory.

[mosra]

Closing as resolved, forgot this issue was even here.

Just for the record, the first patch was integrated with various version-dependent modification as mosra/toolchains@d5d74307fd81c84596b3575c9692681cc61952c2, and the toolchains submodule was updated in 45282f88193be2847fc98f2c44d60a4e956b9af8. The second patch seems to be not needed -- looking into CMake's CMakeFiles/<version>/CMakeCXXCompiler.cmake in the build dir, the CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES already contains it, together with others. I suppose CMake extracts it via emcc -E -v - as it already does for other compilers.

This is for example with CMake 3.17 and Emscripten 2.0.25 on the CI, the sysroot include dir is last:

set(CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "/emsdk/upstream/emscripten/cache/sysroot/include/SDL;/emsdk/upstream/emscripten/cache/sysroot/include/compat;/emsdk/upstream/emscripten/cache/sysroot/include/c++/v1;/emsdk/upstream/lib/clang/13.0.0/include;/emsdk/upstream/emscripten/cache/sysroot/include")

[mosra]

Eh, in the end I actually did hit a similar problem, although in my case I had to add the non-cache include dir there: https://github.com/mosra/toolchains/commit/6a9e0821b4ccd578feee1f30cef245cbd36cdd38

Because FindZstd installed into the non-cache location (because otherwise it wouldn't be able to locate its libraries, because THE DAMN THING copies only contents of include/ but not lib/, EXCEPT FOR lib/cmake/, so I can't even put the CMake configs into lib/cmake/ but have to move them into share/cmake/ instead, as otherwise again it would be copied together with its includes but not libraries) otherwise added the non-cache include dir into the include path, resulting in an eventual error due to including the wrong doomed-to-fail <emscripten/version.h>.

I hate everything about this platform. Everything.