From d4243d6bad9db128bcd813f34b5434324b3e15a3 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Jan 2026 11:28:29 +0100 Subject: [PATCH 1/3] refactor: replace better-sqlite3 with node:sqlite - Use Node.js native node:sqlite module (available from Node 22.5.0) - Add Node.js version check at startup - Add engines field to package.json - Use named parameters consistently in SQL queries - Suppress ExperimentalWarning for node:sqlite in shebang and CI Benefits: - Zero native compilation dependencies (no node-gyp) - Built-in Node.js module - will stabilize over time - Same performance - local file-based SQLite database --- .github/workflows/build.yml | 2 +- bin/index.dev.js | 2 +- bin/src/Main.purs | 16 +++++++++ package.json | 4 ++- src/Spago/Db.js | 57 ++++++++++++++++++-------------- src/Spago/NodeVersion.purs | 37 +++++++++++++++++++++ test/Prelude.purs | 2 +- test/Spago/Unit.purs | 2 ++ test/Spago/Unit/NodeVersion.purs | 40 ++++++++++++++++++++++ 9 files changed, 133 insertions(+), 29 deletions(-) create mode 100644 src/Spago/NodeVersion.purs create mode 100644 test/Spago/Unit/NodeVersion.purs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f4f545a46..c29800b7c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -82,7 +82,7 @@ jobs: run: node ./bin/bundle.js bundle -p docs-search-client-halogen - name: Run tests - run: node ./bin/bundle.js test + run: node --no-warnings=ExperimentalWarning ./bin/bundle.js test - name: Check formatting (Linux only) if: matrix.os == 'ubuntu-latest' diff --git a/bin/index.dev.js b/bin/index.dev.js index e2193ac95..9da755bcf 100755 --- a/bin/index.dev.js +++ b/bin/index.dev.js @@ -1,4 +1,4 @@ -#!/usr/bin/env node +#!/usr/bin/env -S node --no-warnings=ExperimentalWarning import { main } from "../output/Main/index.js"; diff --git a/bin/src/Main.purs b/bin/src/Main.purs index 3c4b1b8bb..2cfb172fa 100644 --- a/bin/src/Main.purs +++ b/bin/src/Main.purs @@ -14,7 +14,9 @@ import Data.Maybe as Maybe import Data.Set as Set import Effect.Aff as Aff import Effect.Aff.AVar as AVar +import Effect.Console as Console import Effect.Now as Now +import Node.Process as Process import Options.Applicative (CommandFields, Mod, Parser, ParserPrefs(..)) import Options.Applicative as O import Options.Applicative.Types (Backtracking(..)) @@ -51,6 +53,8 @@ import Spago.Generated.BuildInfo as BuildInfo import Spago.Git as Git import Spago.Json as Json import Spago.Log (LogVerbosity(..)) +import Spago.NodeVersion (NodeVersionCheck(..)) +import Spago.NodeVersion as NodeVersion import Spago.Path as Path import Spago.Paths as Paths import Spago.Purs as Purs @@ -531,6 +535,7 @@ parseArgs = do main :: Effect Unit main = do + ensureMinimumNodeVersion startingTime <- Now.now parseArgs >>= \c -> Aff.launchAff_ case c of @@ -1049,3 +1054,14 @@ mkDocsEnv args dependencies = do } foreign import supportsColor :: Effect Boolean + +-- | Ensures Node.js version is >= 22.5.0 (required for node:sqlite) +ensureMinimumNodeVersion :: Effect Unit +ensureMinimumNodeVersion = + case NodeVersion.checkNodeVersion { major: 22, minor: 5 } Process.version of + NodeVersionOk -> pure unit + NodeVersionTooOld v -> do + Console.error $ "Error: spago requires Node.js v22.5.0 or later (found " <> v <> ")" + Process.exit' 1 + NodeVersionUnparseable v -> + Console.warn $ "Warning: spago could not parse the Node.js version: " <> v diff --git a/package.json b/package.json index ebdd11f7c..b2a601986 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,9 @@ }, "author": "Fabrizio Ferrai", "type": "module", + "engines": { + "node": ">=22.5.0" + }, "bin": { "spago": "bin/bundle.js" }, @@ -35,7 +38,6 @@ }, "dependencies": { "@nodelib/fs.walk": "^3.0.1", - "better-sqlite3": "^12.5.0", "env-paths": "^3.0.0", "fs-extra": "^11.3.0", "fuse.js": "^7.1.0", diff --git a/src/Spago/Db.js b/src/Spago/Db.js index cd5662903..2d85cb3f5 100644 --- a/src/Spago/Db.js +++ b/src/Spago/Db.js @@ -1,13 +1,20 @@ -import Database from "better-sqlite3"; +import { DatabaseSync } from "node:sqlite"; +import fs from "node:fs"; +import path from "node:path"; -export const connectImpl = (path, logger) => { - logger("Connecting to database at " + path); - let db = new Database(path, { - fileMustExist: false, - // verbose: logger, +export const connectImpl = (databasePath, logger) => { + logger("Connecting to database at " + databasePath); + + // Ensure directory exists + const dir = path.dirname(databasePath); + fs.mkdirSync(dir, { recursive: true }); + + const db = new DatabaseSync(databasePath, { + enableForeignKeyConstraints: true, }); - db.pragma("journal_mode = WAL"); - db.pragma("foreign_keys = ON"); + + db.exec("PRAGMA journal_mode = WAL"); + db.exec("PRAGMA foreign_keys = ON"); db.prepare(`CREATE TABLE IF NOT EXISTS package_sets ( version TEXT PRIMARY KEY NOT NULL @@ -31,7 +38,7 @@ export const connectImpl = (path, logger) => { , last_fetched TEXT NOT NULL )`).run(); // it would be lovely if we'd have a foreign key on package_metadata, but that would - // require reading metadatas before manifests, which we can't always guarantee + // require reading metadata before manifests, which we can't always guarantee db.prepare(`CREATE TABLE IF NOT EXISTS package_manifests ( name TEXT NOT NULL , version TEXT NOT NULL @@ -39,13 +46,13 @@ export const connectImpl = (path, logger) => { , PRIMARY KEY (name, version) )`).run(); return db; -}; +} export const insertPackageSetImpl = (db, packageSet) => { db.prepare( "INSERT OR IGNORE INTO package_sets (version, compiler, date) VALUES (@version, @compiler, @date)" ).run(packageSet); -}; +} export const insertPackageSetEntryImpl = (db, packageSetEntry) => { db.prepare( @@ -55,8 +62,8 @@ export const insertPackageSetEntryImpl = (db, packageSetEntry) => { export const selectLatestPackageSetByCompilerImpl = (db, compiler) => { const row = db - .prepare("SELECT * FROM package_sets WHERE compiler = ? ORDER BY date DESC LIMIT 1") - .get(compiler); + .prepare("SELECT * FROM package_sets WHERE compiler = @compiler ORDER BY date DESC LIMIT 1") + .get({ compiler }); return row; } @@ -69,22 +76,22 @@ export const selectPackageSetsImpl = (db) => { export const selectPackageSetEntriesBySetImpl = (db, packageSetVersion) => { const row = db - .prepare("SELECT * FROM package_set_entries WHERE packageSetVersion = ?") - .all(packageSetVersion); + .prepare("SELECT * FROM package_set_entries WHERE packageSetVersion = @packageSetVersion") + .all({ packageSetVersion }); return row; } export const selectPackageSetEntriesByPackageImpl = (db, packageName, packageVersion) => { const row = db - .prepare("SELECT * FROM package_set_entries WHERE packageName = ? AND packageVersion = ?") - .all(packageName, packageVersion); + .prepare("SELECT * FROM package_set_entries WHERE packageName = @packageName AND packageVersion = @packageVersion") + .all({ packageName, packageVersion }); return row; } export const getLastPullImpl = (db, key) => { const row = db - .prepare("SELECT * FROM last_git_pull WHERE key = ? LIMIT 1") - .get(key); + .prepare("SELECT * FROM last_git_pull WHERE key = @key LIMIT 1") + .get({ key }); return row?.date; } @@ -94,8 +101,8 @@ export const updateLastPullImpl = (db, key, date) => { export const getManifestImpl = (db, name, version) => { const row = db - .prepare("SELECT * FROM package_manifests WHERE name = ? AND version = ? LIMIT 1") - .get(name, version); + .prepare("SELECT * FROM package_manifests WHERE name = @name AND version = @version LIMIT 1") + .get({ name, version }); return row?.manifest; } @@ -104,7 +111,7 @@ export const insertManifestImpl = (db, name, version, manifest) => { } export const removeManifestImpl = (db, name, version) => { - db.prepare("DELETE FROM package_manifests WHERE name = ? AND version = ?").run(name, version); + db.prepare("DELETE FROM package_manifests WHERE name = @name AND version = @version").run({ name, version }); } export const insertMetadataImpl = (db, name, metadata, last_fetched) => { @@ -113,6 +120,6 @@ export const insertMetadataImpl = (db, name, metadata, last_fetched) => { export const getMetadataForPackagesImpl = (db, names) => { // There can be a lot of package names here, potentially hitting the max number of sqlite parameters, so we use json to bypass this - const query = db.prepare("SELECT * FROM package_metadata WHERE name IN (SELECT value FROM json_each(?));"); - return query.all(JSON.stringify(names)); -}; \ No newline at end of file + const query = db.prepare("SELECT * FROM package_metadata WHERE name IN (SELECT value FROM json_each(@names));"); + return query.all({ names: JSON.stringify(names) }); +} diff --git a/src/Spago/NodeVersion.purs b/src/Spago/NodeVersion.purs new file mode 100644 index 000000000..065d56018 --- /dev/null +++ b/src/Spago/NodeVersion.purs @@ -0,0 +1,37 @@ +module Spago.NodeVersion + ( NodeVersionCheck(..) + , checkNodeVersion + ) where + +import Prelude + +import Data.Array as Array +import Data.Int as Int +import Data.Maybe (Maybe(..), fromMaybe) +import Data.String as String +import Data.Traversable (traverse) + +data NodeVersionCheck + = NodeVersionOk + | NodeVersionTooOld String + | NodeVersionUnparseable String + +derive instance Eq NodeVersionCheck +instance Show NodeVersionCheck where + show NodeVersionOk = "NodeVersionOk" + show (NodeVersionTooOld v) = "(NodeVersionTooOld " <> show v <> ")" + show (NodeVersionUnparseable v) = "(NodeVersionUnparseable " <> show v <> ")" + +-- | Check if a version string meets the minimum Node.js version requirement +checkNodeVersion :: { major :: Int, minor :: Int } -> String -> NodeVersionCheck +checkNodeVersion minimum version = + case traverse Int.fromString (Array.take 2 parts) of + Just [major, minor] + | major > minimum.major -> NodeVersionOk + | major == minimum.major && minor >= minimum.minor -> NodeVersionOk + | otherwise -> NodeVersionTooOld version + _ -> NodeVersionUnparseable version + where + -- version is like "v22.5.0" or "22.5.0" + versionStr = String.stripPrefix (String.Pattern "v") version # fromMaybe version + parts = String.split (String.Pattern ".") versionStr diff --git a/test/Prelude.purs b/test/Prelude.purs index 38823edac..35803329a 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -65,7 +65,7 @@ withTempDir = Aff.bracket createTempDir cleanupTempDir spago' stdin args = Cmd.exec (Path.global "node") - ([ Path.toRaw $ oldCwd "bin" "index.dev.js" ] <> args) + ([ "--no-warnings=ExperimentalWarning", Path.toRaw $ oldCwd "bin" "index.dev.js" ] <> args) $ Cmd.defaultExecOptions { pipeStdout = false, pipeStderr = false, pipeStdin = stdin } spago = spago' StdinNewPipe diff --git a/test/Spago/Unit.purs b/test/Spago/Unit.purs index 065c5dd23..56d7a6e1d 100644 --- a/test/Spago/Unit.purs +++ b/test/Spago/Unit.purs @@ -5,6 +5,7 @@ import Prelude import Test.Spago.Unit.CheckInjectivity as CheckInjectivity import Test.Spago.Unit.FindFlags as FindFlags import Test.Spago.Unit.Git as Git +import Test.Spago.Unit.NodeVersion as NodeVersion import Test.Spago.Unit.Path as Path import Test.Spago.Unit.Printer as Printer import Test.Spec (Spec) @@ -17,3 +18,4 @@ spec = Spec.describe "unit" do Printer.spec Git.spec Path.spec + NodeVersion.spec diff --git a/test/Spago/Unit/NodeVersion.purs b/test/Spago/Unit/NodeVersion.purs new file mode 100644 index 000000000..59d638be6 --- /dev/null +++ b/test/Spago/Unit/NodeVersion.purs @@ -0,0 +1,40 @@ +module Test.Spago.Unit.NodeVersion where + +import Prelude + +import Spago.NodeVersion (NodeVersionCheck(..), checkNodeVersion) +import Test.Spec (Spec) +import Test.Spec as Spec +import Test.Spec.Assertions as Assertions + +minimum :: { major :: Int, minor :: Int } +minimum = { major: 22, minor: 5 } + +spec :: Spec Unit +spec = do + Spec.describe "checkNodeVersion" do + Spec.describe "accepts valid versions" do + Spec.it "v22.5.0" do + checkNodeVersion minimum "v22.5.0" `Assertions.shouldEqual` NodeVersionOk + Spec.it "22.5.0" do + checkNodeVersion minimum "22.5.0" `Assertions.shouldEqual` NodeVersionOk + Spec.it "v22.6.0" do + checkNodeVersion minimum "v22.6.0" `Assertions.shouldEqual` NodeVersionOk + Spec.it "v23.0.0" do + checkNodeVersion minimum "v23.0.0" `Assertions.shouldEqual` NodeVersionOk + Spec.it "v25.2.1" do + checkNodeVersion minimum "v25.2.1" `Assertions.shouldEqual` NodeVersionOk + + Spec.describe "rejects old versions" do + Spec.it "v22.4.0" do + checkNodeVersion minimum "v22.4.0" `Assertions.shouldEqual` NodeVersionTooOld "v22.4.0" + Spec.it "v21.0.0" do + checkNodeVersion minimum "v21.0.0" `Assertions.shouldEqual` NodeVersionTooOld "v21.0.0" + Spec.it "v18.17.0" do + checkNodeVersion minimum "v18.17.0" `Assertions.shouldEqual` NodeVersionTooOld "v18.17.0" + + Spec.describe "handles unparseable versions" do + Spec.it "garbage" do + checkNodeVersion minimum "garbage" `Assertions.shouldEqual` NodeVersionUnparseable "garbage" + Spec.it "empty string" do + checkNodeVersion minimum "" `Assertions.shouldEqual` NodeVersionUnparseable "" From 30d641ad102537e8185daa2910d47d016c782c30 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Jan 2026 22:38:11 +0100 Subject: [PATCH 2/3] fix: wrap package set inserts in transaction for Windows node:sqlite has locking issues on Windows where rapid sequential inserts fail with 'database is locked' even with timeout set. Using BEGIN IMMEDIATE acquires the lock once for all inserts. --- src/Spago/Db.js | 13 ++++++++++++- src/Spago/Db.purs | 6 ++++++ src/Spago/Registry.purs | 7 ++++--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/Spago/Db.js b/src/Spago/Db.js index 2d85cb3f5..a21242237 100644 --- a/src/Spago/Db.js +++ b/src/Spago/Db.js @@ -11,10 +11,10 @@ export const connectImpl = (databasePath, logger) => { const db = new DatabaseSync(databasePath, { enableForeignKeyConstraints: true, + timeout: 5000, // Wait up to 5s if database is locked (matches better-sqlite3 default) }); db.exec("PRAGMA journal_mode = WAL"); - db.exec("PRAGMA foreign_keys = ON"); db.prepare(`CREATE TABLE IF NOT EXISTS package_sets ( version TEXT PRIMARY KEY NOT NULL @@ -60,6 +60,17 @@ export const insertPackageSetEntryImpl = (db, packageSetEntry) => { ).run(packageSetEntry); } +export const withTransactionImpl = (db, action) => { + db.exec("BEGIN IMMEDIATE"); + try { + action(); + db.exec("COMMIT"); + } catch (e) { + db.exec("ROLLBACK"); + throw e; + } +} + export const selectLatestPackageSetByCompilerImpl = (db, compiler) => { const row = db .prepare("SELECT * FROM package_sets WHERE compiler = @compiler ORDER BY date DESC LIMIT 1") diff --git a/src/Spago/Db.purs b/src/Spago/Db.purs index 979ad6aaa..c98bd652d 100644 --- a/src/Spago/Db.purs +++ b/src/Spago/Db.purs @@ -17,6 +17,7 @@ module Spago.Db , selectLatestPackageSetByCompiler , selectPackageSets , updateLastPull + , withTransaction ) where import Spago.Prelude @@ -60,6 +61,9 @@ insertPackageSet db = Uncurried.runEffectFn2 insertPackageSetImpl db <<< package insertPackageSetEntry :: Db -> PackageSetEntry -> Effect Unit insertPackageSetEntry db = Uncurried.runEffectFn2 insertPackageSetEntryImpl db <<< packageSetEntryToJs +withTransaction :: Db -> Effect Unit -> Effect Unit +withTransaction db action = Uncurried.runEffectFn2 withTransactionImpl db action + selectPackageSets :: Db -> Effect (Array PackageSet) selectPackageSets db = do packageSets <- Uncurried.runEffectFn1 selectPackageSetsImpl db @@ -238,6 +242,8 @@ foreign import insertPackageSetImpl :: EffectFn2 Db PackageSetJs Unit foreign import insertPackageSetEntryImpl :: EffectFn2 Db PackageSetEntryJs Unit +foreign import withTransactionImpl :: EffectFn2 Db (Effect Unit) Unit + foreign import selectLatestPackageSetByCompilerImpl :: EffectFn2 Db String (Nullable PackageSetJs) foreign import selectPackageSetsImpl :: EffectFn1 Db (Array PackageSetJs) diff --git a/src/Spago/Registry.purs b/src/Spago/Registry.purs index b70b31f5b..03443dbba 100644 --- a/src/Spago/Registry.purs +++ b/src/Spago/Registry.purs @@ -214,9 +214,10 @@ getRegistryFns registryBox registryLock = do -- First insert the package set logDebug $ "Inserting package set in DB: " <> Version.print setVersion liftEffect $ Db.insertPackageSet db { compiler: set.compiler, date: set.published, version: set.version } - -- Then we insert every entry separately - for_ (Map.toUnfoldable set.packages :: Array _) \(Tuple name version) -> do - liftEffect $ Db.insertPackageSetEntry db { packageName: name, packageVersion: version, packageSetVersion: set.version } + -- Then we insert every entry in a transaction (avoids "database is locked" on Windows) + liftEffect $ Db.withTransaction db do + for_ (Map.toUnfoldable set.packages :: Array _) \(Tuple name version) -> do + Db.insertPackageSetEntry db { packageName: name, packageVersion: version, packageSetVersion: set.version } -- | List all the package sets versions available in the Registry repo getAvailablePackageSets :: ∀ a. Spago (LogEnv a) (Array Version) From 93dffe07a880fa25c021dfe10cd73b56488e3419 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Jan 2026 23:07:46 +0100 Subject: [PATCH 3/3] style: format NodeVersion.purs --- src/Spago/NodeVersion.purs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spago/NodeVersion.purs b/src/Spago/NodeVersion.purs index 065d56018..bfdbfb5bf 100644 --- a/src/Spago/NodeVersion.purs +++ b/src/Spago/NodeVersion.purs @@ -26,7 +26,7 @@ instance Show NodeVersionCheck where checkNodeVersion :: { major :: Int, minor :: Int } -> String -> NodeVersionCheck checkNodeVersion minimum version = case traverse Int.fromString (Array.take 2 parts) of - Just [major, minor] + Just [ major, minor ] | major > minimum.major -> NodeVersionOk | major == minimum.major && minor >= minimum.minor -> NodeVersionOk | otherwise -> NodeVersionTooOld version