Skip to content

Commit bc8dbf4

Browse files
committed
fix(io): build under Emscripten without per-consumer workarounds
Two independent problems stopped an Emscripten build of this library, and both forced every consumer to know something it should not have to. RandomAccess.cpp: all twelve calls to ThrowNative() sit in a _WIN32 or POSIX branch, and each of those three-way #if blocks answers Emscripten with PlatformNotSupportedException instead. On that target the helper genuinely has no caller. GCC does not diagnose that; Clang, which Emscripten uses, reports "unused function 'ThrowNative' [-Werror,-Wunused-function]" and the build stops. It is now compiled only where it is used -- no suppression, and the dead code stays out of the wasm binary. NativeDetail() had to move inside the same guard: its only caller is ThrowNative, so excluding just ThrowNative moves the identical error one line up. modules/io-compression: Emscripten ships zlib as a PORT rather than a system library, so find_package(ZLIB REQUIRED) failed with "missing: ZLIB_LIBRARY ZLIB_INCLUDE_DIR" until the consumer ran `embuilder build zlib` and passed both paths by hand. The module now requests the port itself when targeting Emscripten and keeps find_package everywhere else. That link option is PUBLIC deliberately. This is a STATIC library, so -sUSE_ZLIB=1 has to reach the final executable's link line too; PRIVATE would configure and compile cleanly and then fail whoever links the result, which is the worse of the two failures. Verified with emsdk 6.0.6 against the CNA consumer, with NO workaround flags of any kind: * emcmake configure succeeds -- no "Could NOT find ZLIB" * sharp_runtime_io compiles (this was the -Werror failure) * sharp_runtime_io_compression compiles and links Native is unaffected -- the guard keeps both helpers, and the CMake else() branch is unchanged. The CNA HEADLESS suite is identical before and after: 6389 tests, 6172 passed, 217 skipped, 0 failed. Reported as finding 3 in openeggbert/cna's threeissues.md, which is being updated to match.
1 parent 7a46a53 commit bc8dbf4

2 files changed

Lines changed: 26 additions & 2 deletions

File tree

modules/io-compression/CMakeLists.txt

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,21 @@
22
# Copyright (c) Robert Vokac and contributors
33

44
function(_sharp_runtime_setup_io_compression target)
5-
find_package(ZLIB REQUIRED)
6-
target_link_libraries("${target}" PRIVATE ZLIB::ZLIB)
5+
if(EMSCRIPTEN)
6+
# Emscripten ships zlib as a PORT, not as a system library, so find_package(ZLIB) fails
7+
# with "missing: ZLIB_LIBRARY ZLIB_INCLUDE_DIR" until someone runs `embuilder build zlib`
8+
# and passes both paths by hand. Requesting the port here means every consumer no longer
9+
# has to know that.
10+
#
11+
# The link option is PUBLIC on purpose: this is a STATIC library, so the port has to reach
12+
# the FINAL executable's link line as well. PRIVATE would configure and build cleanly and
13+
# then fail whoever links the result, which is the worse failure of the two.
14+
target_compile_options("${target}" PUBLIC "-sUSE_ZLIB=1")
15+
target_link_options("${target}" PUBLIC "-sUSE_ZLIB=1")
16+
else()
17+
find_package(ZLIB REQUIRED)
18+
target_link_libraries("${target}" PRIVATE ZLIB::ZLIB)
19+
endif()
720
endfunction()
821

922
sharp_runtime_register_module(

modules/io/src/System/IO/RandomAccess.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@ intcs CheckedVectorCount(std::size_t size) {
8080
return static_cast<intcs>(size);
8181
}
8282

83+
// Every caller of the two helpers below sits in a _WIN32 or POSIX branch, and each of those
84+
// three-way #if blocks answers Emscripten with PlatformNotSupportedException instead. So on that
85+
// target the helpers genuinely have no caller, and Clang -- which Emscripten uses, where GCC does
86+
// not diagnose this -- reports "unused function 'ThrowNative'" against -Werror and the build stops.
87+
// Compiling them only where they are used fixes it without a suppression, and keeps the dead code
88+
// out of the wasm binary. NativeDetail has exactly one caller, ThrowNative, so both must be inside
89+
// the same guard: excluding only ThrowNative moves the same error one line up.
90+
#if !defined(__EMSCRIPTEN__)
91+
8392
/// The native reason a syscall failed, appended to the IOException message. Before #2100 every
8493
/// failure surfaced as a bare "RandomAccess::<member> failed" with the reason discarded, so a
8594
/// caller could not tell EBADF from EINVAL from ENOSPC.
@@ -94,6 +103,8 @@ std::string NativeDetail(int code) {
94103
throw IOException(std::string(member) + " failed: " + NativeDetail(code));
95104
}
96105

106+
#endif // !defined(__EMSCRIPTEN__)
107+
97108
} // namespace
98109

99110
int64_t RandomAccess::GetLength(int fd) {

0 commit comments

Comments
 (0)