From 4af50180ebb408572ae4d496b5f561e7236eceed Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Oct 2026 21:15:12 +0000 Subject: [PATCH 1/4] fix: compile Node 24+ prebuilds as C++20 prebuildify passes the Node version as --target. That path treated every target as Electron and selected C++17 below version 32, overriding Node's gnu++20 default. Node 24.18 headers require C++20, so the macOS publish prebuild failed and cancelled the rest of the release matrix. Co-authored-by: Jon Ursenbach --- generate/templates/templates/binding.gyp | 2 +- utils/defaultCxxStandard.js | 57 +++++++++++++++------ utils/defaultCxxStandard.test.js | 64 ++++++++++++++++++++++++ utils/isBuildingForElectron.js | 36 +++++++------ 4 files changed, 130 insertions(+), 29 deletions(-) create mode 100644 utils/defaultCxxStandard.test.js diff --git a/generate/templates/templates/binding.gyp b/generate/templates/templates/binding.gyp index b5e189c32..88799b0fd 100644 --- a/generate/templates/templates/binding.gyp +++ b/generate/templates/templates/binding.gyp @@ -7,7 +7,7 @@ "is_IBMi%": "= 32) { - cxxStandard = '20'; - } else if (Number.parseInt(majorVersion) >= 21) { - cxxStandard = '17'; +function standardForNodeMajor(majorVersion) { + // Node 23+ V8 headers require C++20. Node 18–22 build as C++17. + if (majorVersion >= 23) { + return "20"; + } + if (majorVersion >= 18) { + return "17"; + } + return "14"; +} + +function standardForElectronMajor(majorVersion) { + // Electron 32+ is built with C++20; Electron 21–31 with C++17. + if (majorVersion >= 32) { + return "20"; + } + if (majorVersion >= 21) { + return "17"; } + return "14"; +} + +let cxxStandard = "14"; + +if (targetSpecified) { + const majorVersion = Number.parseInt(target.split(".")[0], 10); + // prebuildify always passes --target. That is a Node version unless the + // headers (or npm runtime) say this is an Electron/NW.js build. + const electronTarget = + process.env.npm_config_runtime === "electron" || + process.env.npm_config_runtime === "node-webkit" || + isBuildingForElectron(nodeRootDir); + + cxxStandard = electronTarget + ? standardForElectronMajor(majorVersion) + : standardForNodeMajor(majorVersion); } else { - const abiVersion = Number.parseInt(process.versions.modules) ?? 0; - // Node 18 === 108, Node 20 === 115 + const abiVersion = Number.parseInt(process.versions.modules, 10) || 0; + // Node 18 === 108, Node 20 === 115, Node 23 === 131 if (abiVersion >= 131) { - cxxStandard = '20'; + cxxStandard = "20"; } else if (abiVersion >= 108) { - cxxStandard = '17'; + cxxStandard = "17"; } } diff --git a/utils/defaultCxxStandard.test.js b/utils/defaultCxxStandard.test.js new file mode 100644 index 000000000..8376ddb6c --- /dev/null +++ b/utils/defaultCxxStandard.test.js @@ -0,0 +1,64 @@ +const assert = require("assert"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const { spawnSync } = require("child_process"); +const test = require("node:test"); + +const script = path.join(__dirname, "defaultCxxStandard.js"); + +function writeHeaders(builtWithElectron) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "nodegit-headers-")); + const includeDir = path.join(root, "include", "node"); + fs.mkdirSync(includeDir, { recursive: true }); + const variables = builtWithElectron + ? "{ 'variables': { 'built_with_electron': 1 } }" + : "{ 'variables': { 'node_module_version': 137 } }"; + fs.writeFileSync(path.join(includeDir, "config.gypi"), variables); + return root; +} + +function cxxStandard(target, nodeRootDir, env) { + const args = [script, target]; + if (nodeRootDir) { + args.push(nodeRootDir); + } + const result = spawnSync(process.execPath, args, { + encoding: "utf8", + env: Object.assign({}, process.env, env || {}), + }); + assert.strictEqual(result.status, 0, result.stderr); + return result.stdout; +} + +test("node prebuild targets use C++20 from Node 23 up", () => { + const headers = writeHeaders(false); + assert.strictEqual(cxxStandard("22.22.0", headers), "17"); + assert.strictEqual(cxxStandard("23.0.0", headers), "20"); + assert.strictEqual(cxxStandard("24.18.0", headers), "20"); + assert.strictEqual(cxxStandard("26.5.0", headers), "20"); +}); + +test("electron targets keep the electron C++ mapping", () => { + const headers = writeHeaders(true); + assert.strictEqual(cxxStandard("24.0.0", headers), "17"); + assert.strictEqual(cxxStandard("31.7.7", headers), "17"); + assert.strictEqual(cxxStandard("32.2.0", headers), "20"); +}); + +test("npm electron runtime is treated as electron even without headers", () => { + assert.strictEqual( + cxxStandard("28.0.0", undefined, { npm_config_runtime: "electron" }), + "17" + ); + assert.strictEqual( + cxxStandard("34.0.0", undefined, { npm_config_runtime: "electron" }), + "20" + ); +}); + +test("an unspecified target follows the running Node ABI", () => { + const abi = Number.parseInt(process.versions.modules, 10); + const expected = abi >= 131 ? "20" : abi >= 108 ? "17" : "14"; + assert.strictEqual(cxxStandard("none"), expected); +}); diff --git a/utils/isBuildingForElectron.js b/utils/isBuildingForElectron.js index 295f6ab1f..1d4eb85ca 100644 --- a/utils/isBuildingForElectron.js +++ b/utils/isBuildingForElectron.js @@ -1,17 +1,17 @@ -const fs = require("fs") +const fs = require("fs"); const JSON5 = require("json5"); const path = require("path"); -if (process.argv.length < 3) { - process.exit(1); -} - -const last = arr => arr[arr.length - 1]; -const [, , nodeRootDir] = process.argv; +function isBuildingForElectron(nodeRootDir) { + if (!nodeRootDir) { + return false; + } -let isElectron = last(nodeRootDir.split(path.sep)).startsWith("iojs"); + const last = nodeRootDir.split(path.sep).pop(); + if (last && last.startsWith("iojs")) { + return true; + } -if (!isElectron) { try { // Not ideal, would love it if there were a full featured gyp package to do this operation instead. const { variables: { built_with_electron } } = JSON5.parse( @@ -21,10 +21,18 @@ if (!isElectron) { ) ); - if (built_with_electron) { - isElectron = true; - } - } catch (e) {} + return !!built_with_electron; + } catch (e) { + return false; + } +} + +if (require.main === module) { + if (process.argv.length < 3) { + process.exit(1); + } + + process.stdout.write(isBuildingForElectron(process.argv[2]) ? "1" : "0"); } -process.stdout.write(isElectron ? "1" : "0"); +module.exports = isBuildingForElectron; From d089870d310004921ff46703c9b123a97f5243f5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Oct 2026 21:35:15 +0000 Subject: [PATCH 2/4] test: run C++ standard checks in the test suite CI launches Mocha through test/index.js and never called node --test, so the new compiler-standard checks did not run. Run them before Mocha from both npm test and npm run mocha. Co-authored-by: Jon Ursenbach --- package.json | 2 +- test/index.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 04e756246..2a2a6f878 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "installDebug": "BUILD_DEBUG=true npm install", "lint": "jshint lib test/tests test/utils lifecycleScripts", "mergecov": "lcov-result-merger 'test/**/*.info' 'test/coverage/merged.lcov' && ./lcov-1.10/bin/genhtml test/coverage/merged.lcov --output-directory test/coverage/report", - "mocha": "mocha --expose-gc test/runner test/tests --timeout 15000", + "mocha": "node --test utils/defaultCxxStandard.test.js && mocha --expose-gc test/runner test/tests --timeout 15000", "mochaDebug": "mocha --expose-gc --inspect-brk test/runner test/tests --timeout 15000", "postinstall": "node lifecycleScripts/postinstall", "rebuild": "node generate && node-gyp configure build", diff --git a/test/index.js b/test/index.js index b138525e1..02338be21 100644 --- a/test/index.js +++ b/test/index.js @@ -1,4 +1,5 @@ var fork = require("child_process").fork; +var spawnSync = require("child_process").spawnSync; var path = require("path"); var fs = require('fs'); @@ -30,6 +31,19 @@ if (!process.env.APPVEYOR && !process.env.TRAVIS && !process.env.GITHUB_ACTION) process.env.USERPROFILE = dummyPath; } +// Compiler-standard checks run before Mocha so CI covers them without loading the native addon. +var cxxStandardTests = spawnSync(process.execPath, [ + "--test", + path.join(__dirname, "../utils/defaultCxxStandard.test.js") +], { + cwd: path.join(__dirname, "../"), + stdio: "inherit" +}); + +if (cxxStandardTests.status !== 0) { + process.exit(cxxStandardTests.status || 1); +} + // unencrypt test keys function unencryptKey(fileName) { var base64Contents = fs.readFileSync( From d821f7c77efd66c64c72b4d51b043d6a2c40ccf5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Oct 2026 21:45:18 +0000 Subject: [PATCH 3/4] test: ignore inherited Electron runtime in Node standard checks npm test forwards npm_config_runtime into the new checks. An electron or node-webkit value made the Node 23 fixture select C++17 and stopped the suite before Mocha. Node fixtures now drop that inherited setting unless a test sets it on purpose. Co-authored-by: Jon Ursenbach --- utils/defaultCxxStandard.test.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/utils/defaultCxxStandard.test.js b/utils/defaultCxxStandard.test.js index 8376ddb6c..f8eb06374 100644 --- a/utils/defaultCxxStandard.test.js +++ b/utils/defaultCxxStandard.test.js @@ -23,9 +23,16 @@ function cxxStandard(target, nodeRootDir, env) { if (nodeRootDir) { args.push(nodeRootDir); } + // Drop an inherited Electron or NW.js runtime so Node fixtures stay Node + // fixtures. Callers that need that runtime pass it in env. + const childEnv = Object.assign({}, process.env); + delete childEnv.npm_config_runtime; + if (env) { + Object.assign(childEnv, env); + } const result = spawnSync(process.execPath, args, { encoding: "utf8", - env: Object.assign({}, process.env, env || {}), + env: childEnv, }); assert.strictEqual(result.status, 0, result.stderr); return result.stdout; @@ -46,6 +53,28 @@ test("electron targets keep the electron C++ mapping", () => { assert.strictEqual(cxxStandard("32.2.0", headers), "20"); }); +test("node prebuild targets ignore an inherited electron runtime", () => { + const headers = writeHeaders(false); + const previous = process.env.npm_config_runtime; + process.env.npm_config_runtime = "electron"; + try { + assert.strictEqual(cxxStandard("23.0.0", headers), "20"); + assert.strictEqual(cxxStandard("24.18.0", headers), "20"); + assert.strictEqual( + cxxStandard("28.0.0", undefined, { npm_config_runtime: "electron" }), + "17" + ); + process.env.npm_config_runtime = "node-webkit"; + assert.strictEqual(cxxStandard("24.18.0", headers), "20"); + } finally { + if (previous === undefined) { + delete process.env.npm_config_runtime; + } else { + process.env.npm_config_runtime = previous; + } + } +}); + test("npm electron runtime is treated as electron even without headers", () => { assert.strictEqual( cxxStandard("28.0.0", undefined, { npm_config_runtime: "electron" }), From 881aa0587383550ae10bffa29030897dbd4a2507 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Oct 2026 23:03:42 +0000 Subject: [PATCH 4/4] ci: name the Publish and Tests workflows The Actions list was showing these workflows by filename. Give them the same display names added on the closed prebuild-split branch. Co-authored-by: Jon Ursenbach --- .github/workflows/publish.yml | 2 ++ .github/workflows/tests.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e4cb0afbb..99c8ebc91 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1,3 +1,5 @@ +name: Publish + on: push: tags: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8823031fd..3ed6f4f0e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,3 +1,5 @@ +name: Tests + on: push: branches: