wasm-micro-runtime
wasm-micro-runtime copied to clipboard
bytecodealliance
Merge dev/socket into main
@loganek Are there any PRs to submit to dev/socket recently? Shall we merge the changes from dev/socket into main firstly, since we are planning to release a new version of WAMR. Wandering whether it is a big impact to current socket api user? @lum1n0us What is your opinion?
We'll definitely have more PRs for sockets, but none of the changes we have in the branch are breaking WAMR so I think it's safe to merge. Having said that, please don't remove the dev/socket branch as we'll be sending few more PRs (for ipv6, some of the socket properties, add getaddrinfo implementation in socket libc etc.)
Tend to keep all modifications on dev/socket until all PRs and features(ipv6, socket properties, and so on) are ready.
Some of the changes really impact the current wasm app, for example, the implementation of wasi_sock_ext is changed, which will be linked into wasm app, and the signatures of sock_addr_local/sock_addr_remote/sock_addr_resolve are also changed. These mean that the old wasm app cannot correctly run in the current runtime. @loganek @lum1n0us How about we keep the socket API unchanged in the next release? And merge this PR after the new version is released?
the signatures of sock_addr_local/sock_addr_remote/sock_addr_resolve
@wenyongh I understand your concerns and I agree those APIs have changed. However, they did not have the implementation, so I'm not sure if there's a serious risk in releasing those - not only they didn't have an implementation, but they were also not exposed through wasi_sock_ext so not sure if there was any usecase of those functions?
Having said that, I don't mind waiting with the merge until all the changes we're waiting for are merged to dev/socket. I guess we can have another WAMR release shortly after we merge dev/socket into main?
the signatures of sock_addr_local/sock_addr_remote/sock_addr_resolve
@wenyongh I understand your concerns and I agree those APIs have changed. However, they did not have the implementation, so I'm not sure if there's a serious risk in releasing those - not only they didn't have an implementation, but they were also not exposed through wasi_sock_ext so not sure if there was any usecase of those functions?
Having said that, I don't mind waiting with the merge until all the changes we're waiting for are merged to dev/socket. I guess we can have another WAMR release shortly after we merge dev/socket into main?
@loganek Yes, thanks for your understanding, my concern is that the current wasm app compiled with recent commits (from WAMR-05-18-2022 to current commit in main branch) cannot correctly run with the runtime compiled with dev/socket, developer may complain about the compatibility, though he can recompile the wasm app to resolve the issue. And yes, it seems a good idea to create a release without dev/socket PRs, and then create another release with dev/socket some time later. @lum1n0us What is you opinion? Is that OK?
We've internally performed the tests and the PR is ready for review and merge. @wenyongh could you remove a draft flag from the PR and have a look? Thanks.