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

Enhancement of WASI_SDK_PREFIX in toolchain files #349

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

lum1n0us
Copy link
Contributor

No need to pass WASI_SDK_PREFIX value from the command line. Use CMAKE_CURRENT_LIST_DIR to assemble the value of WASI_SDK_PREFIX now.

@lum1n0us
Copy link
Contributor Author

#346

@@ -21,6 +21,11 @@ else()
set(WASI_HOST_EXE_SUFFIX "")
endif()

# When building from source, WASI_SDK_PREFIX represents the generated directory
if(NOT WASI_SDK_PREFIX)
set(WASI_SDK_PREFIX ${CMAKE_CURRENT_LIST_DIR}/../../)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use a tab for indentation like we already do above?

README.md Outdated
Use a toolchain file to setup the *wasi-sdk* platform.

```
$ cmake -DCMAKE_TOOLCHAIN_FILE=${wasi_sdk_home}/share/cmake/wasi-sdk.cmake -S . -B build
Copy link
Member

Choose a reason for hiding this comment

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

How about just cmake -DCMAKE_TOOLCHAIN_FILE=${wasi_sdk_home}/share/cmake/wasi-sdk.cmake ... .. All those other arguments are very project-specific.

Copy link
Member

Choose a reason for hiding this comment

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

Also, can you use WASI_SDK_PATH rather than wasi_sdk_home for consistency with the existing example code above?

No need to pass `WASI_SDK_PREFIX` value from the command line. Use
`CMAKE_CURRENT_LIST_DIR` to assemble the value of `WASI_SDK_PREFIX`
now.
@sbc100 sbc100 enabled auto-merge (squash) September 12, 2023 14:48
@sbc100 sbc100 merged commit 9b9708e into WebAssembly:main Sep 12, 2023
5 checks passed
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