fix: compile Node 24+ prebuilds as C++20

.github/workflows/publish.yml

@@ -1,3 +1,5 @@
+name: Publish
+
 on:
   push:
     tags:

.github/workflows/tests.yml

@@ -1,3 +1,5 @@
+name: Tests
+
 on:
   push:
     branches:

generate/templates/templates/binding.gyp

@@ -7,7 +7,7 @@
     "is_IBMi%": "<!(node -p \"os.platform() == 'aix' && os.type() == 'OS400' ? 1 : 0\")",
     "electron_openssl_root%": "<!(node ./utils/getElectronOpenSSLRoot.js <(module_root_dir))",
     "electron_openssl_static%": "<!(node -p \"process.platform !== 'linux' || process.env.NODEGIT_OPENSSL_STATIC_LINK === '1' ? 1 : 0\")",
-    "cxx_version%": "<!(node ./utils/defaultCxxStandard.js <(target))",
+    "cxx_version%": "<!(node ./utils/defaultCxxStandard.js \"<(target)\" \"<(node_root_dir)\")",
     "has_cxxflags%": "<!(node -p \"process.env.CXXFLAGS ? 1 : 0\")",
     "macOS_deployment_target": "10.11",
     # https://github.com/nodejs/node-gyp/issues/2673

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",

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(

utils/defaultCxxStandard.js

@@ -1,23 +1,52 @@
-const targetSpecified = process.argv[2] !== 'none';
+const isBuildingForElectron = require("./isBuildingForElectron");
 
-let cxxStandard = '14';
+const target = process.argv[2];
+const nodeRootDir = process.argv[3];
+const targetSpecified = !!target && target !== "none";
 
-if (targetSpecified) {
-  // Assume electron if target is specified.
-  // If building node 18 / 19 via target, will need to specify C++ standard manually
-  const majorVersion = process.argv[2].split('.')[0];
-  if (Number.parseInt(majorVersion) >= 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";
   }
 }
 

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);
+});

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;