Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The latest published Docker image uses a version of CMake that appears incompatible with sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$dynCall #1430

Open
kylerudy-imvu opened this issue Jul 25, 2024 · 8 comments

Comments

@kylerudy-imvu
Copy link

It appears a known issue (https://gitlab.kitware.com/cmake/cmake/-/issues/25049) makes it impossible or perhaps simply difficult to use sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$dynCall. The version of CMake published in the docker image does not escape command line parameters including $ in the same way as the most recent version of CMake.

Version of emscripten/emsdk:
I am using the latest pull of the emscripten/sdk docker image.

$ sudo docker run emscripten/emsdk emcc -v
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.64 (a1fe3902bf73a3802eae0357d273d0e37ea79898)
clang version 19.0.0git (https:/github.com/llvm/llvm-project 4d8e42ea6a89c73f90941fd1b6e899912e31dd34)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /emsdk/upstream/bin

Failing command line in full:
I am using the following commands to build

mkdir -p build
docker pull emscripten/emsdk
docker run -t -i --rm -v $(pwd):/src -u $(id -u):$(id -g) emscripten/emsdk emcmake cmake -S/src -B/src/build -DCMAKE_BUILD_TYPE=$1
docker run -t -i --rm -v $(pwd):/src -u $(id -u):$(id -g) emscripten/emsdk cmake --build /src/build --target Luau.LanguageServer.Web --config $1 --verbose

This results in a working build until the final step:

[100%] Linking CXX executable Luau.LanguageServer.Web.js
/usr/bin/cmake -E cmake_link_script CMakeFiles/Luau.LanguageServer.Web.dir/link.txt --verbose=1
/emsdk/upstream/emscripten/em++  -fexceptions -g -sENVIRONMENT=web "-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE='\$$dynCall'" -sEXPORTED_RUNTIME_METHODS=FS_createDataFile,FS_createPath -sMODULARIZE=1 -sEXPORT_NAME=LuauLanguageServerWeb -sEXPORT_ES6=1 --bind --no-entry -sIGNORE_MISSING_MAIN=1 -sALLOW_MEMORY_GROWTH=1 -sMINIMAL_RUNTIME=2 --extern-pre-js ../imvu/prepend.js @CMakeFiles/Luau.LanguageServer.Web.dir/objects1.rsp -o Luau.LanguageServer.Web.js @CMakeFiles/Luau.LanguageServer.Web.dir/linklibs.rsp
cache:INFO: generating system asset: symbol_lists/1e989c48d25d1cd794743c462e391ba7a85e8787.json... (this will be cached in "/emsdk/upstream/emscripten/cache/symbol_lists/1e989c48d25d1cd794743c462e391ba7a85e8787.json" for subsequent builds)
cache:INFO:  - ok
error: undefined symbol: $$dynCall (referenced by root reference (e.g. compiled C/C++ code))
warning: To disable errors for undefined symbols use `-sERROR_ON_UNDEFINED_SYMBOLS=0`
warning: $dynCall may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library
Error: Aborting compilation due to previous errors
em++: error: '/emsdk/node/18.20.3_64bit/bin/node /emsdk/upstream/emscripten/src/compiler.mjs /tmp/tmpq8pg_pwd.json' failed (returned 1)
gmake[3]: *** [CMakeFiles/Luau.LanguageServer.Web.dir/build.make:109: Luau.LanguageServer.Web.js] Error 1
gmake[3]: Leaving directory '/src/build'
gmake[2]: *** [CMakeFiles/Makefile2:258: CMakeFiles/Luau.LanguageServer.Web.dir/all] Error 2
gmake[2]: Leaving directory '/src/build'
gmake[1]: *** [CMakeFiles/Makefile2:265: CMakeFiles/Luau.LanguageServer.Web.dir/rule] Error 2
gmake[1]: Leaving directory '/src/build'
gmake: *** [Makefile:163: Luau.LanguageServer.Web] Error 2

Using a submodule approach for emsdk, this builds correctly. My infrastructure team has requested we use the docker image, and I'd like to do so.

My build script previously looked like:

mkdir -p build
emsdk/upstream/emscripten/emcmake cmake build -DCMAKE_BUILD_TYPE=$1
cmake --build build --target Luau.LanguageServer.Web --config $1

This produces working output. The difference appears to be due to the change in CMake versions.

$ cmake --version
cmake version 3.28.3

CMake suite maintained and supported by Kitware (kitware.com/cmake).
$ sudo docker run emscripten/emsdk cmake --version
cmake version 3.22.1

CMake suite maintained and supported by Kitware (kitware.com/cmake).

For reference, the problematic CMake line is:

target_link_options(Luau.LanguageServer.Web PRIVATE -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$dynCall)

I've tried several variants attempting to properly escape the command line parameter, but it appears there is no way to properly escape this command line parameter with CMake version 3.22.1.

@kylerudy-imvu
Copy link
Author

I've confirmed this is the issue with the following Dockerfile:

FROM emscripten/emsdk
RUN apt-get update
RUN apt-get install -y ca-certificates gpg wget
RUN test -f /usr/share/doc/kitware-archive-keyring/copyright || wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/null | gpg --dearmor - | sudo tee /usr/share/keyrings/kitware-archive-keyring.gpg >/dev/null
RUN echo 'deb [signed-by=/usr/share/keyrings/kitware-archive-keyring.gpg] https://apt.kitware.com/ubuntu/ jammy main' | sudo tee /etc/apt/sources.list.d/kitware.list >/dev/null
RUN apt-get update
RUN apt-get upgrade -y

With this amended Docker image, the verbose output looks like:

/usr/bin/cmake -E cmake_link_script CMakeFiles/Luau.LanguageServer.Web.dir/link.txt --verbose=1
/emsdk/upstream/emscripten/em++  -fexceptions -g "-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\$dynCall" -sENVIRONMENT=web -sEXPORTED_RUNTIME_METHODS=FS_createDataFile,FS_createPath -sMODULARIZE=1 -sEXPORT_NAME=LuauLanguageServerWeb -sEXPORT_ES6=1 --bind --no-entry -sIGNORE_MISSING_MAIN=1 -sALLOW_MEMORY_GROWTH=1 -sMINIMAL_RUNTIME=2 --extern-pre-js ../imvu/prepend.js @CMakeFiles/Luau.LanguageServer.Web.dir/objects1.rsp -o Luau.LanguageServer.Web.js @CMakeFiles/Luau.LanguageServer.Web.dir/linkLibs.rsp

With the "-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\$dynCall" command line parameter properly escaped, the process completes as expected and provides me with output similar--though not identical--to building locally. (It is worth noting that I only followed the first half of the instructions on https://apt.kitware.com/ when I tested this!)

@sbc100
Copy link
Collaborator

sbc100 commented Jul 31, 2024

Quoting in cmake files can indeed be tricky.

I'm fairly sure there is some way to make this work on cmake 3.22.1 but before we look into that perhaps you can simply remove the need for this command line flag: Where is the dynCall dependency coming from? If its coming from EM_JS/EM_ASM code then you can instead just include this line in the file alongside the EM_JS/EM_ASM: EM_JS_DEPS(deps, "$dynCall");

@kylerudy-imvu
Copy link
Author

The extent of my macro usage is EMSCRIPTEN_BINDINGS. I bind a factory function and expose one method for providing input for the class that results. The parameters for the factory function include an emscripten::val which expects a function for callbacks.

EMSCRIPTEN_BINDINGS(languageServerBindings) {
    emscripten::function("createWasmLanguageServer", &createWasmLanguageServer, emscripten::return_value_policy::take_ownership());
    
    emscripten::class_<LanguageServer>("LanguageServer")
            .smart_ptr<std::shared_ptr<LanguageServer>>("LanguageServer")
            .function("processInput", &LanguageServer::processInput);
}

I could experiment with EM_JS_DEPS, I suppose! I solved the issue by hosting a docker container that contains an updated CMake. (I threw in jq as well, for manipulating data files): https://hub.docker.com/repository/docker/krudyimvu/emscripten_updated

I imagine this issue will probably solve itself should someone migrate the docker image to noble, but I have not tested it. After using my workaround, I moved on.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 1, 2024

Can you explain why you need -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$dynCall in the first place? Where is this requirement coming from?

@kylerudy-imvu
Copy link
Author

kylerudy-imvu commented Aug 1, 2024

No, I cannot. I only know that my attempts to build spat out error messages that $dynCall was missing, when linking.

Edit: I feel I should add that the nature of the $dynCall dependency is irrelevant to the reported issue, which is that there is a patched bug in the version of CMake inside the docker image published by the project and referenced in the documentation. Thank you for commenting in pursuit of a workaround, but I am not actually blocked by this issue. I am reporting it to, hopefully, provoke an update to the published docker image for public consumption, and to provide some assistance / searchable reference for anyone caught by the same issue, should that update never materialize.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 2, 2024

I appreciate the reason for the explicit $dynCall is unrelated to the cmake issue. However, if you ever do have time to track down why its needed that would be great to be files as a separate issue. The reason is that if you are not directly, yourself, calling $dynCall then there should be no need to add it explicitly, and the fact that you need to points to bug in emscripten.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 2, 2024

The version of cmake we supply in the docker image is mostly dictated by the base ubuntu image that we use. I imagine that updating it would probably mean updating the base image.

@kleisauke
Copy link
Collaborator

Ubuntu 24.04 ships with CMake 3.28.3, so upgrading the base image to ubuntu:noble would fix this. See commit edc4dc5 for an example of how this was done previously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants