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

Expose sqlite3_update_hook #210

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Conversation

hazelmeow
Copy link
Contributor

This PR adds support for sqlite3_update_hook.

Checklist

  • I grant to recipients of this Project distribution a perpetual,
    non-exclusive, royalty-free, irrevocable copyright license to reproduce, prepare
    derivative works of, publicly display, sublicense, and distribute this
    Contribution and such derivative works.
  • I certify that I am legally entitled to grant this license, and that this
    Contribution contains no content requiring a license from any third party.

@hazelmeow
Copy link
Contributor Author

@rhashimoto I see that CI failed on "Test with checked-in WASM files". Should I build and commit new WASM files? The output I get (for the js at least) is significantly different from what's checked in right now.

@rhashimoto
Copy link
Owner

Sure, let's try that. What version of Emscripten are you using? I think what is checked in now was built with 3.1.47 so if you're using something significantly more current then that might explain why things are so different.

@hazelmeow
Copy link
Contributor Author

I'm using emcc 3.1.66. I pushed a commit with a new dist/ built so hopefully that passes CI, but it feels a little wrong to me to have binaries compiled by someone random (me) on master...

@rhashimoto
Copy link
Owner

I agree. That's one reason I asked for your Emscripten version - to try to duplicate the built bytes before merging.

Ah, the tests work now! I'll do the security check the next time I have free cycles.

@rhashimoto
Copy link
Owner

My build is byte for byte identical on 3.1.66. Local testing failed, which was resolved by upgrading Chromium in my development container from 122 to 129. I think the problem was changes in the JSPI API that caused the Chromium and Emscripten versions to be out of step. Curiously, the GitHub CI action is still using Chromium 121 and that works, but JSPI support has been evolving behind a flag so it isn't unbelievable.

Thanks for the PR!

@rhashimoto rhashimoto merged commit a7e3676 into rhashimoto:master Sep 23, 2024
1 check passed
@davidisaaclee davidisaaclee mentioned this pull request Nov 3, 2024
2 tasks
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.

2 participants