CMake Toolkit: Shared Library as Side Module

[ax3l]

Enable support to build shared libraries as side modules. This should simplify builds with CMake that currently have to reside to manual link steps.

X-ref:

• SHARED / dynamic library with CMake #15276 (comment)
• dynamic linking fails with cmake #16157
• Set TARGET_SUPPORTS_SHARED_LIBS in cmake config #8362 cc @sbc100 @kripken
• Pkg: openPMD-api pyodide/pyodide#2169

[kripken]

Thanks @ax3l !

I don't know much about CMake, but the flags seem right.

However there are test failures here. Let us know if you need help debugging those.

[ax3l]

Thanks a lot! Let's see if keeping the default in emscripten to "static" already solves this.

If you have pointers where we can add a new test for building a shared library, then I would add that to the PR :)

[ax3l]

I cannot see why the tests are failing, is there an easy way to increase verbosity in CI or run them locally?

[kripken]

Running them locally should be easy, see the docs which are here: https://emscripten.org/docs/getting_started/test-suite.html

For example, ./tests/runner other.test_cmake_js will run one of the failures.

For more details you can read the test code in tests/test_other.py (search for that test name) and see what it is doing.

[ax3l]

Related upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/23237
I would like to allow building shared libs, but keep defaulting to static libs.

[ax3l]

I see when executing test_cmake_js that some symbols cannot be found. (This one builds a shared library now.)

['/home/axel/src/emscripten/emcmake', 'cmake', '-DCMAKE_BUILD_TYPE=Debug', '-DCMAKE_VERBOSE_MAKEFILE=ON', '-G', 'Unix Makefiles', '/home/axel/src/emscripten/tests/cmake/target_js']
em++: warning: ignoring unsupported linker flag: `-soname` [-Wlinkflags]
em++: warning: please replace -g4 with -gsource-map [-Wdeprecated]
em++: warning: ignoring unsupported linker flag: `-rpath` [-Wlinkflags]
error: undefined symbol: _Z20cpp_library_functionv (referenced by top-level compiled C/C++ code)
warning: Link with `-s LLD_REPORT_UNDEFINED` to get more information on undefined symbols
warning: To disable errors for undefined symbols use `-s ERROR_ON_UNDEFINED_SYMBOLS=0`
warning: __Z20cpp_library_functionv may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library
Error: Aborting compilation due to previous errors

I am not sure why, is there another flag we should add to compile/link of shared libs? :) -sEXPORT_ALL?

I also tried -sEXPORTED_FUNCTIONS=_Z20cpp_library_functionv -sEXPORT_ALL without success on the cpp_lib/lib.cpp test target.

[sbc100]

Nice! If we can make the default of BUILD_SHARED_LIBS=OFF work I would support this change.

[sbc100]

Why is this needed if the default is set of OFF above?

[ax3l]

I wonder as well. Without it the test_openjpeg does not seem to build

libopenjpeg/CMakeFiles/openjpeg.dir/build.make:388: bin/libopenjpeg.so.1.4.0

Is the toolchain file even used here?

[ax3l]

Ah, it's because ./tests/third_party/openjpeg/CMakeLists.txt lists:

# OpenJPEG build configuration options.
OPTION(BUILD_SHARED_LIBS "Build OpenJPEG shared library and link executables against it." ON)

Before this change, this was force-changed to static (in add_library) by the global property TARGET_SUPPORTS_SHARED_LIBS FALSE.

[sbc100]

I see.. I'm ok with the explicit extra -DBUILD_SHARED_LIBS=OFF here then . And yes the emcmake toolchain is used for all our cmake builds.

[ax3l]

not really needed, default is also static.

Suggested change

set(BUILD_SHARED_LIBS OFF) # default to static libs, even if we allow shared ones

[nokotan]

Our goal should be this table.

cmake	user option	library type
add_library(foo)		static
add_library(foo)	-DBUILD_SHARED_LIBS=ON	dynamic
add_library(foo SHARED)		dynamic
add_library(foo) option(BUILD_SHARED_LIBS "" ON)		static
add_library(foo) option(BUILD_SHARED_LIBS "" ON)	-DBUILD_SHARED_LIBS=ON	dynamic

