Skip to content

Conversation

@sbc100
Copy link
Member

@sbc100 sbc100 commented Oct 24, 2025

There is now just one emscripten target called binaryen_js. The settings for these two targets were almost identical except for.

  1. Some JS_OF_OCAML specific stuff which is kept for the new unified target.
  2. -sASSERTIONS was being forced (See Avoid errors in binaryen.js assertions builds, and enable ASSERTIONS in debug builds. #2507). I dropped this in favor of doing a debug build in testing where this is enabled by default.

@sbc100 sbc100 force-pushed the drop_pure_js branch 2 times, most recently from 4e05087 to ccdd06a Compare October 25, 2025 00:00
@sbc100
Copy link
Member Author

sbc100 commented Oct 25, 2025

It looks like that JS_OF_OCAML stuff was added back in #4637

@phated are you guys limited to -sWASM=0 in some fundamental way? i.e. is dropping the pure JS build going to be a pain for you?

@spotandjake
Copy link
Contributor

It looks like that JS_OF_OCAML stuff was added back in #4637

@phated are you guys limited to -sWASM=0 in some fundamental way? i.e. is dropping the pure JS build going to be a pain for you?

The requirement for ES5 for JS_OF_OCAML was dropped a little bit ago; we have just been slow to upgrade libbinaryen. I've been playing around with trying to drop both the es5 and JS-only requirements on libbinaryen, mostly with success, though a tiny bit more work to do.

@sbc100 sbc100 force-pushed the drop_pure_js branch 2 times, most recently from 5ce6b39 to f374a5b Compare December 16, 2025 00:45
@sbc100
Copy link
Member Author

sbc100 commented Dec 16, 2025

It looks like that JS_OF_OCAML stuff was added back in #4637
@phated are you guys limited to -sWASM=0 in some fundamental way? i.e. is dropping the pure JS build going to be a pain for you?

The requirement for ES5 for JS_OF_OCAML was dropped a little bit ago; we have just been slow to upgrade libbinaryen. I've been playing around with trying to drop both the es5 and JS-only requirements on libbinaryen, mostly with success, though a tiny bit more work to do.

It would be really nice if we could drop JS_OF_OCAML completely from the CMakeLists file at some point. Are you close to being able to use the defaults?

@sbc100 sbc100 enabled auto-merge (squash) December 16, 2025 00:47
@sbc100 sbc100 requested a review from kripken December 16, 2025 00:47
There is now just one emscripten target called `binaryen_js`.  The
settings for these two targets were almost identical except for.

1. Some JS_OF_OCAML specific stuff which is kept for the new unified
   target.
2. `-sASSERTIONS` was being forced (See #2507).  I dropped this in favor
   of doing a debug build in testing where this is enabled by default.
@spotandjake
Copy link
Contributor

spotandjake commented Dec 16, 2025

It looks like that JS_OF_OCAML stuff was added back in #4637
@phated are you guys limited to -sWASM=0 in some fundamental way? i.e. is dropping the pure JS build going to be a pain for you?

The requirement for ES5 for JS_OF_OCAML was dropped a little bit ago; we have just been slow to upgrade libbinaryen. I've been playing around with trying to drop both the es5 and JS-only requirements on libbinaryen, mostly with success, though a tiny bit more work to do.

It would be really nice if we could drop JS_OF_OCAML completely from the CMakeLists file at some point. Are you close to being able to use the defaults?

We will still need the jsoo export wrapper even after getting the default build settings to work. I was getting somewhere with the defaults, though it seemed like a bit more work was required before we could use them just yet. (hoping to have time over the next month to allocate towards figuring things out).

@sbc100
Copy link
Member Author

sbc100 commented Dec 16, 2025

We will still need the jsoo export wrapper

Can you explain what this is? Are you referring to binaryen.jsoo-extern-pre.js or something else?

binaryen.jsoo-extern-pre.js in particular looks like it should be easy enough to inject from outside of the binaryen project?

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.

4 participants