Bump to 5.1.7 #15
Draft
Bump to 5.1.7 #15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It seems that an old refactoring (~8 years) made a mistake on the way conditions are expressed in gyp. the "conditions" key cannot be set 2 times and the conditions should be all set in the "conditions" array. The impact of this bug is that when sqlite3 is compiled against a custom sqlite3 source tree, the sqlite3 runtime keeps linking with the system sqlite3 instead of linking to the custom compiled sqlite3 library. In my case that lead to a SIGSEGV upon loading an extension.
refs https://www.sqlite.org/releaselog/3_41_1.html - sha3-256sum: `38ecb6b086c5c1ee1e52b57556745055328ac912929ccade9deaefdd71033ddb`
refs GHSA-jqv5-7xpx-qj74 fixes https://github.com/TryGhost/Toolbox/issues/491 - when you call `ToString()` on `Napi::Value`, it calls `napi_coerce_to_string` underneath, which has the ability to run arbitrary JS code if the passed in value is a crafted object - both remote code execution or denial-of-service are possible via this vulnerability - `toString()` on an Object returns `[object Object]` so instead of calling the function, we're going to hardcode it to prevent this issue Credits: Dave McDaniel of Cisco Talos
…h methods callback rows (TryGhost#1686) * Change sqlite3.d.ts to add generic type definition support for Statement methods -For get method callback's row -For all method callback's rows -For each method callback's row * Change sqlite3.d.ts to add generic type definition support for Database methods -For get method callback's row -For all method callback's rows -For each method callback's row
- added myself as a contributor 😀
fixes TryGhost#1690 - the minimum glibc version was inadvertently bumped in the latest release because `log2` was linked to a higher version - this adds the downgrade to the gcc preinclude file to resolve this - in an upcoming commit, we will add a test to prevent these issues from occurring again
- without this, the syntax is not recognised because we use `sh`
refs https://www.sqlite.org/releaselog/3_42_0.html - sha3-256sum: `643898e9fcc8f6069bcd47b0e6057221c1ed17bbee57da20d2752c79d91274e8`
…dability - Implemented RAII to ensure resource safety and exception safety - Leveraged range-based for loops for cleaner and more efficient iteration - Addressed and resolved all project-related warnings for enhanced code quality
This reverts commit 7552aa0.
refs https://www.sqlite.org/releaselog/3_44_2.html - sha3-256sum: `6c427f0547e2f7babe636b748dd5d5a1f2f31601adadef7e2805e7d1f7171861`
- this doesn't do anything because we just rethrow the error
- Python 3.12 no longer ships with this and GHA CI has updated to 3.12, which breaks our build scripts - this should fix that until we can update node-gyp
- merged pretest and test - removed `pack` as it is not needed
- right now our way to check semver compatibility is to run the tests on every version - this is really inefficient as it results in a lot of CI jobs that we don't need - this should run a CI job that ensures the minimum version we accept is Node 10.12.0
- bumped building on Node 16 to Node 18 - removed all matrix executions apart from Node 18
…stall` fixes TryGhost#1641 fixes TryGhost#1721 fixes TryGhost#1714 fixes TryGhost#1713 fixes TryGhost#1700 fixes TryGhost#1704 - `@mapbox/node-pre-gyp` is effectively unmaintained [1] as has a few bugs which our users keep running into - it seems the prebuilt binary world has moved in favor of prebuild + it's various other forms - one option would be to use prebuildify to bundle all binaries into the package, but that's a step too far removed from the current situation for now - instead, we can use prebuild-install to download the binaries, and `prebuild` to build + upload the binaries - this means we can remove node-pre-gyp and fix a bunch of issues! - eventually, we could start providing electron prebuilt binaries too [1]: mapbox/node-pre-gyp#657
- switched to just providing a list of platform + arch instead of the full formatted string
- this fixes building in debug mode because the compiler (at least, on macOS) doesn't like the `assert(status == 0)` - this also DRYs up some common code by moving it into a macro
- the path here was incorrect
- this cleans up similar definitions and makes further refactors easier
- the whole benchmark suite needs reworking but this should be a good poor mans method to make the benchmarking suite heavier
…tiation refs https://github.com/nodejs/node-addon-api/blob/main/doc/object.md#set - according to the node-addon-api docs, you can set various things as the key for a Napi::Object, including a std::string ref - instantiating a `Napi::String::New` is quite heavy, especially when we're doing it for every row we return, so we can avoid doing that and speed up the function - locally, this speeds up the benchmark by 5-15% (a lot of variance) but YMMV
- these extra functions aren't really necessary and means there is more redirection occurring - as a bonus, this fixes a variable shadowing issue in the Statement implementation
- this was never passed to the user because the value below is `1` - our documentation says the close event doesn't emit any parameters, so we can just remove it from the array
- we don't want to suggest it's released until all the builds are done - also configured verbose logging for uploads
- I foolishly used a bash env var format but Windows uses Powershell - simplest here is just to remove the env var and pass it in directly
- the folder here has changed since we switched to prebuild
- we only run CI on one version so it's not needed any more
refs https://www.sqlite.org/releaselog/3_45_0.html - sha3-256sum: `9fc2a78088875ae7c112957d58ccc52b1a0a4afa34ac669290be42f352b1aa76`
Co-authored-by: Hannah Wolfe [email protected]
Merge remote-tracking branch 'upstream/master' into grist-main
fflorent
reviewed
Jun 24, 2025
| @@ -154,7 +158,7 @@ jobs: | |||
| --build-arg NODE_VERSION=${{ matrix.node }} \ | |||
| . | |||
| CONTAINER_ID=$(docker create -it sqlite-builder) | |||
| docker cp $CONTAINER_ID:/usr/src/build/build/ ./build | |||
| docker cp $CONTAINER_ID:/usr/src/build/prebuilds/ ./prebuilds | |||
There was a problem hiding this comment.
This removal from this file does not seem to appear in your diff, any reason for that?
TryGhost@605c7f9#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fL85-L87
- name: Run tests
run: yarn test
- - name: Package prebuilt binaries
- run: yarn node-pre-gyp package --target_arch=${{ env.TARGET }}
-
- name: Upload binaries to commit artifacts
uses: actions/upload-artifact@v3
if: matrix.node == 18
with:| "install": "node-pre-gyp install --fallback-to-build", | ||
| "pretest": "node test/support/createdb.js", | ||
| "test": "mocha -R spec --timeout 480000", | ||
| "test:memory": "node test/support/memory_check.js", |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello dear Grist Labs folks,
This PR is a proposal to be more up to date with this Grist dependency.
It roughly consist in a merge of last commit of node-sqlite3 upstream in
grist-mainbranch and some little fixes of resulting conflicts.It build without fault locally.
I do not have tested the result with grist-core yet.