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

Clarify intentions of vscode-node-sqlite3 #14

Closed
tsondergaard opened this issue Dec 8, 2021 · 2 comments
Closed

Clarify intentions of vscode-node-sqlite3 #14

tsondergaard opened this issue Dec 8, 2021 · 2 comments

Comments

@tsondergaard
Copy link

The lack of response to TryGhost#1493 has caused a bit of frustration and projects are increasingly turning to vscode-node-sqlite3 as a new maintained fork. One example of a project that is changing to @vscode/sqlite3 is Knex. A pretty popular SQL query builder. See knex/knex#4858 and knex/knex#4866.

It is unclear from https://www.npmjs.com/package/@vscode/sqlite3 and the commits, issues, comments associated with this repo whether this is indeed the intent of microsoft/vscode-node-sqlite3.

Could you clarify your intentions? Can this be considered a new long-term public fork of node-sqlite3?

@gnprice
Copy link

gnprice commented Jan 10, 2022

FWIW my reading of this repo's commit logs and so on is that it is pretty clearly intended as a small fork for vscode's own convenience, with no promises at all of being useful to anyone else or responsive to anyone else's needs. (Which, to be clear, is a perfectly reasonable thing for its authors to do!)

For a start, the commit that makes most of the changes in this branch has a commit message of literally "chore: vscode specific changes": 59a82b2.

I think the lack of response to this issue over the past month is also a sign that this repo's maintainers do not intend for it to be something maintained for other people to use.

In the case of mapbox#1493, the thing that's being requested upstream is just to publish a release to NPM; the needed change is already merged in the master branch. So the thing that this repo is providing relative to upstream is purely that they've posted a build to NPM. And… looking at the "Versions" tab on the NPM page: https://www.npmjs.com/package/@vscode/sqlite3 one sees that they posted a burst of new versions in mid-November last year, one version last August, and that's the whole list. So in the absence of any commitments from the maintainers, I think that history doesn't give any reason to predict that they'll be regularly publishing new releases in the future either.


For anyone using sqlite3 who wants a version with the mapbox#1493 issue fixed, I think the good solutions are:

gnprice added a commit to zulip/zulip-mobile that referenced this issue Jan 11, 2022
In particular this leads to using a reasonably recent `tar` package,
fixing vulnerabilities in the old one it was using.

Upstream has already bumped this to node-gyp 7.x in their master
branch, but haven't posted a release to NPM:
  TryGhost/node-sqlite3#1493

Empirically node-gyp 8.x, the latest, works fine.  That's also
reported by someone on that issue thread:
  TryGhost/node-sqlite3#1493 (comment)
May as well go for that, then.  (There was no 8.x yet when the
version specified in sqlite3 was bumped to 7.x.)

Some other people on that thread report using a fork made by the
VS Code developers, which posted some releases in November.  But
that fork seems pretty clearly intended for VS Code's own internal
use, with no promises for broader consumption:
  microsoft/vscode-node-sqlite3#14 (comment)
so that doesn't seem like an improvement over upstream.
sumj25 pushed a commit to sumj25/zulip-mobile that referenced this issue Jan 12, 2022
In particular this leads to using a reasonably recent `tar` package,
fixing vulnerabilities in the old one it was using.

Upstream has already bumped this to node-gyp 7.x in their master
branch, but haven't posted a release to NPM:
  TryGhost/node-sqlite3#1493

Empirically node-gyp 8.x, the latest, works fine.  That's also
reported by someone on that issue thread:
  TryGhost/node-sqlite3#1493 (comment)
May as well go for that, then.  (There was no 8.x yet when the
version specified in sqlite3 was bumped to 7.x.)

Some other people on that thread report using a fork made by the
VS Code developers, which posted some releases in November.  But
that fork seems pretty clearly intended for VS Code's own internal
use, with no promises for broader consumption:
  microsoft/vscode-node-sqlite3#14 (comment)
so that doesn't seem like an improvement over upstream.
@bpasero
Copy link
Member

bpasero commented Feb 19, 2022

Our intent is not to create another distribution of this library but track the original library as closely as possible. In fact, #11 tracks upstreaming our changes and then we will delete this repository very likely.

Not using pre-bundled binaries for native modules is just a choice we made in VSCode where we build all native modules on each CI run and not rely on prebuild binaries. The reason is that we want to build native libraries in the same environment that we build VSCode in.

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

No branches or pull requests

3 participants