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: 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..f8eb06374 --- /dev/null +++ b/utils/defaultCxxStandard.test.js @@ -0,0 +1,93 @@ +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); + } + // 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: childEnv, + }); + 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("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" }), + "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;