-
Notifications
You must be signed in to change notification settings - Fork 216
Port clocks and filesystems functions to use native wasip2 operations #606
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
Conversation
Note: there's no automated way (that I know of) to check that there are no uses of the preview1 filesystems methods when |
6f9de1e
to
76e18ad
Compare
76e18ad
to
39e65a4
Compare
e51b64f
to
bf042e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far reviewed clocks, this one will take time to go through piece by piece
At this point, there should be no further uses of the preview1 component adapter for clocks and filesystems methods when __wasilibc_use_wasip2 is defined. Also added a DEBUG option to the Makefile that enables -O0/g when DEBUG=true is passed in.
Co-authored-by: Pat Hickey <[email protected]>
Co-authored-by: Pat Hickey <[email protected]>
a1a9296
to
b3c729d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today I made it up to the end of the libc itself, and I skimmed the tests. I'll give the tests another read-through later.
It appears __wasilibc_fd_renumber isnt ported in this PR - that should be possible to implement just by manipulating libc's own descriptor table
} | ||
case FIONBIO: { | ||
#ifdef __wasilibc_use_wasip2 | ||
// wasip2 doesn't support setting the non-blocking flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats correct, and for files this doesn't matter, but if this was a socket, I think we'd want to emulate setting the nonblocking flag by switching the implementations of send/recv from blocking to nonblocking stream read/writes. But just googling around it seems like this particular ioctl has a checkered past so maybe it doesn't matter if we just say ENOTSUP? Genuinely not sure what the right thing is to do here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats correct, and for files this doesn't matter, but if this was a socket, I think we'd want to emulate setting the nonblocking flag by switching the implementations of send/recv from blocking to nonblocking stream read/writes.
I'm hesitant to do that in this PR because I don't know how to test the sockets code (it sounds like testing was done manually in the past?) and it seems like a non-trivial change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, lets set this aside and wait for @wingo to provide some way of testing sockets for p3 that we can backport to p2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #621
Done -- I had made that change as part of #613 , but now I've pulled it into this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of all of the feedback, this looks good to go
At this point, there should be no further uses of the preview1 component adapter for clocks and filesystems methods when __wasilibc_use_wasip2 is defined.
Also added a DEBUG option to the Makefile that enables -O0/g when DEBUG=true is passed in.