The most important thing will be that option(BUILD_SHARED_LIBS "" ON) should be overridden to OFF for compatibility unless BUILD_SHARED_LIBS=ON is explicitly specified.

Just adding configure=['cmake', '.', '-DBUILD_SHARED_LIBS=OFF'] will be sufficient for this and set(BUILD_SHARED_LIBS OFF) can be deleted.
The other solution will be that caching BUILD_SHARED_LIBS, this will overwrite option(BUILD_SHARED_LIBS "" ON).

Suggested change

	set(BUILD_SHARED_LIBS OFF) # default to static libs, even if we allow shared ones
	set(BUILD_SHARED_LIBS OFF CACHE BOOL "Build WebAssembly dynamic libraries. Default value is 'OFF'.") # default to static libs, even if we allow shared ones

[ax3l]

Hi @nokotan,

what you write in this table is very reasonable, but there is a misunderstanding on your side how CMake works.

Yes, add_library defaults will be based on the BUILD_SHARED_LIBS default. That one is static now.

No, if a user sets either set(BUILD_SHARED_LIBS ON) or option(BUILD_SHARED_LIBS "" ON) this will be overwritten. This is a clear user decision then (in their CMakeLists.txt) and we cannot overwrite this without disabling shared library support altogether (which was done prior to this PR). In other worlds: the 2nd last row in your table is not possible.
All other rows are exactly with this PR as you also wrote.

I hope this clarifies a bit, please let me know if you have more questions.

[nokotan]

The question remains, option(BUILD_SHARED_LIBS "" ON) seems to do nothing when BUILD_SHARED_LIBS is set by user or is already cached, after CMake 3.13. (CMP0077)

[ax3l]

That is exactly how all cached variables and options work in CMake.

Yes, user CLI parameters, toolchain files and cache files can be used by users to overwrite the default of an option() - that is fine and how all CMake options should work. If we would mingle with that behavior, then we would break a lot of package management software and stray from community standards.

[ax3l]

Nice! If we can make the default of BUILD_SHARED_LIBS=OFF work I would support this change.

Thanks!
Turns out, that works :)

It's only a few libs in the tests where we explicitly do shared builds that fail now. Before this, the property TARGET_SUPPORTS_SHARED_LIBS FALSE overwrote add_library(... SHARED) calls to be static builds.

We need to decide for the failing tests if we declare the exports or keep them to static.

[ax3l]

For test_dlfcn_self (test_core.core0) I am not sure why __data_end is not exported now 🤔

[sbc100]

For test_dlfcn_self (test_core.core0) I am not sure why __data_end is not exported now thinking

A rebase should fix that issue I think.

[ax3l]

Thx, rebased :)

I see this in my PR for tests:

1 workflow awaiting approval
First-time contributors need a maintainer to approve running workflows. [Learn more.](https://docs.github.com/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks) 12 expected, 2 pending, and 4 successful checks

Can someone please approve me? :)

[sbc100]

Whats the deal with CMAKE_STRIP FALSE... if that doesn't work can you open a bug report.. and leave a comment in the place where we disable this?

[ax3l]

The CMAKE_STRIP issue I saw in pyodide/pyodide#2169 when building a pybind11 extension:
https://github.com/pybind/pybind11/blob/v2.9.1/tools/pybind11Common.cmake#L399-L411

Simply did not work with what it picked for CMAKE_STRIP. Do you have your own strip command that needs to be used instead here?

[sbc100]

The CMAKE_STRIP issue I saw in pyodide/pyodide#2169 when building a pybind11 extension: https://github.com/pybind/pybind11/blob/v2.9.1/tools/pybind11Common.cmake#L399-L411

Simply did not work with what it picked for CMAKE_STRIP. Do you have your own strip command that needs to be used instead here?

Is the CMAKE_STRIP separate/unrelated to this one? If so, can leave it out of this change for now?

llvm-strip is supposed to work and I believe it does, but I'll need to take a look at the specific issue here.

[ax3l]

Oh, it definitely did not pick that one. Let's set it here. Let me try a path.

Update: uses now emstrip - thank you for #16431 💖

[anutosh491]

Hi, I have been using these flags for my project separately but it would also be great if support to build shared libraries as side modules is supported from emscripten's side. Is some movement expected here to maybe finish of the pending work if any ?