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.

Sign up for GitHub

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

Jump to bottom

Merged blaugold's WASM branch, plus fixes #232

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Merged blaugold's WASM branch, plus fixes #232

wants to merge 4 commits into from

Conversation

snej
Copy link
Contributor

@snej snej commented Jun 13, 2024

Cherry-picked @blaugold 's branch that adds WASM support via emscripten.
Made a few minor changes to get it to build again.
Added a Makefile to simplify (all types of) builds.

@snej snej requested a review from borrrden June 13, 2024 20:59
@snej snej self-assigned this Jun 13, 2024
borrrden
borrrden reviewed Jun 13, 2024
View reviewed changes
Fleece/Support/ParseDate.cc
@@ -558,6 +558,8 @@ namespace fleece {
auto offset = seconds(-s);
#elif defined(__DARWIN_UNIX03) || defined(__ANDROID__) || defined(_XOPEN_SOURCE) || defined(_SVID_SOURCE)
auto offset = seconds(-timezone);
#elif defined(__EMSCRIPTEN__)
auto offset = seconds(timezone);
Copy link
Member

Choose a reason for hiding this comment

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

Is this negated on purpose? Does ECMAScript somehow change the functionality here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was necessary to make the tests pass, but it seems like a bug in Emscripten which implements all these UNIX APIs. Not sure what the current behavior is, though.

@borrrden
Copy link
Member

Also none of this builds at all on Windows, is that known?

@borrrden
Copy link
Member

borrrden commented Jun 13, 2024 •
edited
Loading

It also appears to not build on Linux unless a specific version of emscripten is required? The final link fails possibly because of an unrecognized argument but somewhat unclear and too long to paste here.

With the latest emsdk it works on Ubuntu but the tests abort with the following:

node build_cmake/test_wasm/FleeceTests.js 7 ↵
failed to asynchronously prepare wasm: CompileError: WebAssembly.instantiate(): unexpected section (enable with --experimental-wasm-eh) @+20676
Aborted(CompileError: WebAssembly.instantiate(): unexpected section (enable with --experimental-wasm-eh) @+20676)
/home/borrrden/Development/couchbase-lite-c/vendor/couchbase-lite-core/vendor/fleece/build_cmake/test_wasm/FleeceTests.js:143
throw ex;
^
RuntimeError: Aborted(CompileError: WebAssembly.instantiate(): unexpected section (enable with --experimental-wasm-eh) @+20676)
at abort (/home/borrrden/Development/couchbase-lite-c/vendor/couchbase-lite-core/vendor/fleece/build_cmake/test_wasm/FleeceTests.js:697:41)
at /home/borrrden/Development/couchbase-lite-c/vendor/couchbase-lite-core/vendor/fleece/build_cmake/test_wasm/FleeceTests.js:765:5

@snej snej mentioned this pull request Jun 14, 2024
Draft
@Iceburgino
Copy link

Are there any updates on this?

@snej
Copy link
Contributor Author

snej commented Sep 24, 2024

I think it would be OK to merge this, since it causes no real changes in "normal" supported builds. The conflicts are probably easy to resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blaugold blaugold blaugold left review comments

@borrrden borrrden borrrden left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@snej snej

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@snej @borrrden @Iceburgino @blaugold