emscripten: Duplicate defined webgl symbols

[Squareys]

The FindGLES3.cmake links the GL library and for CMake >= 3.13 also adds -s USE_WEBGL2=1.
Doing both leads to duplicate defined GL symbols, removing the INTERFACE_LINK_LIBRARIES property fixes the issue.

See also emscripten-core/emscripten#14622.

@mosra Any preference on how this should be fixed? i.e. Remove the link library or the link option?

[mosra]

Wait, what? So you're saying -lGL -s USE_WEBGL2=1 causes ... what actually? I suspect the problem is somewhere else, this has always worked.

The -s USE_WEBGL2=1 flag is added for 3.13+, yes, but as the comment says, for versions before the same is done in FindMagnum.cmake, so what you're doing in that commit is not really fixing anything:

magnum/modules/FindMagnum.cmake

Lines 1035 to 1051 in 527ee0f

	# If we are on CMake 3.13 and up, `-s USE_WEBGL2=1` linker option is
	# propagated from FindOpenGLES3.cmake already. If not (and the GL library
	# is used), we need to modify the global CMAKE_EXE_LINKER_FLAGS. Do it here
	# instead of in FindOpenGLES3.cmake so it works also for CMake subprojects
	# (in which case find_package(OpenGLES3) is called in (and so
	# CMAKE_EXE_LINKER_FLAGS would be modified in) Magnum's root CMakeLists.txt
	# and thus can't affect the variable in the outer project). CMake supports
	# IN_LIST as an operator since 3.1 (Emscripten needs at least 3.7), but
	# it's behind a policy, so enable that one as well.
	cmake_policy(SET CMP0057 NEW)
	# TODO since 1.39.19 it's possible to use `-sUSE_WEBGL2=1`, which can be
	# then passed via target_link_libraries() etc. without requiring CMake
	# 3.13: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md#13919-07072020
	# -- change to that once we drop support for older Emscripten versions
	if(CMAKE_VERSION VERSION_LESS 3.13 AND GL IN_LIST Magnum_FIND_COMPONENTS AND NOT MAGNUM_TARGET_GLES2 AND NOT CMAKE_EXE_LINKER_FLAGS MATCHES "-s USE_WEBGL2=1")
	set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -s USE_WEBGL2=1")
	endif()

The -lGL is there because it's required for the minimal Emscripten runtime -- see fa50929. Reverting that means MINIMAL_RUNTIME gets broken again, so I don't think this is something I should be removing.

The referenced Emscripten issue is a year old and seems to point to something inside Emscripten's own CMake toolchain, which we don't use for various reasons, so I don't think that's related. What linker errors are you actually getting? Can you paste them here? Ideally, including the whole linker command line as shown by ninja -v.

[Squareys]

As you know we had some issues with our CMake toolchain setup, so this might resolve once we resolve them. I'll report back once we figured that out.
Either way, emscripten figures out the -l flags itself, so -lGL should not be required, it's implicitly added via -sUSE_WEBGL2=1.

[mosra]

emscripten figures out the -l flags itself, so -lGL should not be required

Not the case for MINIMAL_RUNTIME, as I said above. There it needs to be explicit, and that's the whole reason for the flag to be present.

I'll wait until you report back, but I think the issue was something with the toolchain setup rather than the presence of -lGL being a problem.

[mosra]

Bump -- is any of this still relevant? As I said above, I don't think this has anything to do with Magnum or the toolchain, but rather seems to be some particularly nasty case of a broken setup 😅

Cc: @pezcode

[mosra]

Did some more investigation. Not reproducible on Emscripten 3.1.44 in PlatformEmscriptenApplicationTest with -s MINIMAL_RUNTIME=2 -s USE_WEBGL2=1 -lGL. On the other omitting -lGL leads to undefined symbol: emscripten_webgl_init_context_attributes with MINIMAL_RUNTIME being used, so the flag is clearly needed.