feat: add support for running wasm target in browser#7293
feat: add support for running wasm target in browser#7293waruqi merged 4 commits intoxmake-io:devfrom
Conversation
Summary of ChangesHello @jinke18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for running wasm targets in the browser by starting a Python http.server. The implementation is a good starting point. My review includes a suggestion to improve the security and robustness of the server by explicitly binding to localhost and specifying the port, which also opens the door for making the port configurable in the future.
xmake/actions/run/main.lua
Outdated
| local url = "http://localhost:8000/" .. path.relative(targetfile, rundir):gsub("\\", "/") | ||
| print("please open the url in browser") | ||
| cprint("${color.success}%s${clear}", url) | ||
| os.execv(python.program, {"-m", "http.server"}, {curdir = rundir, detach = option.get("detach"), addenvs = addenvs, setenvs = setenvs}) |
There was a problem hiding this comment.
Python's http.server binds to 0.0.0.0 by default, which exposes the server to your local network. This could be a security risk, especially on untrusted networks. It's safer to explicitly bind to 127.0.0.1 (localhost).
Additionally, while http.server defaults to port 8000, making it explicit in the command improves clarity. As a future enhancement, you could make this port configurable (e.g. via option.get("port")).
os.execv(python.program, {"-m", "http.server", "--bind", "127.0.0.1", "8000"}, {curdir = rundir, detach = option.get("detach"), addenvs = addenvs, setenvs = setenvs})
|
Hello, It would be a better idea to rely on the official emscripten test webserver Plus it opens the browser automatically |
|
emrun is used to execute WebAssembly targets, with Python serving as a fallback. @SirLynix |
xmake/actions/run/main.lua
Outdated
| -- run wasm target in browser | ||
| if target:is_plat("wasm") then | ||
| -- try to run with emrun | ||
| local emrun = find_tool("emrun") |
There was a problem hiding this comment.
I tried it, it is not found on windows.
It could be emrun.bat, emrun.ps1, emrun.py, or the emrun shell script. Different platforms should use different script files.
Additionally, it is located in the upstream/emscripten subdirectory.
There was a problem hiding this comment.
在不同系统下检测emrun的逻辑在find_emrun.lua里面处理了,与find_emcc.lua逻辑一致,应该问题不大。
如果需要改进,是不是同时需要更改find_emcc.lua和find_emar.lua?
The logic for detecting emrun in different systems has been processed in find_ emrun. lua, which is consistent with the logic of find_ emcc. lua, so there should be no problem.
If improvement is needed, do we need to change both find_ emcc. lua and find_ emar. lua at the same time?
There was a problem hiding this comment.
查找问题是 find emsdk 导致,已经修了,得 rebase 下
|
please rebase to dev |
add support for running wasm target in browser