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

Change whence_t constant values to match pre-existing agreed-upon values. #106

Merged
merged 3 commits into from Oct 11, 2019

Conversation

@kripken
Copy link
Member

kripken commented Oct 3, 2019

Background: #103

This reorders the whence constants to match existing values, shared by things like musl and SDL.

Without this, using a libc with the old wasi constants will not work in SDL, which assumes those values are fixed. As SDL is a stable and portable codebase, apparently these constants are consistent in practice across different platforms.

We only noticed the SDL issue by chance, so it's very possible there are more such issues out there. By changing the constants we'll minimize code porting issues in the long term. However, in the short term existing wasi runtimes and binaries may disagree.

@kripken kripken mentioned this pull request Oct 4, 2019
@sbc100

This comment has been minimized.

Copy link
Member

sbc100 commented Oct 7, 2019

I agree. I don't see any reason to diverge from the status quo and the fact the an extensively ported codebase like SDL relies on these values is a good sign that they don't generally vary between platforms.

We should prepare a parallel patch from wasi-libc.

@sunfishcode

This comment has been minimized.

Copy link
Member

sunfishcode commented Oct 8, 2019

No objection from me. The current order comes from cloudlibc, so wasi-libc isn't the only one here. However, I don't feel it's important enough for wasi-libc to take a stand on. For completeness, I have now filed a bug in SDL. (For musl's part, it explicitly assumes it's running on Linux.)

As a procedural matter, this change should be made to the "ephemeral" tree, rather than the "unstable" tree. We make ABI-breaking changes to "ephemeral" and occasionally promote them to "unstable" with version bumps. We can update wasi-libc when we do the version bump.

@kripken

This comment has been minimized.

Copy link
Member Author

kripken commented Oct 8, 2019

Thanks, changed to ephemeral.

@sbc100

This comment has been minimized.

Copy link
Member

sbc100 commented Oct 8, 2019

Cloudabi is actually even more strange and uses 1, 2, 3. No zero at all. Odd.

sbc100 added a commit to CraneStation/wasi-libc that referenced this pull request Oct 8, 2019
@sbc100 sbc100 mentioned this pull request Oct 8, 2019
kripken added a commit to emscripten-core/emscripten that referenced this pull request Oct 8, 2019
Update wasi to use the new whence constants, see
WebAssembly/WASI#106
@sunfishcode

This comment has been minimized.

Copy link
Member

sunfishcode commented Oct 11, 2019

No objections and this is a minor change, so let's merge!

@sunfishcode sunfishcode merged commit 5d0b1e7 into WebAssembly:master Oct 11, 2019
4 checks passed
4 checks passed
Test (ubuntu-latest)
Details
Test (macos-latest)
Details
Test (windows-latest)
Details
Rustfmt
Details
google-feinberg added a commit to google-feinberg/emscripten that referenced this pull request Dec 1, 2019
cjihrig added a commit to cjihrig/uvwasi that referenced this pull request Dec 4, 2019
cjihrig added a commit to cjihrig/uvwasi that referenced this pull request Dec 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.