-
Notifications
You must be signed in to change notification settings - Fork 77
fix: added workflow permissions to build-wasm.yml #134
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
Conversation
tomayac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Still seems to fail: https://github.com/sqlite/sqlite-wasm/actions/runs/21176315929 |
|
It's a different issue now. Branch was actually created, but it can't open a PR, because we need to check a box in the settings. Here's the branch: https://github.com/sqlite/sqlite-wasm/compare/update-sqlite-wasm-d4dc275ebc2941a850d1a3efebe3e706e4f1967f?expand=1 This needs to be checked for the PR to be opened automatically though: |
|
@sgbeal Can you open https://github.com/sqlite/sqlite-wasm/settings and toggle the checkbox from the above screenshot? It 404s for me, which is strange, as I thought I was the repo owner, but maybe you are? I honestly forgot who set this up. |
|
/settings is also 404 for me. Edit: the settings button along the top is also 404. |
|
Apparently a wider issue: https://github.com/orgs/community/discussions/176383. *sigh |
From Oct. 2025 and still being reported. Sigh. |
|
I'm sorry, I didn't know about this 😢 I think we can still open PRs manually, since branch with changes is still created successfully until this is resolved. |
SGTM, can you do this and I can review? |
|
I can't, sorry. It says I'm not a collaborator :( Does this work for you? https://github.com/sqlite/sqlite-wasm/compare/sqlite:main...sqlite:update-sqlite-wasm-d4dc275ebc2941a850d1a3efebe3e706e4f1967f?expand=1 |
No lie, I wanted to make you one yesterday, and the Settings 404'ed, and I thought it was just a temporary glitch. So we're stuck for now.
This seems to have worked: #135. Can you go over it and check if everything you expect is there? |
|
Thank you for the vote of confidence! It's good to go! 😄 |
|
Release is out: https://github.com/sqlite/sqlite-wasm/releases/tag/3.51.2-build2 |
|
@jurerotar Could I ask you to let me know which Issues can be closed now? It seems like you currently have more insights than I do. |
|
Absolutely! I'm just testing the release on some of my projects and so far it seems to work perfectly! 😄 Give me a couple more minutes to make sure everything is fine, please, but after that we should be good to close the following: This one I think is just out of scope: #104 |
|
There's an issue with bundler-friendly builds. I think I know what it is, but I'm not exactly sure why it worked before when I tested it locally. I think this injection of If this
I'm currently investigating if this whole |
|
Thank you! |
|
Re. locateFile(), see: https://sqlite.org/src/info/c8e6be9241 Edit: that's not a fix for this (what is?) but it's related. |
|
Please try https://sqlite.org/src/info/982a91abc0, which makes pre-js disappear from bundler builds. |
Not entirely - it was only elided for bundler-friendly builds, not sqlite3.mjs (which is an ESM, non-bundler build). i can't remove them from all esm builds without breaking (at a minimum) tests. However, since https://sqlite.org/src/info/c8e6be9241e3e178, i believe/strongly suspect that pre-js is innocuous for non-bundler use, as people who need to override |



I think this should fix failing build-wasm workflow runs