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

gh-101538: Add experimental wasi-threads build #101537

Merged
merged 9 commits into from
Jun 22, 2023
Merged

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Feb 3, 2023

eg.

export WASI_SDK_PATH=/opt/wasi-sdk-19.5g0236e959edbc
./Tools/wasm/wasm_build.py wasi-threads build
~/git/toywasm/b.thread/toywasm --wasi --wasi-dir . --wasi-dir ~/git/garbage/py/th2 -- builddir/wasi-threads/python.wasm ~/git/garbage/py/th2/thread.py
  • I used an unreleased version of wasi-sdk

  • I used toywasm to test because wasmtime doesn't have
    necessary functionality yet.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Feb 3, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@arhadthedev arhadthedev added interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-wasi labels Feb 3, 2023
@arhadthedev
Copy link
Member

Would you mind adding the news entry as the bot suggests? Add experimental wasi-threads support. Patch by Takashi Yamamoto. is sufficient.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@yamt yamt changed the title Add experimental wasi-threads build gh-101538: Add experimental wasi-threads build Feb 3, 2023
eg.
```
autoconf
export WASI_SDK_PATH=/opt/wasi-sdk-19.5g0236e959edbc
./Tools/wasm/wasm_build.py wasi-threads build
~/git/toywasm/b.thread/toywasm --wasi --wasi-dir . --wasi-dir ~/git/garbage/py/th2 -- builddir/wasi-threads/python.wasm ~/git/garbage/py/th2/thread.py
```

* I used an unreleased version of wasi-sdk

* I used toywasm to test because wasmtime doesn't have
  necessary functionality yet.
@yamt
Copy link
Contributor Author

yamt commented Feb 3, 2023

Would you mind adding the news entry as the bot suggests? Add experimental wasi-threads support. Patch by Takashi Yamamoto. is sufficient.

thank you. done.

```
docker run --rm -v $(pwd):/src quay.io/tiran/cpython_autoconf:269
```
@arhadthedev
Copy link
Member

No need to rebase and force-push; everything will be squashed into a single commit while merging anyway.

@yamt
Copy link
Contributor Author

yamt commented Feb 3, 2023

No need to rebase and force-push; everything will be squashed into a single commit while merging anyway.

ok. i was not aware of the policy. thank you.

@erlend-aasland
Copy link
Contributor

ok. i was not aware of the policy. thank you.

It is documented in the devguide: https://devguide.python.org/getting-started/pull-request-lifecycle/#submitting

configure.ac Outdated Show resolved Hide resolved
@yamt
Copy link
Contributor Author

yamt commented Feb 6, 2023

ok. i was not aware of the policy. thank you.

It is documented in the devguide: https://devguide.python.org/getting-started/pull-request-lifecycle/#submitting

ok. i was aware of the doc. but apparently i haven't read it carefully enough. thank you for pointing out.

@brettcannon
Copy link
Member

As I rejected the feature request I am also unfortunately rejecting this PR.

@brettcannon
Copy link
Member

FYI in case this PR gets re-opened (instead of a new one) when WASI threads goes stable, I was able to build with WASI SDK 20 and this PR.

@brettcannon brettcannon reopened this Jun 12, 2023
@brettcannon
Copy link
Member

@yamt I got tacit approval to turn this on as long as it is clearly marked as experimental. Did you want to refresh this PR as necessary?

@erlend-aasland
Copy link
Contributor

@yamt, I marked our Autoconf conversation as resolved and also took the liberty to apply a small Autoconf style change. Hope you don't mind.

@yamt
Copy link
Contributor Author

yamt commented Jun 21, 2023

@yamt, I marked our Autoconf conversation as resolved and also took the liberty to apply a small Autoconf style change. Hope you don't mind.

thank you.

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Jun 22, 2023

AFAICS, we can land this now, @brettcannon. I'll wait a couple of days to give you a chance to look over it before merging :)

@brettcannon
Copy link
Member

It's experimental (and even undocumented ATM 😅), so I'm good with it.

@brettcannon brettcannon merged commit d8f87cd into python:main Jun 22, 2023
@brettcannon
Copy link
Member

@yamt Thanks! I will update https://github.com/brettcannon/cpython-wasi-build probably in the next week or so to start producing a threaded build for WASI on top of the "normal" build w/o threads.

bentasker pushed a commit to bentasker/cpython that referenced this pull request Jun 23, 2023
@brettcannon
Copy link
Member

@Yhg1s is it okay to backport this to 3.12?

@Yhg1s
Copy link
Member

Yhg1s commented Jul 17, 2023

Yes, this is fine for 3.12 (if it gets into RC1, scheduled for 2 weeks from now).

@erlend-aasland erlend-aasland added the needs backport to 3.12 bug and security fixes label Jul 17, 2023
@miss-islington
Copy link
Contributor

Thanks @yamt for the PR, and @brettcannon for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-106834 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Jul 17, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 17, 2023
(cherry picked from commit d8f87cd)

Co-authored-by: YAMAMOTO Takashi <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
erlend-aasland added a commit that referenced this pull request Jul 17, 2023
…06834)

(cherry picked from commit d8f87cd)

Co-authored-by: YAMAMOTO Takashi <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-wasi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants