Skip to content

Conversation

@mhdawson
Copy link
Member

@mhdawsonmhdawson commented Jan 31, 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 [email protected]

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/wasi

@nodejs-github-botnodejs-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
@mhdawsonmhdawson removed the wasi Issues and PRs related to the WebAssembly System Interface. label Jan 31, 2023
@mhdawsonmhdawson added the wasi Issues and PRs related to the WebAssembly System Interface. label Jan 31, 2023
@mhdawsonmhdawson changed the title Wasi sockets2wasi: add wasi sock_accept stubJan 31, 2023
Copy link
Contributor

@cjihrigcjihrig left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments.

@mhdawson
Copy link
MemberAuthor

Pushed commit to address all comment so far.

@mhdawson
Copy link
MemberAuthor

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

Signed-off-by: Michael Dawson <[email protected]>
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 <[email protected]>
@mhdawson
Copy link
MemberAuthor

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.

@mhdawsonmhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@cjihrigcjihrig left a comment

Choose a reason for hiding this comment

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

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 <[email protected]> PR-URL: #46434 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: #46434 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@mhdawson
Copy link
MemberAuthor

Landed in 8e4fa26...bd04106

@mhdawsonmhdawson closed this Mar 1, 2023
targos pushed a commit that referenced this pull request Mar 13, 2023
Signed-off-by: Michael Dawson <[email protected]> PR-URL: #46434 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: #46434 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this pull request Mar 14, 2023
Signed-off-by: Michael Dawson <[email protected]> PR-URL: #46434 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: #46434 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@targostargos mentioned this pull request Mar 14, 2023
@danielleadams
Copy link
Contributor

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

@mhdawson
Copy link
MemberAuthor

@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 <[email protected]> PR-URL: nodejs#46434 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: nodejs#46434 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@mhdawson
Copy link
MemberAuthor

@danielleadams PR for backport - #47455

danielleadams pushed a commit that referenced this pull request May 29, 2023
Signed-off-by: Michael Dawson <[email protected]> PR-URL: #46434 Backport-PR-URL: #47455 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: #46434 Backport-PR-URL: #47455 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
aduh95 pushed a commit to aduh95/node that referenced this pull request Feb 18, 2025
Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#46434 Backport-PR-URL: nodejs#47455 Refs: nodejs/uvwasi#185 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> PR-URL: nodejs#46434 Backport-PR-URL: nodejs#47455 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto 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-ciPRs that need a full CI run.wasiIssues and PRs related to the WebAssembly System Interface.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@mhdawson@nodejs-github-bot@danielleadams@jasnell@cjihrig