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

Update CMakeLists.txt to support Emscripten #3852

Closed
wants to merge 2 commits into from

Conversation

danoli3
Copy link

@danoli3 danoli3 commented Feb 15, 2024

Allow for compile target for WASM Emscripten.

Thanks FMT is wicked !

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Could you add a small section showing how to use this to the docs in https://github.com/fmtlib/fmt/blob/master/doc/usage.rst?

CMakeLists.txt Outdated
${FMT_SOURCES}
${FMT_HEADERS}
)
add_executable(fmt_wasm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it an executable and not a library.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emscripten requires executable code for wasm library for cmake is specified
https://stunlock.gg/posts/emscripten_with_cmake/

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Author

@danoli3 danoli3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated Emscripten target cmake

@vitaut
Copy link
Contributor

vitaut commented Feb 21, 2024

@danoli3, could you add a small usage section per my earlier comment?

@sykhro
Copy link

sykhro commented Feb 21, 2024

fmt already works on Emscripten without issues. What's the reason behind this PR?

@vitaut
Copy link
Contributor

vitaut commented Feb 21, 2024

I think @sykhro is right, at most you'd need to pass extra compile/link flags which can be done via CMake. We don't really need to introduce an extra target. But thanks for PR anyway.

@vitaut vitaut closed this Feb 21, 2024
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

Successfully merging this pull request may close these issues.

3 participants