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

threads: change wasm32-wasi-pthread to wasm32-wasi-threads #381

Merged
merged 2 commits into from
Jan 13, 2023

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented Jan 11, 2023

After some thought, I think that we should rename the THREAD_MODEL=posix build to avoid confusion. Why? Though in this project the use of this target does involve pthreads, it will not be so in other standard libraries or languages (see, e.g., rust-lang/compiler-team#574). I think it would be preferable to emphasize the "threads" Wasm-level proposal and the "wasi-threads" proposal rather than the specific details of which threading API is being exposed. I also think it would be good to settle on the name now before this gets exposed in wasi-sdk (WebAssembly/wasi-sdk#287).

@abrown abrown changed the title Change wasm32-wasi-pthread to wasm32-wasi-threads threads: change wasm32-wasi-pthread to wasm32-wasi-threads Jan 11, 2023
Copy link
Contributor

@loganek loganek left a comment

Choose a reason for hiding this comment

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

That makes sense to me. Was there any feedback from the Rust compiler about the name?

@abrown
Copy link
Collaborator Author

abrown commented Jan 12, 2023

No specific guidance like "we reject this name" but you can see from this comment and this discussion that the name could potentially be considered an issue. With this change I'm trying to avoid any such issues.

After some thought, I think that we should rename the `THREAD_MODEL=posix` build to avoid confusion. Why? Though in this project the use of this target does involve pthreads, it will not be so in other standard libraries or languages (see, e.g., rust-lang/compiler-team#574). I think it would be preferable to emphasize the "threads" Wasm-level proposal and the "wasi-threads" proposal rather than the specific details of which threading API is being exposed.
@abrown abrown force-pushed the wasm32-wasi-threads branch from b91939f to 3124868 Compare January 13, 2023 01:01
@abrown
Copy link
Collaborator Author

abrown commented Jan 13, 2023

@sbc100, @sunfishcode: any objections?

@abrown
Copy link
Collaborator Author

abrown commented Jan 13, 2023

cc: @penzn, @alexcrichton, @yamt

@sunfishcode sunfishcode self-requested a review January 13, 2023 01:04
@abrown abrown merged commit 4362b18 into main Jan 13, 2023
@abrown abrown deleted the wasm32-wasi-threads branch January 13, 2023 02:56
yamt added a commit to yamt/wasi-sdk that referenced this pull request Jan 13, 2023
yamt added a commit to yamt/toywasm that referenced this pull request Jan 20, 2023
sunfishcode pushed a commit to WebAssembly/wasi-sdk that referenced this pull request Jan 20, 2023
* Add a variation of wasi-sdk.cmake for pthread-using apps

* Install wasi-sdk-pthread.cmake

* Build wasi-libc with THREAD_MODEL=posix as well

* wasi-sdk-pthread.cmake: target rename to wasm32-wasi-threads

after WebAssembly/wasi-libc#381
loganek added a commit to loganek/wasm-micro-runtime that referenced this pull request Jan 24, 2023
The target was updated to wasm32-wasi-threads in wasi-libc (and most likely will stay like that for a while)
WebAssembly/wasi-libc#381
loganek added a commit to loganek/wasm-micro-runtime that referenced this pull request Jan 24, 2023
The target was updated to wasm32-wasi-threads in wasi-libc (and most likely will stay like that for a while)
WebAssembly/wasi-libc#381
john-sharratt pushed a commit to john-sharratt/wasix-libc that referenced this pull request Mar 6, 2023
…sembly#381)

* Change `wasm32-wasi-pthread` to `wasm32-wasi-threads`

After some thought, I think that we should rename the `THREAD_MODEL=posix` build to avoid confusion. Why? Though in this project the use of this target does involve pthreads, it will not be so in other standard libraries or languages (see, e.g., rust-lang/compiler-team#574). I think it would be preferable to emphasize the "threads" Wasm-level proposal and the "wasi-threads" proposal rather than the specific details of which threading API is being exposed.

* fix: rename the `expected` output directory as well
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.

4 participants