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

Exclude APIs that dup given file descriptors from WASI builds #3475

Conversation

kateinoigakukun
Copy link
Contributor

@kateinoigakukun kateinoigakukun commented Jul 18, 2024

WASI doesn't support dup(2) system call, so we cannot implement the print_dot_graph and print_dot_graphs functions with exactly the same semantics as in other platforms.

In file included from ChimeHQ/SwiftTreeSitter/Packages/tree-sitter/lib/src/lib.c:13:
ChimeHQ/SwiftTreeSitter/Packages/tree-sitter/lib/src/./tree.c:156:10: error: call to undeclared function 'dup'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  156 |   return dup(file_descriptor);
      |          ^
1 error generated.

If we really need those functionalities in WASI, we can provide new variants of them that consumes the given fds.


Alternative consideration

If we can accept semantics changes on the fd consumption for those public APIs, we can provide those APIs without adding new variants.

@maxbrunsfeld
Copy link
Contributor

Maybe the ideal fix would be to move away from using fdopen at all, and write to the file descriptor directly with ‘write’. We could buffer the input ourselves using snprintf

@maxbrunsfeld
Copy link
Contributor

I’m fine with this change for now. I don’t want to change the semantics to consume the fd.

@kateinoigakukun kateinoigakukun force-pushed the pr-823711af3ae2558a96cd8caea4b244074ac3cec7 branch from 2089385 to 7d4aad0 Compare July 19, 2024 14:32
@kateinoigakukun
Copy link
Contributor Author

@maxbrunsfeld Even if we write contents by write(2) directly, it changes the offset of the fd. So if we want to keep the current semantics (user provided fd should not change its offset), we still need to dup the fd IIUC.

@maxbrunsfeld
Copy link
Contributor

Oh, good point. I think that is a fine semantics change though. I don’t think it’s as disruptive as taking ownership of the file descriptor.

WASI doesn't support `dup(2)` system call, so we cannot implement the
`print_dot_graph` and `print_dot_graphs` functions with exactly the same
semantics as in other platforms.
@amaanq amaanq force-pushed the pr-823711af3ae2558a96cd8caea4b244074ac3cec7 branch from 7d4aad0 to 06c85d1 Compare September 30, 2024 00:53
@amaanq amaanq merged commit 94a8262 into tree-sitter:master Sep 30, 2024
12 checks passed
@tree-sitter-ci-bot
Copy link

Successfully created backport PR for release-0.23:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants