FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

wasi: add wasi sock_accept stub by mhdawson · Pull Request #46434 · nodejs/node · GitHub

/ node Public

wasi: add wasi sock_accept stub - #46434

Closed
mhdawson wants to merge 2 commits into
nodejs:mainfrom
mhdawson:wasi-sockets2
Closed

wasi: add wasi sock_accept stub#46434
mhdawson wants to merge 2 commits into
nodejs:mainfrom
mhdawson:wasi-sockets2

Conversation

mhdawson commented Jan 31, 2023
edited
Loading

Copy link
Copy Markdown
Member

Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson mdawson@devrus.com

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/wasi

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. wasi Issues and PRs related to the WebAssembly System Interface. labels Jan 31, 2023
mhdawson removed the wasi Issues and PRs related to the WebAssembly System Interface. label Jan 31, 2023
mhdawson added the wasi Issues and PRs related to the WebAssembly System Interface. label Jan 31, 2023
mhdawson changed the title Wasi sockets2 wasi: add wasi sock_accept stub Jan 31, 2023

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with minor comments.

Comment thread src/node_wasi.cc Outdated
Comment thread src/node_wasi.cc Outdated
Comment thread src/node_wasi.h Outdated
Comment thread test/wasi/c/sock.c Outdated
Comment thread test/wasi/c/sock.c Outdated
Comment thread test/wasi/c/sock.c Outdated

Copy link
Copy Markdown
Member Author

Pushed commit to address all comment so far.

Copy link
Copy Markdown
Member Author

As mentioned needs an updated version of uvwasi before tests will pass.

Signed-off-by: Michael Dawson <mdawson@devrus.com>
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

Copy link
Copy Markdown
Member Author

Just added commit to update uvwasi to v0.0.16 so that tests can pass.

@cjihrig a re-LGTM from you might make sense due to that addition.

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2023

Copy link
Copy Markdown
Collaborator

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The release commit LGTM. Assuming the other commit hasn't changed, the PR LGTM

mhdawson added a commit that referenced this pull request Mar 1, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
mhdawson added a commit that referenced this pull request Mar 1, 2023
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

mhdawson commented Mar 1, 2023

Copy link
Copy Markdown
Member Author

Landed in 8e4fa26...bd04106

mhdawson closed this Mar 1, 2023
targos pushed a commit that referenced this pull request Mar 13, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Mar 13, 2023
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Mar 14, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Mar 14, 2023
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Copy link
Copy Markdown
Contributor

@mhdawson this didn't land cleanly on v18.x-staging. Do you mind opening a backport PR to v18.x? Thank you

mhdawson commented Apr 4, 2023

Copy link
Copy Markdown
Member Author

@danielleadams thanks for the head up. Will add it to my list of things.

mhdawson added a commit to mhdawson/io.js that referenced this pull request Apr 6, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#46434
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
mhdawson added a commit to mhdawson/io.js that referenced this pull request Apr 6, 2023
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#46434
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

mhdawson commented Apr 6, 2023

Copy link
Copy Markdown
Member Author

@danielleadams PR for backport - #47455

danielleadams pushed a commit that referenced this pull request May 29, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Backport-PR-URL: #47455
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this pull request May 29, 2023
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #46434
Backport-PR-URL: #47455
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this pull request Feb 18, 2025
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#46434
Backport-PR-URL: nodejs#47455
Refs: nodejs/uvwasi#185
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this pull request Feb 18, 2025
Refs: nodejs/uvwasi#185

Add stub for sock_accept so that we have stubs
for all of the sock methods in wasi_snapshot_preview1.
Its a bit awkward as the method was added after the
initial definitial of wasi_snapshot-preview1 but I
think it should be semver minor at most to add
the method.

Depends on nodejs/uvwasi#185
being landed in uvwasi first and an updated version
of uvwasi that includes that being pulled into
Node.js

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#46434
Backport-PR-URL: nodejs#47455
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. wasi Issues and PRs related to the WebAssembly System Interface.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL