Skip to content

sqlite: add build option to build without sqlite - #58122

Closed
mhdawson wants to merge 4 commits into
nodejs:mainfrom
mhdawson:sqlite-option
Closed

sqlite: add build option to build without sqlite#58122
mhdawson wants to merge 4 commits into
nodejs:mainfrom
mhdawson:sqlite-option

Conversation

@mhdawson

Copy link
Copy Markdown
Member

Distributors of Node.js may want to ship without all of surface area so in some cases
build time options are provided to to that.

I was asked by the RHEL team to add an option to build without sqlite so that have that
flexibility and this PR does that.

Signed-off-by: Michael Dawson <midawson@redhat.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/sqlite
  • @nodejs/startup

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 2, 2025
Signed-off-by: Michael Dawson <midawson@redhat.com>
Comment threadsrc/node_metadata.h Outdated
Comment threadtest/common/index.js
hasIntl,
hasCrypto,
hasQuic,
hasSQLite,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the declaration here?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anonrig can you clarify what you mean on this one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having the variable declaration on top of the file, what about:

Suggested change
hasSQLite,
hasSQLite: Boolean(...),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not consistent with how any of the other ones are done as far as I can see. I was just trying to follow what seems to be existing practice.

Comment threadtest/parallel/test-config-file.js Outdated
Comment threadtest/parallel/test-sqlite-backup.mjs Outdated
@codecov

codecovBot commented May 2, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.14%. Comparing base (53abd1a) to head (65e8818).
Report is 109 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #58122 +/- ##
==========================================
- Coverage 90.20% 90.14% -0.07% 
==========================================
Files 630 630 Lines 186391 186784 +393 Branches 36612 36651 +39 ==========================================
+ Hits 168134 168369 +235 - Misses 11068 11199 +131 - Partials 7189 7216 +27 
Files with missing linesCoverage Δ
src/node_binding.cc82.67% <ø> (ø)
src/node_builtins.cc79.17% <100.00%> (-0.49%)⬇️
src/node_metadata.cc91.52% <ø> (ø)

... and 65 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Michael Dawson <midawson@redhat.com>
@mhdawson

Copy link
Copy Markdown
MemberAuthor

@anonrig addressed all of your comments except the one I asked for clarification on .

Comment threadsrc/node_builtins.cc
@geeksilva97geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label May 3, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 3, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadMakefile Outdated
Signed-off-by: Michael Dawson <midawson@redhat.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@geeksilva97geeksilva97 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 7, 2025
@mhdawsonmhdawson added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label May 7, 2025
mhdawson added a commit that referenced this pull request May 7, 2025
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #58122
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson

Copy link
Copy Markdown
MemberAuthor

Landed in 535c2f7

@mhdawsonmhdawson closed this May 7, 2025
targos pushed a commit that referenced this pull request May 16, 2025
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #58122
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jun 10, 2025
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #58122
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jul 10, 2025
codebytere added a commit to electron/electron that referenced this pull request Jul 11, 2025
codebytere added a commit to electron/electron that referenced this pull request Jul 11, 2025
codebytere added a commit to electron/electron that referenced this pull request Jul 14, 2025
codebytere added a commit to electron/electron that referenced this pull request Jul 15, 2025
tropBot added a commit to electron/electron that referenced this pull request Jul 15, 2025
nodejs/node#58122
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
tropBot added a commit to electron/electron that referenced this pull request Jul 15, 2025
nodejs/node#58122
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
tropBot added a commit to electron/electron that referenced this pull request Jul 15, 2025
nodejs/node#58122
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jul 16, 2025
nodejs/node#58122
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jul 16, 2025
nodejs/node#58122
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jul 16, 2025
nodejs/node#58122
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
georgexu99 pushed a commit to electron/electron that referenced this pull request Jul 16, 2025
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@mhdawson@nodejs-github-bot@jasnell@anonrig@aduh95@geeksilva97@RafaelGSS