Skip to content
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

wasi-common: deprecate fd_allocate #6217

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

pchickey
Copy link
Collaborator

@pchickey pchickey commented Apr 15, 2023

This removes features from WASI Preview 1 that were supported inconsistently across platforms.

We are doing so because these features never worked on windows, and can't be supported consistiently on mac and linux. They have all been removed from the WASI Preview 2 spec in progress because of these inconsistencies.

We are not aware of any real-world programs or WASI users that are broken by removing this support.

fd_allocate

This operation from cloudabi is linux-specific, isn't even supported across all linux filesystems, and has no support on macos
or windows. Rather than ship spotty support, it has been removed from preview 2, and this PR removes support for it in preview 1 as well.

The PR also removes flags from the test suite which work around this operation only being available on linux.

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Apr 15, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@pchickey pchickey changed the title wasi-common: deprecate fd_{sync, datasync, allocate} wasi-common: deprecate fd_allocate Apr 17, 2023
@pchickey pchickey marked this pull request as ready for review April 17, 2023 15:29
@pchickey pchickey requested a review from a team as a code owner April 17, 2023 15:29
@pchickey pchickey requested review from alexcrichton and removed request for a team April 17, 2023 15:29
@pchickey pchickey added this pull request to the merge queue Apr 17, 2023
// semantics on those two platforms. WASI never specified the semantics, and our test suite
// never included this operation. We have removed this operation from preview 2, and we are
// no longer supporting it in preview 1 as well.
Err(Error::not_supported())
Copy link
Member

Choose a reason for hiding this comment

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

This is still removing support for fd_sync and fd_datasync, which I think we should keep. Even though they're not portable, there is value in being able to expose what functionality the host might have to applications.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got rid of that change locally but I didnt hit -f when I pushed it! Sorry.

@pchickey pchickey removed this pull request from the merge queue due to a manual request Apr 17, 2023
Pat Hickey added 2 commits April 17, 2023 14:14
…TSUP

This operation from cloudabi is linux-specific, isn't even
supported across all linux filesystems, and has no support on macos
or windows. Rather than ship spotty support, it has been removed
from preview 2, and we are no longer supporting it in preview 1 as
well.

The preview 1 implementation will still check if fd is a file, and has
rights, just to reject those cases with the errors expected.
rewrite the file_allocate test to just check for failure.

remove use of fd_allocate from fd_advise test, and remove test
configuration setting used for excluding use of fd_allocate on macos and
windows.
@pchickey pchickey force-pushed the pch/deprecate_fd_sync_datasync_allocate branch from 4afa25a to ccf347e Compare April 17, 2023 21:15
@pchickey pchickey enabled auto-merge April 17, 2023 21:23
@pchickey pchickey added this pull request to the merge queue Apr 17, 2023
Merged via the queue into main with commit 9ee613a Apr 17, 2023
@pchickey pchickey deleted the pch/deprecate_fd_sync_datasync_allocate branch April 17, 2023 22:10
@pchickey pchickey mentioned this pull request Jul 10, 2023
@pchickey pchickey mentioned this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants