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

Fix static wasm build without wasi #94

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Apr 22, 2022

libc doesn't have off_t for WebAssembly without WASI, so it falls
back to long. Do the same in this crate to fix the build error when
trying to statically build libz for WebAssembly.

Rough code from zconf.h declaring z_off_t:

#ifndef Z_SOLO
#  if defined(Z_HAVE_UNISTD_H) || defined(_LARGEFILE64_SOURCE)
// [...]
#    ifndef z_off_t
#      define z_off_t off_t
#    endif
#  endif
#endif

// [...]

#ifndef z_off_t
#  define z_off_t long
#endif

Fixes #95.

@joshtriplett
Copy link
Member

@tbu- I'd prefer not to special-case WebAssembly here. Could you, instead, submit a PR to the libc crate defining off_t for WebAssembly targets?

@tbu-
Copy link
Contributor Author

tbu- commented Apr 24, 2022

@joshtriplett There's no off_t for WebAssembly, as far as I understand it. off_t is defined by POSIX, and WebAssembly doesn't implement POSIX. zlib defines off_t to be long when off_t isn't defined. It seems to me that the handling needs to be in zlib-sys.

@joshtriplett
Copy link
Member

@tbu- AFAICT WASI defines filesize and filedelta, which provide the equivalents of off_t. So it's defined that offsets are always 64-bit.

@tbu-
Copy link
Contributor Author

tbu- commented Apr 26, 2022

WASI does indeed define off_t:

https://github.com/rust-lang/libc/blob/f6df53fd694f6fc903058c765efc10d77725b31b/src/wasi.rs#L23

This is only included when targeting WASI though, not on all of WebAssembly:

https://github.com/rust-lang/libc/blob/f6df53fd694f6fc903058c765efc10d77725b31b/src/lib.rs#L150-L156

But WASI is just a possible environment of WebAssembly, libz also works without WASI, and in this case libz-sys (but not libz itself) fails to build. libz falls back to C's long type if off_t isn't present. Since libz does that, libz-sys probably should as well?

@joshtriplett
Copy link
Member

@tbu- Then in that case, can you please modify this to only apply for wasm targets with OS "unknown", but not WASI targets?

@joshtriplett joshtriplett reopened this Apr 26, 2022
@tbu- tbu- changed the title Fix static wasm build Fix static wasm build without wasi Apr 27, 2022
@tbu- tbu- force-pushed the pr_wasm_static branch 2 times, most recently from a1488c9 to 9b1d7a6 Compare April 27, 2022 08:02
@tbu-
Copy link
Contributor Author

tbu- commented Apr 27, 2022

The target_env of unknown targets is actually the empty string "" and not "unknown". :o

@tbu-
Copy link
Contributor Author

tbu- commented Apr 27, 2022

@tbu- Then in that case, can you please modify this to only apply for wasm targets with OS "unknown", but not WASI targets?

Done. Thanks for your patience and working with me. :) The CI failure is unrelated, it's a 500 error from crates.io.

Should I add a CI job testing this? It works locally.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
`libc` doesn't have `off_t` for WebAssembly without WASI, so it falls
back to `long`.  Do the same in this crate to fix the build error when
trying to statically build libz for WebAssembly.

Rough code from `zconf.h` declaring `z_off_t`:
```c++
#ifndef Z_SOLO
#  if defined(Z_HAVE_UNISTD_H) || defined(_LARGEFILE64_SOURCE)
// [...]
#    ifndef z_off_t
#      define z_off_t off_t
#    endif
#  endif
#endif

// [...]

#ifndef z_off_t
#  define z_off_t long
#endif
```

Fixes rust-lang#95.
@tbu-
Copy link
Contributor Author

tbu- commented Apr 27, 2022

Done.

By experimenting, I found that I need target_os = "unknown" instead of target_os = "none".

@joshtriplett
Copy link
Member

joshtriplett commented Apr 27, 2022

Done.

By experimenting, I found that I need target_os = "unknown" instead of target_os = "none".

Right, good catch. Mixed it up with x86_64-unknown-none. 🤦

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

Successfully merging this pull request may close these issues.

Doesn't compile for WebAssembly with static feature
2 participants